On Mon, Dec 12, 2016 at 4:09 PM, Marek Polacek <pola...@redhat.com> wrote: > So I'm wondering how/if should we proceed with this. Seems that if we'll > go with my patch, we might have to add a bunch of sentinels (it seemed that > it's mostly warnings about GNU extensions that we want to silence for > system headers), but it's hard to gauge the overall effect.
I guess we want to make the change for warnings about how an expression is used, and not for warnings about the expression itself (as with the GNU extensions). > On Thu, Nov 03, 2016 at 11:26:04AM +0100, Marek Polacek wrote: >> On Wed, Nov 02, 2016 at 01:39:22PM -0400, Jason Merrill wrote: >> > On Wed, Nov 2, 2016 at 12:37 PM, Joseph Myers <jos...@codesourcery.com> >> > wrote: >> > > On Wed, 2 Nov 2016, Jason Merrill wrote: >> > > >> > >> It seems to me that the general principle is that we should consider >> > >> the location where the thing we're warning about is happening. In >> > >> >> > >> float_var = LLONG_MIN; >> > >> >> > >> The relevant location is that of the assignment, not the constant on >> > >> the RHS. In your ?: example, a simple answer would be to warn based >> > > >> > > I'm not sure we track locations well enough to handle that yet. >> > > >> > > Say you have an implicit conversion of a function argument to the type >> > > from the prototype and something about that conversion should be warned >> > > about. Then you should obviously warn if the argument is a macro from a >> > > system header but the call is outside a system header. But say the >> > > function call itself comes from a macro defined in a system header - you >> > > should still warn if the user passed an argument of the wrong type, even >> > > if that argument is a macro from a system header. >> > > >> > > That is: >> > > >> > > /* System header. */ >> > > int __foo (int); >> > > /* This sort of macro to avoid accidental interposition issues has been >> > > discussed lately on libc-alpha, so it's a realistic example. */ >> > > #define foo(x) (0 + __foo (x)) >> > > /* User code. */ >> > > v = foo (NULL); >> > > >> > > should warn because the call to __foo logically results from user code >> > > even though both NULL and foo are macros defined in system headers. I'm >> > > not sure what the locations look like here. >> > >> > Sure, the problem here comes from the user combining the two macros. >> > I suppose in this case you could notice that the macro expansions of >> > 'foo' and 'NULL' are not nested. >> >> That testcase is handled ok even without the patch: >> x.c: In function ‘f’: >> x.c:7:7: warning: passing argument 1 of ‘__foo’ makes integer from pointer >> without a cast [-Wint-conversion] >> v = foo (NULL); >> ^~~ >> In file included from x.c:1:0: >> x.h:2:5: note: expected ‘int’ but argument is of type ‘void *’ >> int __foo (int); >> ^~~~~ >> >> because convert_for_assignment already has the call to >> expansion_point_location_if_in_system_header. >> >> I will add that testcase to my patch. > > Marek
