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

[Aaron Ballman]

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)?

>> > (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:

https://www.securecoding.cert.org/confluence/display/seccode/DCL37-C.+Do+not+declare+or+define+a+reserved+identifier
https://www.securecoding.cert.org/confluence/display/cplusplus/DCL32-CPP.+Do+not+declare+or+define+a+reserved+identifier

(Not a perfect match to this warning, since this warning doesn't cover
cases where identifiers reserved to the library are being defined in
user code.)

Other tools also catch something like this, such as PRQA, LDRA, and ECLAIR.

FWIW, in a past life, I saw code which inexplicably did something like
#define int short for just one translation unit, and debugging that
was rather painful. However, I feel like that has to be an exceptional
piece of horrible code. ;-)

~Aaron