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;
}
}