| Message ID | 20161102215939.36765-4-luc.vanoostenryck@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Thu, Nov 3, 2016 at 5:59 AM, Luc Van Oostenryck <luc.vanoostenryck@gmail.com> wrote: > Generally, we won't be interested by the warnings from this flag, > but we can always explicitly ask for them if needed. I don't want to disable it by default. I think default should be on so that we can collect test case and prepare to add them later one. I would much rather see a patch to make those attribute parsed eventually. I think gcc or clang should have a collection of all the attribute stash some where. I haven't take a closer look myself. Most of the attribute are trivial to parse correctly. We should have some thing like empty_int_attribute instead of blindly ignore them. Also ideally the attribute should be a list instead of member in the ctype structure. Most of the symbol has at most one attribute any way. That is a much bigger change though. Chris -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Nov 18, 2016 at 12:52:37AM +0800, Christopher Li wrote: > On Thu, Nov 3, 2016 at 5:59 AM, Luc Van Oostenryck > <luc.vanoostenryck@gmail.com> wrote: > > Generally, we won't be interested by the warnings from this flag, > > but we can always explicitly ask for them if needed. > > I don't want to disable it by default. I think default should be on so > that we can collect test case and prepare to add them later one. > > I would much rather see a patch to make those attribute parsed > eventually. I think gcc or clang should have a collection of all > the attribute stash some where. I haven't take a closer look myself. > > Most of the attribute are trivial to parse correctly. We should have > some thing like empty_int_attribute instead of blindly ignore them. > > Also ideally the attribute should be a list instead of member in the > ctype structure. Most of the symbol has at most one attribute any > way. That is a much bigger change though. I'm not surprised that you don't want to have it disabled by default. But leaving it on would, in my opinion, miss the point entirely. Why would sparses *users* be annoyed with warnings that doesn't concern them? What kind of useful information it gives to them that sparse doesn't know about this or this new attribute? Even worse, what the difference for sparse between having this option disabled by default and just adding yet another attribute to the lists of known ones (even if it is not in the format of a list)? I have taken a look at such gcc list before I wrote this serie. There is a whole bunch of them for exotic uses on exotic architectures and a more limited number fo them for more common use. But don't forget that it's also not only the question of the existing attributes but the fact that each gcc version (and maybe clang) may add a few new ones and soon or later someone will use them. Don't take me wrong, if it would make a visible semantic difference I would be the first to say that such or such attribute need to be handled correctly. And I think that patches doing the parsing of some more attributes should be welcomed. But given what sparse curently do and is used, leaving this option enabled by default is, in my opinion, useless and just annoyance for its users. Also, I don't exagerate too much by saying that during the last 2 years or so there has been almost as much patches for these new attributes than anything else. It's a bit ridiculous. Luc -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/lib.c b/lib.c index 138736e7..b4b09a78 100644 --- a/lib.c +++ b/lib.c @@ -240,7 +240,7 @@ int Wtransparent_union = 0; int Wtypesign = 0; int Wundef = 0; int Wuninitialized = 1; -int Wunknown_attribute = 1; +int Wunknown_attribute = 0; int Wvla = 1; int dbg_entry = 0; diff --git a/validation/Wunknown-attribute-def.c b/validation/Wunknown-attribute-def.c index 0c0868d6..defe643d 100644 --- a/validation/Wunknown-attribute-def.c +++ b/validation/Wunknown-attribute-def.c @@ -4,6 +4,5 @@ static int foo(void) __attribute__((unknown_attribute)); * check-name: warn-unknown-attribute * * check-error-start -Wunknown-attribute-def.c:1:37: warning: attribute 'unknown_attribute': unknown attribute * check-error-end */
Generally, we won't be interested by the warnings from this flag, but we can always explicitly ask for them if needed. Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com> --- lib.c | 2 +- validation/Wunknown-attribute-def.c | 1 - 2 files changed, 1 insertion(+), 2 deletions(-)