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

[Serge Pavlov]

2014-12-24 5:08 GMT+06:00 Hal Finkel <hfinkel at anl.gov>:

> ----- Original Message -----
> > From: "Richard Smith" <richard at metafoo.co.uk>
> > To: "Aaron Ballman" <aaron at aaronballman.com>
> > Cc: "Joerg Sonnenberger" <joerg at britannica.bec.de>, "llvm cfe" <
> cfe-commits at cs.uiuc.edu>
> > Sent: Thursday, December 18, 2014 4:06:38 PM
> > Subject: Re: r224012 - Emit warning if define or undef reserved
> identifier or keyword.
> >
> >
> > On Thu, Dec 18, 2014 at 5:04 AM, Aaron Ballman <
> > aaron at aaronballman.com > wrote:
> >
> > On Wed, Dec 17, 2014 at 9:55 PM, Richard Smith <
> > richard at metafoo.co.uk > wrote:
> > > On Mon, Dec 15, 2014 at 9:44 AM, Aaron Ballman <
> > > aaron at aaronballman.com >
> > > wrote:
> > >>
> > >> On Mon, Dec 15, 2014 at 12:26 PM, Joerg Sonnenberger
> > >> < joerg at britannica.bec.de > wrote:
> > >> > On Mon, Dec 15, 2014 at 11:47:23AM -0500, Aaron Ballman wrote:
> > >> >> On Mon, Dec 15, 2014 at 11:35 AM, Joerg Sonnenberger
> > >> >> < joerg at britannica.bec.de > wrote:
> > >> >> > On Mon, Dec 15, 2014 at 11:32:02AM -0500, Aaron Ballman
> > >> >> > wrote:
> > >> >> >> On Sat, Dec 13, 2014 at 2:46 PM, Joerg Sonnenberger
> > >> >> >> > (2) #define restrict __restrict__ --> code that has to
> > >> >> >> > deal with
> > >> >> >> > pre-C99
> > >> >> >> > default in GCC.
> > >> >> >>
> > >> >> >> I'm not certain this should have triggered the warning in
> > >> >> >> the first
> > >> >> >> place. Either restrict is a keyword (at which point, the
> > >> >> >> #define
> > >> >> >> should be considered possibly harmful), or restrict isn't a
> > >> >> >> keyword
> > >> >> >> (and then there's no reason for the warning in the first
> > >> >> >> place).
> > >> >> >
> > >> >> > It happens in code that has to deal with different compilers.
> > >> >> > Consider a
> > >> >> > pre-generated config.h. In NetBSD we have to deal with
> > >> >> > different GCC
> > >> >> > versions and Clang, at least the former doesn't default to
> > >> >> > C99 :(
> > >> >>
> > >> >> I'm confused. Are you talking about vendors where restrict is a
> > >> >> keyword, but it's not implemented (properly)?
> > >> >
> > >> > configure will not pick it up for GCC with the C90 default.
> > >>
> > >> Thank you for the clarification (here and on IRC), that makes a
> > >> lot
> > >> more sense to me now.
> > >>
> > >> >
> > >> >> >> > (3) #define extern, #define static --> debug, code sharing
> > >> >> >>
> > >> >> >> I think these should be a warning. If you're doing a debug
> > >> >> >> build
> > >> >> >> where
> > >> >> >> you want to turn off keywords, you can add a
> > >> >> >> -Wno-keyword-macro flag
> > >> >> >> to your command line.
> > >> >> >
> > >> >> > I disagree. Having to change the warning settings just to get
> > >> >> > a debug
> > >> >> > build to work is stupid.
> > >> >>
> > >> >> Redefining keywords to be empty macros in production code is
> > >> >> equally
> > >> >> stupid. ;-)
> > >> >>
> > >> >> > My question remains: what bugs has this warning caught? I've
> > >> >> > heard
> > >> >> > that
> > >> >> > Microsoft believes this to be reasonable to warn about, but
> > >> >> > that's
> > >> >> > about
> > >> >> > it.
> > >> >>
> > >> >> CERT also feels this is reasonable to diagnose:
> > >> >> ...
> > >> >
> > >> > Well, keep in mind that C and restricted interface contracts
> > >> > don't
> > >> > necessarily agree with each other, so often various tricks are
> > >> > played.
> > >> > So summarize, I think the following should be accepted silently
> > >> > by
> > >> > default:
> > >> >
> > >> > (1) macro name equals macro body.
> > >>
> > >> Agreed.
> > >>
> > >> > (2) macro name is a "storage" class keyword (extern, static,
> > >> > inline) and
> > >> > the body is empty.
> > >> > (3) macro name is const.
> > >>
> > >> I think this makes sense, but as an explicit whitelist of: static,
> > >> extern, inline, and const.
> > >
> > >
> > > Can you explain here on the mailing list why this
> > > seemingly-arbitrary
> > > whitelist is a good idea?
> >
> > These specific keywords are sometimes disabled for debug build
> > purposes,
> >
> >
> > I find it hard to believe that extern and static are sometimes
> > disabled for debug build purposes, since doing so is likely to
> > completely break the program. "#define inline" will also typically
> > break the program due to multiple-definition errors.
> >
> >
> >
> > I can understand "#define const /**/" for compilers that don't
> > support const, but Clang is not such a compiler, so code that does
> > that and is built with Clang seems to deserve a warning.
> >
>
> I've certainly seen plenty of code that "#define restrict /**/" when not
> compiling in C99 mode. That should probably not get a warning (unless we
> want to suggest using __restrict__ instead).
>
>
>
That should not be a problem. If mode is not C99, 'restrict' is not a
keyword.

Thanks,
--Serge
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141224/26d9796e/attachment.html>