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