<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Dec 18, 2014 at 5:04 AM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span> wrote:<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">On Wed, Dec 17, 2014 at 9:55 PM, Richard Smith <<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 <<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 wrote:<br>
>> >> >> On Sat, Dec 13, 2014 at 2:46 PM, Joerg Sonnenberger<br>
>> >> >> > (2) #define restrict __restrict__ --> code that has to deal with<br>
>> >> >> > pre-C99<br>
>> >> >> > default in GCC.<br>
>> >> >><br>
>> >> >> I'm not certain this should have triggered the warning in the first<br>
>> >> >> place. Either restrict is a keyword (at which point, the #define<br>
>> >> >> should be considered possibly harmful), or restrict isn't a keyword<br>
>> >> >> (and then there's no reason for the warning in the first 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 different GCC<br>
>> >> > versions and Clang, at least the former doesn't default to 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 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 build<br>
>> >> >> where<br>
>> >> >> you want to turn off keywords, you can add a -Wno-keyword-macro flag<br>
>> >> >> to your command line.<br>
>> >> ><br>
>> >> > I disagree. Having to change the warning settings just to get a debug<br>
>> >> > build to work is stupid.<br>
>> >><br>
>> >> Redefining keywords to be empty macros in production code is equally<br>
>> >> stupid. ;-)<br>
>> >><br>
>> >> > My question remains: what bugs has this warning caught? I've heard<br>
>> >> > that<br>
>> >> > Microsoft believes this to be reasonable to warn about, but 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 don't<br>
>> > necessarily agree with each other, so often various tricks are played.<br>
>> > So summarize, I think the following should be accepted silently 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, 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 seemingly-arbitrary<br>
> whitelist is a good idea?<br>
<br>
</div></div>These specific keywords are sometimes disabled for debug build<br>
purposes,</blockquote><div><br></div><div>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.</div><div><div><br></div></div><div>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.</div><div><br></div><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">and Joerg had made a compelling argument that you should not<br>
have to disable a specific warning just to get debug builds to work.<br>
When discussed, his feeling was that these keywords were the only ones<br>
compelling or reasonable enough to disable.<br>
<br>
I'm not (personally) strongly tied to the ability to effectively<br>
disable keywords, but since it sounds like at least one sizable code<br>
base relies on such behavior, it seems a reasonable suggestion.<br>
<span class=""><br>
><br>
>> > (4) macro body wrapped with __ equals macro name.<br>
>><br>
>> Agreed, but slightly more relaxed -- a macro body prefixed with __<br>
>> equals macro name, optionally with a suffixed with __. Eg)<br>
><br>
><br>
> I disagree; the right thing to check is that the macro name and macro<br>
> expansion are equivalent tokens. If we support an inline and __inline<br>
> keyword, it's fine to define one as the other, this us just a special case<br>
> of (1).<br>
<br>
</span>Agreed -- that's a much better heuristic.<br>
<span class=""><font color="#888888"><br>
~Aaron<br>
</font></span><div class=""><div class="h5"><br>
><br>
>><br>
>> #define inline __inline__ // Ok<br>
>> #define inline __inline // Ok<br>
>> #define inline // Ok<br>
>> #define inline inline // Ok<br>
>> #define inline not_inline // Not Ok<br>
>> #define inline inline__ // Not Ok<br>
>><br>
>> ~Aaron<br>
>> _______________________________________________<br>
>> cfe-commits mailing list<br>
>> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</div></div></blockquote></div></div></div>