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.
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
