[PATCH][RFC] -Wno-... option to suppress builtin macro redefined warnings

[PATCH][RFC] -Wno-... option to suppress builtin macro redefined warnings

[Simon Baldwin]

This patch adds a warning suppression flag, -Wno-builtin-macro-redefined,
to silence gcc warnings where builtin macros such as __TIME__ are undefined
or redefined, either on the command line or by directives.

This change permits a tightly controlled build system, one that uses
'-Werror', to redefine __TIME__, __DATE__, __TIMESTAMP__ and so on without
raising compilation errors.  As a result, such a system can generate object
files from code, containing date and time macros, that will be bitwise equal
to object files built from the same code, no matter when compiled.

Once redefined, a builtin macro is no longer builtin, and so becomes subject
to the usual warnings for redefinition (for example, if set to X, it may
only be reset to X without warning if not undefined explicitly first).

Tested and confirmed for C and C++ with testsuite and bootstrap.

Thoughts, comments, suggestions?  Aimed at trunk.  Thanks.


libcpp/ChangeLog:
2008-07-30  Simon Baldwin  <simonb@google.com>

	* include/cpplib.h (struct cpp_options): Added new boolean flag
	warn_builtin_macro_redefined.
	* init.c (cpp_create_reader): Initialize warn_builtin_macro_redefined.
	* (cpp_init_special_builtins): Add NODE_WARN to builtins only if
	warn_builtin_macro_redefined.
	* macro.c (warn_of_redefinition): Return false if a builtin macro
	is not flagged with NODE_WARN.

gcc/ChangeLog:
2008-07-30  Simon Baldwin  <simonb@google.com>

	* c-opts.c (c_common_handle_option): Added handling for
	-Wbuiltin-macro-redefined command line option.
	* c.opt: Added builtin-macro-redefined option.
	* doc/invoke.texi (Warning Options): Added -Wbuiltin-macro-redefined
	documentation.

