r224012 - Emit warning if define or undef reserved identifier or keyword.

[Nico Weber]

I agree that off by default warnings are a "warning smell". But this
doesn't look like a warning that's supposed to find bugs, it's more a
language compliance thing, like -pedantic. Maybe it should be an ExtWarn.
But is someone is actively implementing the suggested heuristics, we can
wait and see how they do first, I suppose.

For the "final(a,b,c)" define: should we warn on that? It doesn't change
the meaning of the keyword "final" since that doesn't have parameters.
On Dec 15, 2014 10:14 AM, "Aaron Ballman" <aaron at aaronballman.com> wrote:

> On Mon, Dec 15, 2014 at 1:01 PM, Serge Pavlov <sepavloff at gmail.com> wrote:
> > Maybe turn this warning off by default? As Nico pointed out, there are
> > additional cases:
> >
> > #define inline _inline
>
> I think this is fine to cover from my suggestions above. _inline is an
> extension just the same as __inline is.
>
> > #define final(a,b,c) some_computation(a, b,c )
>
> This *should* warn for modes where final is a keyword because it's
> overriding the keyword's behavior.
>
> I am not keen on off-by-default warnings. I think the general rule of
> thumb is that if it's not important enough to be on by default, it may
> not be important enough to include. Personally, I think this is
> borderline, but have a preference to see it remain on by default, with
> the proper heuristics.
>
> ~Aaron
>
> >
> > They are not covered by the rules above.
> >
> > Thanks,
> > --Serge
> >
> > 2014-12-15 23:45 GMT+06:00 Aaron Ballman <aaron at aaronballman.com>:
> >>
> >> On Mon, Dec 15, 2014 at 12:11 PM, Serge Pavlov <sepavloff at gmail.com>
> >> wrote:
> >> > 2014-12-15 22:32 GMT+06:00 Aaron Ballman <aaron at aaronballman.com>:
> >> >>
> >> >>
> >> >> This commit appears to be lacking tests, and I would like to see some
> >> >> added for each of the new diagnostics, as well as for cases that
> >> >> should be valid (such as keywords in language modes that do not have
> >> >> the keyword).
> >> >>
> >> >>
> >> > Test are added in r224100.
> >>
> >> So they are, thank you for pointing that out!
> >>
> >> ~Aaron
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141215/064883dc/attachment.html>