[PATCH][RFC] -Wno-... option to suppress builtin macro redefined warnings
Simon Baldwin
simonb@google.com
Tue Sep 2 11:29:00 GMT 2008
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
More information about the Gcc-patches
mailing list