gcc/testsuite/ChangeLog:
2008-07-30  Simon Baldwin  <simonb@google.com>

	* gcc.dg/builtin-redefine.c: New.


Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 138245)
+++ gcc/doc/invoke.texi	(working copy)
@@ -228,7 +228,8 @@ Objective-C and Objective-C++ Dialects}.
 @xref{Warning Options,,Options to Request or Suppress Warnings}.
 @gccoptlist{-fsyntax-only  -pedantic  -pedantic-errors @gol
 -w  -Wextra  -Wall  -Waddress  -Waggregate-return  -Warray-bounds @gol
--Wno-attributes -Wc++-compat -Wc++0x-compat -Wcast-align  -Wcast-qual  @gol
+-Wno-attributes -Wno-builtin-macro-redefined @gol
+-Wc++-compat -Wc++0x-compat -Wcast-align  -Wcast-qual  @gol
 -Wchar-subscripts -Wclobbered  -Wcomment @gol
 -Wconversion  -Wcoverage-mismatch  -Wno-deprecated  @gol
 -Wno-deprecated-declarations -Wdisabled-optimization  @gol
@@ -3720,6 +3721,11 @@ unrecognized attributes, function attrib
 etc.  This will not stop errors for incorrect use of supported
 attributes.
 
+@item -Wno-builtin-macro-redefined
+@opindex Wno-builtin-macro-redefined
+@opindex Wbuiltin-macro-redefined
+Do not warn if a builtin macro, such as @code{__TIME__}, is redefined.
+
 @item -Wstrict-prototypes @r{(C and Objective-C only)}
 @opindex Wstrict-prototypes
 @opindex Wno-strict-prototypes
Index: gcc/testsuite/gcc.dg/builtin-redefine.c
===================================================================
--- gcc/testsuite/gcc.dg/builtin-redefine.c	(revision 0)
+++ gcc/testsuite/gcc.dg/builtin-redefine.c	(revision 0)
@@ -0,0 +1,51 @@
+/* Test -Wno-builtin-macro-redefined warnings.  */
+
+/* { dg-do compile } */
+/* { dg-options "-Wno-builtin-macro-redefined -U__DATE__ -D__TIME__=X" } */
+
+#if defined(__DATE__)
+#error "-U error: __DATE__ is defined, but should not be"
+#endif
+
+#if __TIME__ != X
+#error "-D error: __TIME__ is not defined as expected"
+#endif
+
+#if !defined(__TIMESTAMP__)
+#error "Builtin macro error: __TIMESTAMP__ is not defined"
+#endif
+
+
+#undef __TIME__              /* Undefine while defined.  */
+#undef __TIME__              /* Undefine while already undefined.  */
+
+#define __TIME__ "X"         /* Define while undefined.  */
+#define __TIME__ "X"         /* Re-define while defined.  */
+
+#define __TIME__ "Y"         /* { dg-warning "\"__TIME__\" redefined" } */
+/* { dg-warning "previous definition" "" { target *-*-* } 23 } */
+
+#undef __TIME__              /* Undefine while defined.  */
+
+
+#undef __DATE__              /* Undefine while already undefined.  */
+
+#define __DATE__ "X"         /* Define while undefined.  */
+#define __DATE__ "X"         /* Re-define while defined.  */
+
+#define __DATE__ "Y"         /* { dg-warning "\"__DATE__\" redefined" } */
+/* { dg-warning "previous definition" "" { target *-*-* } 34 } */
+
+#undef __DATE__              /* Undefine while defined.  */
+
+
+#define __TIMESTAMP__ "X"    /* Define while undefined.  */
+#define __TIMESTAMP__ "X"    /* Re-define while defined.  */
+
+#define __TIMESTAMP__ "Y"    /* { dg-warning "\"__TIMESTAMP__\" redefined" } */
+/* { dg-warning "previous definition" "" { target *-*-* } 43 } */
+
+#undef __TIMESTAMP__         /* Undefine while defined.  */
+
+
+int unused;  /* Silence `ISO C forbids an empty translation unit' warning.  */
Index: gcc/c.opt
===================================================================
--- gcc/c.opt	(revision 138245)
+++ gcc/c.opt	(working copy)
@@ -131,6 +131,10 @@ Wbad-function-cast
 C ObjC Var(warn_bad_function_cast) Warning
 Warn about casting functions to incompatible types
 
+Wbuiltin-macro-redefined
+C ObjC C++ ObjC++ Warning
+Warn when a builtin preprocessor macro is undefined or redefined
+
 Wc++-compat
 C ObjC Var(warn_cxx_compat) Warning
 Warn about C constructs that are not in the common subset of C and C++
Index: gcc/c-opts.c
===================================================================
--- gcc/c-opts.c	(revision 138245)
+++ gcc/c-opts.c	(working copy)
@@ -423,6 +423,10 @@ c_common_handle_option (size_t scode, co
 	warn_pointer_sign = 1;
       break;
 
+    case OPT_Wbuiltin_macro_redefined:
+      cpp_opts->warn_builtin_macro_redefined = value;
+      break;
+
     case OPT_Wcomment:
     case OPT_Wcomments:
       cpp_opts->warn_comments = value;
Index: libcpp/macro.c
===================================================================
--- libcpp/macro.c	(revision 138245)
+++ libcpp/macro.c	(working copy)
@@ -1392,6 +1392,10 @@ warn_of_redefinition (cpp_reader *pfile,
   if (node->flags & NODE_WARN)
     return true;
 
+  /* Suppress warnings for builtins that lack the NODE_WARN flag.  */
+  if (node->flags & NODE_BUILTIN)
+    return false;
+
   /* Redefinitions of conditional (context-sensitive) macros, on
      the other hand, must be allowed silently.  */
   if (node->flags & NODE_CONDITIONAL)
Index: libcpp/include/cpplib.h
===================================================================
--- libcpp/include/cpplib.h	(revision 138245)
+++ libcpp/include/cpplib.h	(working copy)
@@ -349,6 +349,10 @@ struct cpp_options
      Presumably the usage is protected by the appropriate #ifdef.  */
   unsigned char warn_variadic_macros;
 
+  /* Nonzero means warn about builtin macros that are redefined or
+     explicitly undefined.  */
+  unsigned char warn_builtin_macro_redefined;
+
   /* Nonzero means turn warnings into errors.  */
   unsigned char warnings_are_errors;
 
Index: libcpp/init.c
===================================================================
--- libcpp/init.c	(revision 138245)
+++ libcpp/init.c	(working copy)
@@ -163,6 +163,7 @@ cpp_create_reader (enum c_lang lang, has
   CPP_OPTION (pfile, dollars_in_ident) = 1;
   CPP_OPTION (pfile, warn_dollars) = 1;
   CPP_OPTION (pfile, warn_variadic_macros) = 1;
+  CPP_OPTION (pfile, warn_builtin_macro_redefined) = 1;
   CPP_OPTION (pfile, warn_normalize) = normalized_C;
 
   /* Default CPP arithmetic to something sensible for the host for the
@@ -376,7 +377,9 @@ cpp_init_special_builtins (cpp_reader *p
     {
       cpp_hashnode *hp = cpp_lookup (pfile, b->name, b->len);
       hp->type = NT_MACRO;
-      hp->flags |= NODE_BUILTIN | NODE_WARN;
+      hp->flags |= NODE_BUILTIN;
+      if (CPP_OPTION (pfile, warn_builtin_macro_redefined))
+	hp->flags |= NODE_WARN;
       hp->value.builtin = (enum builtin_type) b->value;
     }
 }

Re: [PATCH][RFC] -Wno-... option to suppress builtin macro redefined warnings

[Joseph S. Myers]

On Wed, 30 Jul 2008, Simon Baldwin wrote:

> +Do not warn if a builtin macro, such as @code{__TIME__}, is redefined.

The spelling is "built-in" as an adjective in documentation; see 
codingconventions.html.

> +Warn when a builtin preprocessor macro is undefined or redefined

Likewise.

I'll leave the actual patch to preprocessor maintainers to review.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH][RFC] -Wno-... option to suppress builtin macro redefined warnings

[Tom Tromey]

>>>>> "Simon" == Simon Baldwin <simonb@google.com> writes:

Simon> This patch adds a warning suppression flag,
Simon> -Wno-builtin-macro-redefined, to silence gcc warnings where
Simon> builtin macros such as __TIME__ are undefined or redefined,
Simon> either on the command line or by directives.

This seems like a reasonable thing to want to do.

Simon> Thoughts, comments, suggestions?  Aimed at trunk.  Thanks.

The libcpp bits look generally ok.

However, it seems to me that we would want to allow redefinition of
some macros (__TIME__ et al) but not others (e.g., __LINE__).

So, how about splitting builtin_array into two pieces (and just FYI,
there's a comment above referring to "two tables" that should be
changed) and then unconditionally setting NODE_WARN for one table but
not the other?  Or, just adding a special case in the builtin
definition loop for the BT_* constants we care to allow?

Tom

Re: [PATCH][RFC] -Wno-... option to suppress builtin macro redefined warnings

[Manuel López-Ibáñez]

2008/8/8 Tom Tromey <tromey@redhat.com>:
>>>>>> "Simon" == Simon Baldwin <simonb@google.com> writes:
>
> Simon> This patch adds a warning suppression flag,
> Simon> -Wno-builtin-macro-redefined, to silence gcc warnings where
> Simon> builtin macros such as __TIME__ are undefined or redefined,
> Simon> either on the command line or by directives.
>
> This seems like a reasonable thing to want to do.
>

Do we need an option? Why not allow redefinition of (some) builtin
macros by default?

Cheers,

Manuel.

Re: [PATCH][RFC] -Wno-... option to suppress builtin macro redefined warnings

[Tom Tromey]

>>>>> "Manuel" == Manuel López-Ibáñez <lopezibanez@gmail.com> writes:

Manuel> 2008/8/8 Tom Tromey <tromey@redhat.com>:
>>>>>>> "Simon" == Simon Baldwin <simonb@google.com> writes:

Simon> This patch adds a warning suppression flag,
Simon> -Wno-builtin-macro-redefined, to silence gcc warnings where
Simon> builtin macros such as __TIME__ are undefined or redefined,
Simon> either on the command line or by directives.

>> This seems like a reasonable thing to want to do.

Manuel> Do we need an option? Why not allow redefinition of (some) builtin
Manuel> macros by default?

For standard predefined macros (at least __DATE__ and __TIME__), the
standard prohibits redefinition.  So, I think it makes sense to
require users to ask for this.

We could do something different for GNU C, I guess.  By default I'd
prefer not to add a case here, but I don't feel very strongly about
it.  I do think it makes sense to let users use this new feature
without committing to GNU C, though.  So, at least IMO, the option is
needed regardless.

Tom

Re: [PATCH][RFC] -Wno-... option to suppress builtin macro redefined warnings

[Simon Baldwin]

Tom Tromey wrote:
>>>>>> "Simon" == Simon Baldwin <simonb@google.com> writes:
>>>>>>             
>
> ...
> However, it seems to me that we would want to allow redefinition of
> some macros (__TIME__ et al) but not others (e.g., __LINE__).
>
> So, how about splitting builtin_array into two pieces (and just FYI,
> there's a comment above referring to "two tables" that should be
> changed) and then unconditionally setting NODE_WARN for one table but
> not the other?  Or, just adding a special case in the builtin
> definition loop for the BT_* constants we care to allow.
>   

Thank you for the note.

I guess that in general it just seems more, um, seamless to either allow 
or disallow any builtin macro to be redefined.  It means that there's no 
"is it redefinable or not?" decision to be made when new builtins are 
added.  It also neatly sidesteps the issue of then having to document 
which builtins are redefinable and which aren't (also no update to this 
doc should new builtins be added), or having to deal with requests to 
move builtins between redefinable/fixed groups.  In other words, the 
lowest impact on future code maintainers consistent with low impact on 
current code.

Granted, somebody could well redefine __LINE__ or similar and make a 
mess of compilation, but there are of course plenty of other ways to 
make a mess of compilation with other gcc flags.  
-Wno-builtin-macro-redefined is aimed at the control-freak automated 
build system rather than the casual user, so it seemed okay, to me 
anyway, for it to bear closer resemblance to a chainsaw than to a scalpel.

That said, I'm not wildly opposed to creating two "classes" of builtin.  
It just seems like doing so might sow slightly more confusion than it 
prevents.

--S

Re: [PATCH][RFC] -Wno-... option to suppress builtin macro redefined warnings

[Simon Baldwin]

Simon Baldwin wrote:
> Tom Tromey wrote:
>>>>>>> "Simon" == Simon Baldwin <simonb@google.com> writes:
>>>>>>>             
>>
>> ...
>> However, it seems to me that we would want to allow redefinition of
>> some macros (__TIME__ et al) but not others (e.g., __LINE__).
>>
>> So, how about splitting builtin_array into two pieces (and just FYI,
>> there's a comment above referring to "two tables" that should be
>> changed) and then unconditionally setting NODE_WARN for one table but
>> not the other?  Or, just adding a special case in the builtin
>> definition loop for the BT_* constants we care to allow.
>>   
>
> Thank you for the note.
>
> I guess that in general it just seems more, um, seamless to either 
> allow or disallow any builtin macro to be redefined.  It means that 
> there's no "is it redefinable or not?" decision to be made when new 
> builtins are added.  It also neatly sidesteps the issue of then having 
> to document which builtins are redefinable and which aren't (also no 
> update to this doc should new builtins be added), or having to deal 
> with requests to move builtins between redefinable/fixed groups.  In 
> other words, the lowest impact on future code maintainers consistent 
> with low impact on current code.
>
> Granted, somebody could well redefine __LINE__ or similar and make a 
> mess of compilation, but there are of course plenty of other ways to 
> make a mess of compilation with other gcc flags.  
> -Wno-builtin-macro-redefined is aimed at the control-freak automated 
> build system rather than the casual user, so it seemed okay, to me 
> anyway, for it to bear closer resemblance to a chainsaw than to a 
> scalpel.
>
> That said, I'm not wildly opposed to creating two "classes" of 
> builtin.  It just seems like doing so might sow slightly more 
> confusion than it prevents.

Tom, any further thoughts on this?

It's certainly not hard to split built-in macros into two tiers, those 
where redefinition warning can be suppressed by providing 
-Wno-builtin-macro-redefined, and those where it can't.  However, since 
it's all just about suppressing a warning, it may be that one policy to 
cover all of them will suffice, and be simpler to manage and maintain.

Thanks.

Re: [PATCH][RFC] -Wno-... option to suppress builtin macro redefined warnings

[Tom Tromey]

>>>>> "Simon" == Simon Baldwin <simonb@google.com> writes:

Simon> Tom, any further thoughts on this?

Simon> It's certainly not hard to split built-in macros into two tiers,
Simon> those where redefinition warning can be suppressed by providing
Simon> -Wno-builtin-macro-redefined, and those where it can't.  However,
Simon> since it's all just about suppressing a warning, it may be that one
Simon> policy to cover all of them will suffice, and be simpler to manage
Simon> and maintain.


Simon> No response from Tom to date.

Sorry, I had a bit of a mail problem and missed a few things.

I think that, as a rule of thumb, we should only relax existing
pedantic checks for a good reason.  My thinking here is that, in the
past, we've had bad experiences with relaxing these rules, and so it
is best to defer it as long as we possibly can.

In this case, I can understand wanting to override date or time
macros.  However, I couldn't think of a scenario where it would make
sense to modify __LINE__.  So, I still think it would be better to
separate the cases.  I don't think the maintenance burden is a major
problem.

Perhaps some other GCC maintainer would care to weigh in.

Tom

Re: [PATCH][RFC] -Wno-... option to suppress builtin macro redefined warnings

[Simon Baldwin]

[-- Attachment #1: Type: text/plain, Size: 1823 bytes --]

Tom Tromey wrote:
>>>>>> "Simon" == Simon Baldwin <simonb@google.com> writes:
>>>>>>             
>
> Simon> Tom, any further thoughts on this?
>
> Simon> It's certainly not hard to split built-in macros into two tiers,
> Simon> those where redefinition warning can be suppressed by providing
> Simon> -Wno-builtin-macro-redefined, and those where it can't.  However,
> Simon> since it's all just about suppressing a warning, it may be that one
> Simon> policy to cover all of them will suffice, and be simpler to manage
> Simon> and maintain.
>
>
> Simon> No response from Tom to date.
>
> Sorry, I had a bit of a mail problem and missed a few things.
>
> I think that, as a rule of thumb, we should only relax existing
> pedantic checks for a good reason.  My thinking here is that, in the
> past, we've had bad experiences with relaxing these rules, and so it
> is best to defer it as long as we possibly can.
>
> In this case, I can understand wanting to override date or time
> macros.  However, I couldn't think of a scenario where it would make
> sense to modify __LINE__.  So, I still think it would be better to
> separate the cases.  I don't think the maintenance burden is a major
> problem.
>
> Perhaps some other GCC maintainer would care to weigh in.
>   

Nothing additional so far...

...so, attached below is a re-draft of this patch to bifurcate built-in 
macros so that only TIMESTAMP, TIME, DATE, FILE, and FILE_BASE are 
affected by -Wno-builtin-macro-redefined, and other built-in macros 
always warn if redefined.

Please take another look when time permits, to see if you like this 
version better than the first.  And thanks again for the feedback.

--S

-- 
Google UK Limited | Registered Office: Belgrave House, 76 Buckingham
Palace Road, London SW1W 9TQ | Registered in England Number: 3977902


[-- Attachment #2: builtin-redefined.patch --]
[-- Type: text/x-patch, Size: 12237 bytes --]

This patch adds a warning suppression flag, -Wno-builtin-macro-redefined,
to silence gcc warnings where builtin macros such as __TIME__ are undefined
or redefined, either on the command line or by directives.

This change permits a tightly controlled build system, one that uses
'-Werror', to redefine __TIME__, __DATE__, __TIMESTAMP__ and selected other
built-in macros without raising compilation errors.  As a result, such a
system can generate object files from code, containing date and time macros,
that will be bitwise equal to object files built from the same code, no
matter when compiled.

Once redefined, a builtin macro is no longer builtin, and so becomes subject
to the usual warnings for redefinition (for example, if set to X, it may
only be reset to X without warning if not undefined explicitly first).

Tested and confirmed for C and C++ with testsuite and bootstrap.


libcpp/ChangeLog:
2008-08-26  Simon Baldwin  <simonb@google.com>

	* include/cpplib.h (struct cpp_options): Add new boolean flag
	warn_builtin_macro_redefined.
	* init.c (cpp_create_reader): Initialize warn_builtin_macro_redefined.
	* (struct builtin_operator): Split out from previous struct builtin,
	enhance extra const correctness.
	* (struct builtin_macro): Split out from previous struct builtin, add
	new always_warn_if_redefined flag, enhance const correctness.
	* (mark_named_operators): Use struct builtin_operator.
	* (cpp_init_special_builtins): Use struct builtin_macro, add NODE_WARN
	to builtins selectively.
	* macro.c (warn_of_redefinition): Return false if a builtin macro
	is not flagged with NODE_WARN.

gcc/ChangeLog:
2008-08-26  Simon Baldwin  <simonb@google.com>

	* c-opts.c (c_common_handle_option): Add handling for
	-Wbuiltin-macro-redefined command line option.
	* c.opt: Added builtin-macro-redefined option.
	* doc/invoke.texi (Warning Options): Add -Wbuiltin-macro-redefined
	documentation.

gcc/testsuite/ChangeLog:
2008-08-26  Simon Baldwin  <simonb@google.com>

	* gcc.dg/builtin-redefine.c: New.


Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 139589)
+++ gcc/doc/invoke.texi	(working copy)
@@ -228,7 +228,8 @@ Objective-C and Objective-C++ Dialects}.
 @xref{Warning Options,,Options to Request or Suppress Warnings}.
 @gccoptlist{-fsyntax-only  -pedantic  -pedantic-errors @gol
 -w  -Wextra  -Wall  -Waddress  -Waggregate-return  -Warray-bounds @gol
--Wno-attributes -Wc++-compat -Wc++0x-compat -Wcast-align  -Wcast-qual  @gol
+-Wno-attributes -Wno-builtin-macro-redefined @gol
+-Wc++-compat -Wc++0x-compat -Wcast-align  -Wcast-qual  @gol
 -Wchar-subscripts -Wclobbered  -Wcomment @gol
 -Wconversion  -Wcoverage-mismatch  -Wno-deprecated  @gol
 -Wno-deprecated-declarations -Wdisabled-optimization  @gol
@@ -3716,6 +3717,13 @@ unrecognized attributes, function attrib
 etc.  This will not stop errors for incorrect use of supported
 attributes.
 
+@item -Wno-builtin-macro-redefined
+@opindex Wno-builtin-macro-redefined
+@opindex Wbuiltin-macro-redefined
+Do not warn if a certain built-in macros are redefined.  This suppresses
+warnings for redefinition of @code{__TIMESTAMP__}, @code{__TIME__},
+@code{__DATE__}, @code{__FILE__}, and @code{__BASE_FILE__}.
+
 @item -Wstrict-prototypes @r{(C and Objective-C only)}
 @opindex Wstrict-prototypes
 @opindex Wno-strict-prototypes
Index: gcc/testsuite/gcc.dg/builtin-redefine.c
===================================================================
--- gcc/testsuite/gcc.dg/builtin-redefine.c	(revision 0)
+++ gcc/testsuite/gcc.dg/builtin-redefine.c	(revision 0)
@@ -0,0 +1,79 @@
+/* Test -Wno-builtin-macro-redefined warnings.  */
+
+/* { dg-do compile } */
+/* { dg-options "-Wno-builtin-macro-redefined -U__DATE__ -D__TIME__=X" } */
+
+/* Check date, time, and datestamp built-ins warnings may be suppressed.  */
+
+#if defined(__DATE__)
+#error "__DATE__ is defined, but should not be (-U command line error)"
+/* { dg-bogus "__DATE__ is defined" "" { target *-*-* } 9 } */
+#endif
+
+#if __TIME__ != X
+#error "__TIME__ is not defined as expected (-D command line error)"
+/* { dg-bogus "__TIME__ is not defined" "" { target *-*-* } 14 } */
+#endif
+
+#if !defined(__TIMESTAMP__)
+#error "__TIMESTAMP__ is not defined (built-in macro expectation error)"
+/* { dg-bogus "__TIMESTAMP__ is not defined" "" { target *-*-* } 19 } */
+#endif
+
+
+#undef __TIME__              /* Undefine while defined.  */
+#undef __TIME__              /* Undefine while already undefined.  */
+
+#define __TIME__ "X"         /* Define while undefined.  */
+#define __TIME__ "X"         /* Re-define while defined.  */
+
+#define __TIME__ "Y"         /* { dg-warning "\"__TIME__\" redefined" } */
+/* { dg-warning "previous definition" "" { target *-*-* } 28 } */
+
+#undef __TIME__              /* Undefine while defined.  */
+
+
+#undef __DATE__              /* Undefine while already undefined.  */
+
+#define __DATE__ "X"         /* Define while undefined.  */
+#define __DATE__ "X"         /* Re-define while defined.  */
+
+#define __DATE__ "Y"         /* { dg-warning "\"__DATE__\" redefined" } */
+/* { dg-warning "previous definition" "" { target *-*-* } 39 } */
+
+#undef __DATE__              /* Undefine while defined.  */
+
+
+#define __TIMESTAMP__ "X"    /* Define while already defined.  */
+#define __TIMESTAMP__ "X"    /* Re-define while defined.  */
+
+#define __TIMESTAMP__ "Y"    /* { dg-warning "\"__TIMESTAMP__\" redefined" } */
+/* { dg-warning "previous definition" "" { target *-*-* } 48 } */
+
+#undef __TIMESTAMP__         /* Undefine while defined.  */
+
+
+/* Check other built-ins with warnings that may be suppressed.  */
+
+#if !defined(__FILE__) || !defined(__BASE_FILE__)
+#error "Expected built-in is not defined (built-in macro expectation error)"
+/* { dg-bogus "Expected built-in is not defined" "" { target *-*-* } 59 } */
+#endif
+
+#define __FILE__ "X"         /* Define while already defined.  */
+#define __BASE_FILE__ "X"    /* Define while already defined.  */
+
+
+/* Check selected built-ins not affected by warning suppression. */
+
+#if !defined(__LINE__) || !defined(__INCLUDE_LEVEL__) || !defined(__COUNTER__)
+#error "Expected built-in is not defined (built-in macro expectation error)"
+/* { dg-bogus "Expected built-in is not defined" "" { target *-*-* } 70 } */
+#endif
+
+#define __LINE__ 0           /* { dg-warning "\"__LINE__\" redef" } */
+#define __INCLUDE_LEVEL__ 0  /* { dg-warning "\"__INCLUDE_LEVEL__\" redef" } */
+#define __COUNTER__ 0        /* { dg-warning "\"__COUNTER__\" redef" } */
+
+
+int unused;  /* Silence `ISO C forbids an empty translation unit' warning.  */
Index: gcc/c.opt
===================================================================
--- gcc/c.opt	(revision 139589)
+++ gcc/c.opt	(working copy)
@@ -131,6 +131,10 @@ Wbad-function-cast
 C ObjC Var(warn_bad_function_cast) Warning
 Warn about casting functions to incompatible types
 
+Wbuiltin-macro-redefined
+C ObjC C++ ObjC++ Warning
+Warn when a built-in preprocessor macro is undefined or redefined
+
 Wc++-compat
 C ObjC Var(warn_cxx_compat) Warning
 Warn about C constructs that are not in the common subset of C and C++
Index: gcc/c-opts.c
===================================================================
--- gcc/c-opts.c	(revision 139589)
+++ gcc/c-opts.c	(working copy)
@@ -426,6 +426,10 @@ c_common_handle_option (size_t scode, co
 	warn_pointer_sign = 1;
       break;
 
+    case OPT_Wbuiltin_macro_redefined:
+      cpp_opts->warn_builtin_macro_redefined = value;
+      break;
+
     case OPT_Wcomment:
     case OPT_Wcomments:
       cpp_opts->warn_comments = value;
Index: libcpp/macro.c
===================================================================
--- libcpp/macro.c	(revision 139589)
+++ libcpp/macro.c	(working copy)
@@ -1392,6 +1392,10 @@ warn_of_redefinition (cpp_reader *pfile,
   if (node->flags & NODE_WARN)
     return true;
 
+  /* Suppress warnings for builtins that lack the NODE_WARN flag.  */
+  if (node->flags & NODE_BUILTIN)
+    return false;
+
   /* Redefinitions of conditional (context-sensitive) macros, on
      the other hand, must be allowed silently.  */
   if (node->flags & NODE_CONDITIONAL)
Index: libcpp/include/cpplib.h
===================================================================
--- libcpp/include/cpplib.h	(revision 139589)
+++ libcpp/include/cpplib.h	(working copy)
@@ -349,6 +349,10 @@ struct cpp_options
      Presumably the usage is protected by the appropriate #ifdef.  */
   unsigned char warn_variadic_macros;
 
+  /* Nonzero means warn about builtin macros that are redefined or
+     explicitly undefined.  */
+  unsigned char warn_builtin_macro_redefined;
+
   /* Nonzero means turn warnings into errors.  */
   unsigned char warnings_are_errors;
 
Index: libcpp/init.c
===================================================================
--- libcpp/init.c	(revision 139589)
+++ libcpp/init.c	(working copy)
@@ -163,6 +163,7 @@ cpp_create_reader (enum c_lang lang, has
   CPP_OPTION (pfile, dollars_in_ident) = 1;
   CPP_OPTION (pfile, warn_dollars) = 1;
   CPP_OPTION (pfile, warn_variadic_macros) = 1;
+  CPP_OPTION (pfile, warn_builtin_macro_redefined) = 1;
   CPP_OPTION (pfile, warn_normalize) = normalized_C;
 
   /* Default CPP arithmetic to something sensible for the host for the
@@ -303,31 +304,41 @@ cpp_destroy (cpp_reader *pfile)
    altered through #define, and #if recognizes them as operators.  In
    C, these are not entered into the hash table at all (but see
    <iso646.h>).  The value is a token-type enumerator.  */
-struct builtin
+struct builtin_macro
 {
-  const uchar *name;
-  unsigned short len;
-  unsigned short value;
+  const uchar *const name;
+  const unsigned short len;
+  const unsigned short value;
+  const bool always_warn_if_redefined;
 };
 
-#define B(n, t)    { DSC(n), t }
-static const struct builtin builtin_array[] =
+#define B(n, t, f)    { DSC(n), t, f }
+static const struct builtin_macro builtin_array[] =
 {
-  B("__TIMESTAMP__",	 BT_TIMESTAMP),
-  B("__TIME__",		 BT_TIME),
-  B("__DATE__",		 BT_DATE),
-  B("__FILE__",		 BT_FILE),
-  B("__BASE_FILE__",	 BT_BASE_FILE),
-  B("__LINE__",		 BT_SPECLINE),
-  B("__INCLUDE_LEVEL__", BT_INCLUDE_LEVEL),
-  B("__COUNTER__",	 BT_COUNTER),
+  B("__TIMESTAMP__",	 BT_TIMESTAMP,     false),
+  B("__TIME__",		 BT_TIME,          false),
+  B("__DATE__",		 BT_DATE,          false),
+  B("__FILE__",		 BT_FILE,          false),
+  B("__BASE_FILE__",	 BT_BASE_FILE,     false),
+  B("__LINE__",		 BT_SPECLINE,      true),
+  B("__INCLUDE_LEVEL__", BT_INCLUDE_LEVEL, true),
+  B("__COUNTER__",	 BT_COUNTER,       true),
   /* Keep builtins not used for -traditional-cpp at the end, and
      update init_builtins() if any more are added.  */
-  B("_Pragma",		 BT_PRAGMA),
-  B("__STDC__",		 BT_STDC),
+  B("_Pragma",		 BT_PRAGMA,        true),
+  B("__STDC__",		 BT_STDC,          true),
+};
+#undef B
+
+struct builtin_operator
+{
+  const uchar *const name;
+  const unsigned short len;
+  const unsigned short value;
 };
 
-static const struct builtin operator_array[] =
+#define B(n, t)    { DSC(n), t }
+static const struct builtin_operator operator_array[] =
 {
   B("and",	CPP_AND_AND),
   B("and_eq",	CPP_AND_EQ),
@@ -347,7 +358,7 @@ static const struct builtin operator_arr
 static void
 mark_named_operators (cpp_reader *pfile)
 {
-  const struct builtin *b;
+  const struct builtin_operator *b;
 
   for (b = operator_array;
        b < (operator_array + ARRAY_SIZE (operator_array));
@@ -363,7 +374,7 @@ mark_named_operators (cpp_reader *pfile)
 void
 cpp_init_special_builtins (cpp_reader *pfile)
 {
-  const struct builtin *b;
+  const struct builtin_macro *b;
   size_t n = ARRAY_SIZE (builtin_array);
 
   if (CPP_OPTION (pfile, traditional))
@@ -376,7 +387,10 @@ cpp_init_special_builtins (cpp_reader *p
     {
       cpp_hashnode *hp = cpp_lookup (pfile, b->name, b->len);
       hp->type = NT_MACRO;
-      hp->flags |= NODE_BUILTIN | NODE_WARN;
+      hp->flags |= NODE_BUILTIN;
+      if (b->always_warn_if_redefined
+          || CPP_OPTION (pfile, warn_builtin_macro_redefined))
+	hp->flags |= NODE_WARN;
       hp->value.builtin = (enum builtin_type) b->value;
     }
 }

Re: [PATCH][RFC] -Wno-... option to suppress builtin macro redefined warnings

[Simon Baldwin]

Ping -- Tom?  Anyone?  Thanks.

--S


Simon Baldwin wrote:
> Tom Tromey wrote:
>>>>>>> "Simon" == Simon Baldwin <simonb@google.com> writes:
>>>>>>>             
>>
>> Simon> Tom, any further thoughts on this?
>>
>> Simon> It's certainly not hard to split built-in macros into two tiers,
>> Simon> those where redefinition warning can be suppressed by providing
>> Simon> -Wno-builtin-macro-redefined, and those where it can't.  However,
>> Simon> since it's all just about suppressing a warning, it may be 
>> that one
>> Simon> policy to cover all of them will suffice, and be simpler to 
>> manage
>> Simon> and maintain.
>>
>>
>> Simon> No response from Tom to date.
>>
>> Sorry, I had a bit of a mail problem and missed a few things.
>>
>> I think that, as a rule of thumb, we should only relax existing
>> pedantic checks for a good reason.  My thinking here is that, in the
>> past, we've had bad experiences with relaxing these rules, and so it
>> is best to defer it as long as we possibly can.
>>
>> In this case, I can understand wanting to override date or time
>> macros.  However, I couldn't think of a scenario where it would make
>> sense to modify __LINE__.  So, I still think it would be better to
>> separate the cases.  I don't think the maintenance burden is a major
>> problem.
>>
>> Perhaps some other GCC maintainer would care to weigh in.
>>   
>
> Nothing additional so far...
>
> ...so, attached below is a re-draft of this patch to bifurcate 
> built-in macros so that only TIMESTAMP, TIME, DATE, FILE, and 
> FILE_BASE are affected by -Wno-builtin-macro-redefined, and other 
> built-in macros always warn if redefined.
>
> Please take another look when time permits, to see if you like this 
> version better than the first.  And thanks again for the feedback.
>
> --S
>
> ------------------------------------------------------------------------
>
> This patch adds a warning suppression flag, -Wno-builtin-macro-redefined,
> to silence gcc warnings where builtin macros such as __TIME__ are undefined
> or redefined, either on the command line or by directives.
>
> This change permits a tightly controlled build system, one that uses
> '-Werror', to redefine __TIME__, __DATE__, __TIMESTAMP__ and selected other
> built-in macros without raising compilation errors.  As a result, such a
> system can generate object files from code, containing date and time macros,
> that will be bitwise equal to object files built from the same code, no
> matter when compiled.
>
> Once redefined, a builtin macro is no longer builtin, and so becomes subject
> to the usual warnings for redefinition (for example, if set to X, it may
> only be reset to X without warning if not undefined explicitly first).
>
> Tested and confirmed for C and C++ with testsuite and bootstrap.
>
>
> libcpp/ChangeLog:
> 2008-08-26  Simon Baldwin  <simonb@google.com>
>
> 	* include/cpplib.h (struct cpp_options): Add new boolean flag
> 	warn_builtin_macro_redefined.
> 	* init.c (cpp_create_reader): Initialize warn_builtin_macro_redefined.
> 	* (struct builtin_operator): Split out from previous struct builtin,
> 	enhance extra const correctness.
> 	* (struct builtin_macro): Split out from previous struct builtin, add
> 	new always_warn_if_redefined flag, enhance const correctness.
> 	* (mark_named_operators): Use struct builtin_operator.
> 	* (cpp_init_special_builtins): Use struct builtin_macro, add NODE_WARN
> 	to builtins selectively.
> 	* macro.c (warn_of_redefinition): Return false if a builtin macro
> 	is not flagged with NODE_WARN.
>
> gcc/ChangeLog:
> 2008-08-26  Simon Baldwin  <simonb@google.com>
>
> 	* c-opts.c (c_common_handle_option): Add handling for
> 	-Wbuiltin-macro-redefined command line option.
> 	* c.opt: Added builtin-macro-redefined option.
> 	* doc/invoke.texi (Warning Options): Add -Wbuiltin-macro-redefined
> 	documentation.
>
> gcc/testsuite/ChangeLog:
> 2008-08-26  Simon Baldwin  <simonb@google.com>
>
> 	* gcc.dg/builtin-redefine.c: New.
>
>
> Index: gcc/doc/invoke.texi
> ===================================================================
> --- gcc/doc/invoke.texi	(revision 139589)
> +++ gcc/doc/invoke.texi	(working copy)
> @@ -228,7 +228,8 @@ Objective-C and Objective-C++ Dialects}.
>  @xref{Warning Options,,Options to Request or Suppress Warnings}.
>  @gccoptlist{-fsyntax-only  -pedantic  -pedantic-errors @gol
>  -w  -Wextra  -Wall  -Waddress  -Waggregate-return  -Warray-bounds @gol
> --Wno-attributes -Wc++-compat -Wc++0x-compat -Wcast-align  -Wcast-qual  @gol
> +-Wno-attributes -Wno-builtin-macro-redefined @gol
> +-Wc++-compat -Wc++0x-compat -Wcast-align  -Wcast-qual  @gol
>  -Wchar-subscripts -Wclobbered  -Wcomment @gol
>  -Wconversion  -Wcoverage-mismatch  -Wno-deprecated  @gol
>  -Wno-deprecated-declarations -Wdisabled-optimization  @gol
> @@ -3716,6 +3717,13 @@ unrecognized attributes, function attrib
>  etc.  This will not stop errors for incorrect use of supported
>  attributes.
>  
> +@item -Wno-builtin-macro-redefined
> +@opindex Wno-builtin-macro-redefined
> +@opindex Wbuiltin-macro-redefined
> +Do not warn if a certain built-in macros are redefined.  This suppresses
> +warnings for redefinition of @code{__TIMESTAMP__}, @code{__TIME__},
> +@code{__DATE__}, @code{__FILE__}, and @code{__BASE_FILE__}.
> +
>  @item -Wstrict-prototypes @r{(C and Objective-C only)}
>  @opindex Wstrict-prototypes
>  @opindex Wno-strict-prototypes
> Index: gcc/testsuite/gcc.dg/builtin-redefine.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/builtin-redefine.c	(revision 0)
> +++ gcc/testsuite/gcc.dg/builtin-redefine.c	(revision 0)
> @@ -0,0 +1,79 @@
> +/* Test -Wno-builtin-macro-redefined warnings.  */
> +
> +/* { dg-do compile } */
> +/* { dg-options "-Wno-builtin-macro-redefined -U__DATE__ -D__TIME__=X" } */
> +
> +/* Check date, time, and datestamp built-ins warnings may be suppressed.  */
> +
> +#if defined(__DATE__)
> +#error "__DATE__ is defined, but should not be (-U command line error)"
> +/* { dg-bogus "__DATE__ is defined" "" { target *-*-* } 9 } */
> +#endif
> +
> +#if __TIME__ != X
> +#error "__TIME__ is not defined as expected (-D command line error)"
> +/* { dg-bogus "__TIME__ is not defined" "" { target *-*-* } 14 } */
> +#endif
> +
> +#if !defined(__TIMESTAMP__)
> +#error "__TIMESTAMP__ is not defined (built-in macro expectation error)"
> +/* { dg-bogus "__TIMESTAMP__ is not defined" "" { target *-*-* } 19 } */
> +#endif
> +
> +
> +#undef __TIME__              /* Undefine while defined.  */
> +#undef __TIME__              /* Undefine while already undefined.  */
> +
> +#define __TIME__ "X"         /* Define while undefined.  */
> +#define __TIME__ "X"         /* Re-define while defined.  */
> +
> +#define __TIME__ "Y"         /* { dg-warning "\"__TIME__\" redefined" } */
> +/* { dg-warning "previous definition" "" { target *-*-* } 28 } */
> +
> +#undef __TIME__              /* Undefine while defined.  */
> +
> +
> +#undef __DATE__              /* Undefine while already undefined.  */
> +
> +#define __DATE__ "X"         /* Define while undefined.  */
> +#define __DATE__ "X"         /* Re-define while defined.  */
> +
> +#define __DATE__ "Y"         /* { dg-warning "\"__DATE__\" redefined" } */
> +/* { dg-warning "previous definition" "" { target *-*-* } 39 } */
> +
> +#undef __DATE__              /* Undefine while defined.  */
> +
> +
> +#define __TIMESTAMP__ "X"    /* Define while already defined.  */
> +#define __TIMESTAMP__ "X"    /* Re-define while defined.  */
> +
> +#define __TIMESTAMP__ "Y"    /* { dg-warning "\"__TIMESTAMP__\" redefined" } */
> +/* { dg-warning "previous definition" "" { target *-*-* } 48 } */
> +
> +#undef __TIMESTAMP__         /* Undefine while defined.  */
> +
> +
> +/* Check other built-ins with warnings that may be suppressed.  */
> +
> +#if !defined(__FILE__) || !defined(__BASE_FILE__)
> +#error "Expected built-in is not defined (built-in macro expectation error)"
> +/* { dg-bogus "Expected built-in is not defined" "" { target *-*-* } 59 } */
> +#endif
> +
> +#define __FILE__ "X"         /* Define while already defined.  */
> +#define __BASE_FILE__ "X"    /* Define while already defined.  */
> +
> +
> +/* Check selected built-ins not affected by warning suppression. */
> +
> +#if !defined(__LINE__) || !defined(__INCLUDE_LEVEL__) || !defined(__COUNTER__)
> +#error "Expected built-in is not defined (built-in macro expectation error)"
> +/* { dg-bogus "Expected built-in is not defined" "" { target *-*-* } 70 } */
> +#endif
> +
> +#define __LINE__ 0           /* { dg-warning "\"__LINE__\" redef" } */
> +#define __INCLUDE_LEVEL__ 0  /* { dg-warning "\"__INCLUDE_LEVEL__\" redef" } */
> +#define __COUNTER__ 0        /* { dg-warning "\"__COUNTER__\" redef" } */
> +
> +
> +int unused;  /* Silence `ISO C forbids an empty translation unit' warning.  */
> Index: gcc/c.opt
> ===================================================================
> --- gcc/c.opt	(revision 139589)
> +++ gcc/c.opt	(working copy)
> @@ -131,6 +131,10 @@ Wbad-function-cast
>  C ObjC Var(warn_bad_function_cast) Warning
>  Warn about casting functions to incompatible types
>  
> +Wbuiltin-macro-redefined
> +C ObjC C++ ObjC++ Warning
> +Warn when a built-in preprocessor macro is undefined or redefined
> +
>  Wc++-compat
>  C ObjC Var(warn_cxx_compat) Warning
>  Warn about C constructs that are not in the common subset of C and C++
> Index: gcc/c-opts.c
> ===================================================================
> --- gcc/c-opts.c	(revision 139589)
> +++ gcc/c-opts.c	(working copy)
> @@ -426,6 +426,10 @@ c_common_handle_option (size_t scode, co
>  	warn_pointer_sign = 1;
>        break;
>  
> +    case OPT_Wbuiltin_macro_redefined:
> +      cpp_opts->warn_builtin_macro_redefined = value;
> +      break;
> +
>      case OPT_Wcomment:
>      case OPT_Wcomments:
>        cpp_opts->warn_comments = value;
> Index: libcpp/macro.c
> ===================================================================
> --- libcpp/macro.c	(revision 139589)
> +++ libcpp/macro.c	(working copy)
> @@ -1392,6 +1392,10 @@ warn_of_redefinition (cpp_reader *pfile,
>    if (node->flags & NODE_WARN)
>      return true;
>  
> +  /* Suppress warnings for builtins that lack the NODE_WARN flag.  */
> +  if (node->flags & NODE_BUILTIN)
> +    return false;
> +
>    /* Redefinitions of conditional (context-sensitive) macros, on
>       the other hand, must be allowed silently.  */
>    if (node->flags & NODE_CONDITIONAL)
> Index: libcpp/include/cpplib.h
> ===================================================================
> --- libcpp/include/cpplib.h	(revision 139589)
> +++ libcpp/include/cpplib.h	(working copy)
> @@ -349,6 +349,10 @@ struct cpp_options
>       Presumably the usage is protected by the appropriate #ifdef.  */
>    unsigned char warn_variadic_macros;
>  
> +  /* Nonzero means warn about builtin macros that are redefined or
> +     explicitly undefined.  */
> +  unsigned char warn_builtin_macro_redefined;
> +
>    /* Nonzero means turn warnings into errors.  */
>    unsigned char warnings_are_errors;
>  
> Index: libcpp/init.c
> ===================================================================
> --- libcpp/init.c	(revision 139589)
> +++ libcpp/init.c	(working copy)
> @@ -163,6 +163,7 @@ cpp_create_reader (enum c_lang lang, has
>    CPP_OPTION (pfile, dollars_in_ident) = 1;
>    CPP_OPTION (pfile, warn_dollars) = 1;
>    CPP_OPTION (pfile, warn_variadic_macros) = 1;
> +  CPP_OPTION (pfile, warn_builtin_macro_redefined) = 1;
>    CPP_OPTION (pfile, warn_normalize) = normalized_C;
>  
>    /* Default CPP arithmetic to something sensible for the host for the
> @@ -303,31 +304,41 @@ cpp_destroy (cpp_reader *pfile)
>     altered through #define, and #if recognizes them as operators.  In
>     C, these are not entered into the hash table at all (but see
>     <iso646.h>).  The value is a token-type enumerator.  */
> -struct builtin
> +struct builtin_macro
>  {
> -  const uchar *name;
> -  unsigned short len;
> -  unsigned short value;
> +  const uchar *const name;
> +  const unsigned short len;
> +  const unsigned short value;
> +  const bool always_warn_if_redefined;
>  };
>  
> -#define B(n, t)    { DSC(n), t }
> -static const struct builtin builtin_array[] =
> +#define B(n, t, f)    { DSC(n), t, f }
> +static const struct builtin_macro builtin_array[] =
>  {
> -  B("__TIMESTAMP__",	 BT_TIMESTAMP),
> -  B("__TIME__",		 BT_TIME),
> -  B("__DATE__",		 BT_DATE),
> -  B("__FILE__",		 BT_FILE),
> -  B("__BASE_FILE__",	 BT_BASE_FILE),
> -  B("__LINE__",		 BT_SPECLINE),
> -  B("__INCLUDE_LEVEL__", BT_INCLUDE_LEVEL),
> -  B("__COUNTER__",	 BT_COUNTER),
> +  B("__TIMESTAMP__",	 BT_TIMESTAMP,     false),
> +  B("__TIME__",		 BT_TIME,          false),
> +  B("__DATE__",		 BT_DATE,          false),
> +  B("__FILE__",		 BT_FILE,          false),
> +  B("__BASE_FILE__",	 BT_BASE_FILE,     false),
> +  B("__LINE__",		 BT_SPECLINE,      true),
> +  B("__INCLUDE_LEVEL__", BT_INCLUDE_LEVEL, true),
> +  B("__COUNTER__",	 BT_COUNTER,       true),
>    /* Keep builtins not used for -traditional-cpp at the end, and
>       update init_builtins() if any more are added.  */
> -  B("_Pragma",		 BT_PRAGMA),
> -  B("__STDC__",		 BT_STDC),
> +  B("_Pragma",		 BT_PRAGMA,        true),
> +  B("__STDC__",		 BT_STDC,          true),
> +};
> +#undef B
> +
> +struct builtin_operator
> +{
> +  const uchar *const name;
> +  const unsigned short len;
> +  const unsigned short value;
>  };
>  
> -static const struct builtin operator_array[] =
> +#define B(n, t)    { DSC(n), t }
> +static const struct builtin_operator operator_array[] =
>  {
>    B("and",	CPP_AND_AND),
>    B("and_eq",	CPP_AND_EQ),
> @@ -347,7 +358,7 @@ static const struct builtin operator_arr
>  static void
>  mark_named_operators (cpp_reader *pfile)
>  {
> -  const struct builtin *b;
> +  const struct builtin_operator *b;
>  
>    for (b = operator_array;
>         b < (operator_array + ARRAY_SIZE (operator_array));
> @@ -363,7 +374,7 @@ mark_named_operators (cpp_reader *pfile)
>  void
>  cpp_init_special_builtins (cpp_reader *pfile)
>  {
> -  const struct builtin *b;
> +  const struct builtin_macro *b;
>    size_t n = ARRAY_SIZE (builtin_array);
>  
>    if (CPP_OPTION (pfile, traditional))
> @@ -376,7 +387,10 @@ cpp_init_special_builtins (cpp_reader *p
>      {
>        cpp_hashnode *hp = cpp_lookup (pfile, b->name, b->len);
>        hp->type = NT_MACRO;
> -      hp->flags |= NODE_BUILTIN | NODE_WARN;
> +      hp->flags |= NODE_BUILTIN;
> +      if (b->always_warn_if_redefined
> +          || CPP_OPTION (pfile, warn_builtin_macro_redefined))
> +	hp->flags |= NODE_WARN;
>        hp->value.builtin = (enum builtin_type) b->value;
>      }
>  }
>   


-- 
Google UK Limited | Registered Office: Belgrave House, 76 Buckingham
Palace Road, London SW1W 9TQ | Registered in England Number: 3977902

Re: [PATCH][RFC] -Wno-... option to suppress builtin macro redefined warnings

[Tom Tromey]

>>>>> "Simon" == Simon Baldwin <simonb@google.com> writes:

Simon> ...so, attached below is a re-draft of this patch to bifurcate
Simon> built-in macros so that only TIMESTAMP, TIME, DATE, FILE, and
Simon> FILE_BASE are affected by -Wno-builtin-macro-redefined, and other
Simon> built-in macros always warn if redefined.

Simon> Please take another look when time permits, to see if you like this
Simon> version better than the first.  And thanks again for the feedback.

I can only approve the libcpp parts -- those are ok.

thanks,
Tom

Re: [PATCH][RFC] -Wno-... option to suppress builtin macro redefined warnings

[Simon Baldwin]

Tom Tromey wrote:
>>>>>> "Simon" == Simon Baldwin <simonb@google.com> writes:
>>>>>>             
>
> Simon> ...so, attached below is a re-draft of this patch to bifurcate
> Simon> built-in macros so that only TIMESTAMP, TIME, DATE, FILE, and
> Simon> FILE_BASE are affected by -Wno-builtin-macro-redefined, and other
> Simon> built-in macros always warn if redefined.
>
> Simon> Please take another look when time permits, to see if you like this
> Simon> version better than the first.  And thanks again for the feedback.
>
> I can only approve the libcpp parts -- those are ok.

Thanks for the nod, much appreciated.

Any ideas of a victim to approve the tiny c.opt and c-opts.c parts to 
control this?  Volunteers haven't exactly been leaping forwards.  Thanks.

-- 
Google UK Limited | Registered Office: Belgrave House, 76 Buckingham
Palace Road, London SW1W 9TQ | Registered in England Number: 3977902

Re: [PATCH][RFC] -Wno-... option to suppress builtin macro redefined warnings

[Ian Lance Taylor]

Tom Tromey <tromey@redhat.com> writes:

>>>>>> "Simon" == Simon Baldwin <simonb@google.com> writes:
>
> Simon> ...so, attached below is a re-draft of this patch to bifurcate
> Simon> built-in macros so that only TIMESTAMP, TIME, DATE, FILE, and
> Simon> FILE_BASE are affected by -Wno-builtin-macro-redefined, and other
> Simon> built-in macros always warn if redefined.
>
> Simon> Please take another look when time permits, to see if you like this
> Simon> version better than the first.  And thanks again for the feedback.
>
> I can only approve the libcpp parts -- those are ok.

The rest of the patch is OK.

Thanks.

Ian

Re: [PATCH][RFC] -Wno-... option to suppress builtin macro redefined warnings

[Simon Baldwin]

> Ian Lance Taylor wrote:
>> > +@item -Wno-builtin-macro-redefined
>> > +@opindex Wno-builtin-macro-redefined
>> > +@opindex Wbuiltin-macro-redefined
>> > +Do not warn if a certain built-in macros are redefined.  This suppresses
>> > +warnings for redefinition of @code{__TIMESTAMP__}, @code{__TIME__},
>> > +@code{__DATE__}, @code{__FILE__}, and @code{__BASE_FILE__}.
>>     
>
> s/if a certain/if certain/
>   

Updated, thanks.

> Tom Tromey <tromey@redhat.com> writes:
>
>   
>>>>>>> "Simon" == Simon Baldwin <simonb@google.com> writes:
>>>>>>>               
>> Simon> ...so, attached below is a re-draft of this patch to bifurcate
>> Simon> built-in macros so that only TIMESTAMP, TIME, DATE, FILE, and
>> Simon> FILE_BASE are affected by -Wno-builtin-macro-redefined, and other
>> Simon> built-in macros always warn if redefined.
>>
>> Simon> Please take another look when time permits, to see if you like this
>> Simon> version better than the first.  And thanks again for the feedback.
>>
>> I can only approve the libcpp parts -- those are ok.
>>     
>
> The rest of the patch is OK.

Thanks.  Committed revision 140461.

-- 
Google UK Limited | Registered Office: Belgrave House, 76 Buckingham
Palace Road, London SW1W 9TQ | Registered in England Number: 3977902

Re: [PATCH][RFC] -Wno-... option to suppress builtin macro redefined warnings

[Ian Lance Taylor]

Simon Baldwin <simonb@google.com> writes:

> +@item -Wno-builtin-macro-redefined
> +@opindex Wno-builtin-macro-redefined
> +@opindex Wbuiltin-macro-redefined
> +Do not warn if a certain built-in macros are redefined.  This suppresses
> +warnings for redefinition of @code{__TIMESTAMP__}, @code{__TIME__},
> +@code{__DATE__}, @code{__FILE__}, and @code{__BASE_FILE__}.

s/if a certain/if certain/

Ian

Re: [PATCH][RFC] -Wno-... option to suppress builtin macro redefined warnings

[Simon Baldwin]

Simon Baldwin wrote:
> Simon Baldwin wrote:
>> Tom Tromey wrote:
>>>>>>>> "Simon" == Simon Baldwin <simonb@google.com> writes:
>>>>>>>>             
>>>
>>> ...
>>> However, it seems to me that we would want to allow redefinition of
>>> some macros (__TIME__ et al) but not others (e.g., __LINE__).
>>>
>>> So, how about splitting builtin_array into two pieces (and just FYI,
>>> there's a comment above referring to "two tables" that should be
>>> changed) and then unconditionally setting NODE_WARN for one table but
>>> not the other?  Or, just adding a special case in the builtin
>>> definition loop for the BT_* constants we care to allow.
>>>   
>>
>> Thank you for the note.
>>
>> I guess that in general it just seems more, um, seamless to either 
>> allow or disallow any builtin macro to be redefined.  It means that 
>> there's no "is it redefinable or not?" decision to be made when new 
>> builtins are added.  It also neatly sidesteps the issue of then 
>> having to document which builtins are redefinable and which aren't 
>> (also no update to this doc should new builtins be added), or having 
>> to deal with requests to move builtins between redefinable/fixed 
>> groups.  In other words, the lowest impact on future code maintainers 
>> consistent with low impact on current code.
>>
>> Granted, somebody could well redefine __LINE__ or similar and make a 
>> mess of compilation, but there are of course plenty of other ways to 
>> make a mess of compilation with other gcc flags.  
>> -Wno-builtin-macro-redefined is aimed at the control-freak automated 
>> build system rather than the casual user, so it seemed okay, to me 
>> anyway, for it to bear closer resemblance to a chainsaw than to a 
>> scalpel.
>>
>> That said, I'm not wildly opposed to creating two "classes" of 
>> builtin.  It just seems like doing so might sow slightly more 
>> confusion than it prevents.
>
> Tom, any further thoughts on this?
>
> It's certainly not hard to split built-in macros into two tiers, those 
> where redefinition warning can be suppressed by providing 
> -Wno-builtin-macro-redefined, and those where it can't.  However, 
> since it's all just about suppressing a warning, it may be that one 
> policy to cover all of them will suffice, and be simpler to manage and 
> maintain.
>
> Thanks.
>


No response from Tom to date.

Would anyone else be prepared to pick this up in the interim?  Thanks.

-- 
Google UK Limited | Registered Office: Belgrave House, 76 Buckingham
Palace Road, London SW1W 9TQ | Registered in England Number: 3977902