<div dir="ltr">In r224512 implementation changed in accordance with the recommendations. Thanks to all for feedback.<div><br><div>The solution still lacks support of an important case. As were noted, the pattern:</div></div><div><br></div><div>    #define inline</div><div><br></div><div>is used in configuration scripts. It may convert inline function to global functions, linker will report duplicate definitions in this case. The reason may be obscure for a user, because source code is OK. Finding the place where the definition takes place also may be not simple. A warning that helps discover the point where a keyword was hidden could be a great help. Compiling PHP sources is a real world example of such use case.</div><div><br></div><div>We could use warning -Wreserved-id-macro for macros that redefine a keyword allowed for configuration scripts, or introduce new warning group, which is turned off by default.</div><div><br></div></div><div class="gmail_extra"><br clear="all"><div><div class="gmail_signature">Thanks,<br>--Serge<br></div></div>
<br><div class="gmail_quote">2014-12-18 8:55 GMT+06:00 Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span>:<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5">On Mon, Dec 15, 2014 at 9:44 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:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Mon, Dec 15, 2014 at 12:26 PM, Joerg Sonnenberger<br>
<span><<a href="mailto:joerg@britannica.bec.de" target="_blank">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" target="_blank">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 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. 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>
</span>Thank you for the clarification (here and on IRC), that makes a lot<br>
more sense to me now.<br>
<span><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 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 stupid. ;-)<br>
>><br>
>> > My question remains: what bugs has this warning caught? I've heard that<br>
>> > Microsoft believes this to be reasonable to warn about, but that's 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>
</span>Agreed.<br>
<span><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>
</span>I think this makes sense, but as an explicit whitelist of: static,<br>
extern, inline, and const.</blockquote><div><br></div></div></div><div>Can you explain here on the mailing list why this seemingly-arbitrary whitelist is a good idea?</div><span class=""><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>
> (4) macro body wrapped with __ equals macro name.<br>
<br>
</span>Agreed, but slightly more relaxed -- a macro body prefixed with __<br>
equals macro name, optionally with a suffixed with __. Eg)<br></blockquote><div><br></div></span><div>I disagree; the right thing to check is that the macro name and macro expansion are equivalent tokens. If we support an inline and __inline keyword, it's fine to define one as the other, this us just a special case of (1).</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
#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>
<span><font color="#888888"><br>
~Aaron<br>
</font></span></span><span class=""><div><div>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">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></span></blockquote></div></div></div>
</blockquote></div></div>