From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22864 invoked by alias); 2 Sep 2008 11:29:44 -0000 Received: (qmail 22854 invoked by uid 22791); 2 Sep 2008 11:29:43 -0000 X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (216.239.33.17) by sourceware.org (qpsmtpd/0.31) with ESMTP; Tue, 02 Sep 2008 11:29:05 +0000 Received: from wpaz17.hot.corp.google.com (wpaz17.hot.corp.google.com [172.24.198.81]) by smtp-out3.google.com with ESMTP id m82BSwxJ013596; Tue, 2 Sep 2008 12:28:58 +0100 Received: from [172.16.9.2] (simonb.lon.corp.google.com [172.16.9.2]) by wpaz17.hot.corp.google.com with ESMTP id m82BStPh002889; Tue, 2 Sep 2008 04:28:56 -0700 Message-ID: <48BD2377.6050009@google.com> Date: Tue, 02 Sep 2008 11:29:00 -0000 From: Simon Baldwin User-Agent: Thunderbird 1.5.0.14ubu (X11/20080502) MIME-Version: 1.0 To: Tom Tromey , gcc-patches@gcc.gnu.org Subject: Re: [PATCH][RFC] -Wno-... option to suppress builtin macro redefined warnings References: <20080730140920.0F9CF3FE1B9@localhost> <489C728B.4040505@google.com> <48A5B2C6.1090600@google.com> <48AEE7A7.9000509@google.com> <48B42327.1090204@google.com> In-Reply-To: <48B42327.1090204@google.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2008-09/txt/msg00076.txt.bz2 Ping -- Tom? Anyone? Thanks. --S Simon Baldwin wrote: > Tom Tromey wrote: >>>>>>> "Simon" == Simon Baldwin 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 > > * 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 > > * 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 > > * 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 > ). 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