[libcpp] Use CPP() for Wbuiltin-macro-redefined
diff mbox

Message ID CAESRpQDMehzs99iOZgeysLQ6s+OTd_1EhBbKPFH=8V4fJgdqDA@mail.gmail.com
State New
Headers show

Commit Message

Manuel López-Ibáñez Aug. 23, 2014, 10:35 a.m. UTC
Hi,

The problem here is that builtin macros are marked with NO_WARN flag
depending on the value of Wbuiltin-macro-redefined, but this only
happens during libcpp initialization, so it doesn't work for #pragmas.
It neither works when using CPP(), since the default in the c.opt file
is set before libcpp initialization, overriding the default of libcpp.

The problem with #pragma is fixed in this patch by making NO_WARN
independent of the value of Wbuiltin-macro-redefined.

The problem with overriding the default in libcpp is fixed by setting
Init(1) in c.opt to match the default. However, it is too easy to
forget the Init(). It would be better if no Init() meant "use the
default of libcpp". This would require calling another generated
function just after initializing cpp_opts to set the defaults in
global_opts. Would that be ok? Or should I just set Init() explicitly
for every CPP()?

In any case, bootstrapped & regression tested x86_64-linux-gnu.

libcpp/ChangeLog:

2014-08-23  Manuel López-Ibáñez  <manu@gcc.gnu.org>

    * macro.c (warn_of_redefinition): Suppress warnings for builtins
    that lack the NODE_WARN flag, unless Wbuiltin-macro-redefined.
    (_cpp_create_definition): Use Wbuiltin-macro-redefined for
    builtins that lack the NODE_WARN flag.
    * directives.c (do_undef): Likewise.
    * init.c (cpp_init_special_builtins): Do not change flags
    depending on Wbuiltin-macro-redefined.


gcc/c-family/ChangeLog:

2014-08-23  Manuel López-Ibáñez  <manu@gcc.gnu.org>

    * c.opt (Wbuiltin-macro-redefined): Use CPP, Var and Init.
    * c-opts.c (c_common_handle_option): Do not handle here.

Comments

Joseph Myers Aug. 27, 2014, 5:10 p.m. UTC | #1
On Sat, 23 Aug 2014, Manuel López-Ibáñez wrote:

> The problem with overriding the default in libcpp is fixed by setting
> Init(1) in c.opt to match the default. However, it is too easy to
> forget the Init(). It would be better if no Init() meant "use the
> default of libcpp". This would require calling another generated
> function just after initializing cpp_opts to set the defaults in
> global_opts. Would that be ok? Or should I just set Init() explicitly
> for every CPP()?

I think I prefer having the defaults explicit in the .opt file (generally, 
prefer having information about option settings and how options relate to 
each other as data in the .opt files over having it in code).  So that 
means setting Init() explicitly for such options.

> libcpp/ChangeLog:
> 
> 2014-08-23  Manuel López-Ibáñez  <manu@gcc.gnu.org>
> 
>     * macro.c (warn_of_redefinition): Suppress warnings for builtins
>     that lack the NODE_WARN flag, unless Wbuiltin-macro-redefined.
>     (_cpp_create_definition): Use Wbuiltin-macro-redefined for
>     builtins that lack the NODE_WARN flag.
>     * directives.c (do_undef): Likewise.
>     * init.c (cpp_init_special_builtins): Do not change flags
>     depending on Wbuiltin-macro-redefined.
> 
> 
> gcc/c-family/ChangeLog:
> 
> 2014-08-23  Manuel López-Ibáñez  <manu@gcc.gnu.org>
> 
>     * c.opt (Wbuiltin-macro-redefined): Use CPP, Var and Init.
>     * c-opts.c (c_common_handle_option): Do not handle here.

OK.

Patch
diff mbox

Index: gcc/c-family/c.opt
===================================================================
--- gcc/c-family/c.opt	(revision 214396)
+++ gcc/c-family/c.opt	(working copy)
@@ -290,11 +290,11 @@  Warn about casting functions to incompat
 Wbool-compare
 C ObjC C++ ObjC++ Var(warn_bool_compare) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
 Warn about boolean expression compared with an integer value different from true/false
 
 Wbuiltin-macro-redefined
-C ObjC C++ ObjC++ Warning
+C ObjC C++ ObjC++ CPP(warn_builtin_macro_redefined) Var(cpp_warn_builtin_macro_redefined) Init(1) Warning
 Warn when a built-in preprocessor macro is undefined or redefined
 
 Wc90-c99-compat
 C ObjC Var(warn_c90_c99_compat) Init(-1) Warning
 Warn about features not present in ISO C90, but present in ISO C99
Index: gcc/c-family/c-opts.c
===================================================================
--- gcc/c-family/c-opts.c	(revision 214396)
+++ gcc/c-family/c-opts.c	(working copy)
@@ -383,14 +383,10 @@  c_common_handle_option (size_t scode, co
 
       cpp_opts->warn_trigraphs = value;
       cpp_opts->warn_num_sign_change = value;
       break;
 
-    case OPT_Wbuiltin_macro_redefined:
-      cpp_opts->warn_builtin_macro_redefined = value;
-      break;
-
     case OPT_Wc___compat:
       cpp_opts->warn_cxx_operator_names = value;
       break;
 
     case OPT_Wdeprecated:
Index: libcpp/macro.c
===================================================================
--- libcpp/macro.c	(revision 214396)
+++ libcpp/macro.c	(working copy)
@@ -2697,17 +2697,16 @@  warn_of_redefinition (cpp_reader *pfile,
 
   /* Some redefinitions need to be warned about regardless.  */
   if (node->flags & NODE_WARN)
     return true;
 
-  /* Suppress warnings for builtins that lack the NODE_WARN flag.  */
-  if (node->flags & NODE_BUILTIN)
-    {
-      if (!pfile->cb.user_builtin_macro
-	  || !pfile->cb.user_builtin_macro (pfile, node))
-	return false;
-    }
+  /* Suppress warnings for builtins that lack the NODE_WARN flag,
+     unless Wbuiltin-macro-redefined.  */
+  if (node->flags & NODE_BUILTIN
+      && (!pfile->cb.user_builtin_macro
+	  || !pfile->cb.user_builtin_macro (pfile, node)))
+    return CPP_OPTION (pfile, warn_builtin_macro_redefined);
 
   /* Redefinitions of conditional (context-sensitive) macros, on
      the other hand, must be allowed silently.  */
   if (node->flags & NODE_CONDITIONAL)
     return false;
@@ -3179,18 +3178,18 @@  _cpp_create_definition (cpp_reader *pfil
       if (CPP_OPTION (pfile, warn_unused_macros))
 	_cpp_warn_if_unused_macro (pfile, node, NULL);
 
       if (warn_of_redefinition (pfile, node, macro))
 	{
-          const int reason = (node->flags & NODE_BUILTIN)
+          const int reason = ((node->flags & NODE_BUILTIN)
+			      && !(node->flags & NODE_WARN))
                              ? CPP_W_BUILTIN_MACRO_REDEFINED : CPP_W_NONE;
-	  bool warned;
 
-	  warned = cpp_pedwarning_with_line (pfile, reason,
-					     pfile->directive_line, 0,
-					     "\"%s\" redefined",
-                                             NODE_NAME (node));
+	  bool warned = 
+	    cpp_pedwarning_with_line (pfile, reason,
+				      pfile->directive_line, 0,
+				      "\"%s\" redefined", NODE_NAME (node));
 
 	  if (warned && node->type == NT_MACRO && !(node->flags & NODE_BUILTIN))
 	    cpp_error_with_line (pfile, CPP_DL_NOTE,
 				 node->value.macro->line, 0,
 			 "this is the location of the previous definition");
Index: libcpp/directives.c
===================================================================
--- libcpp/directives.c	(revision 214396)
+++ libcpp/directives.c	(working copy)
@@ -608,10 +608,15 @@  do_undef (cpp_reader *pfile)
       if (node->type == NT_MACRO)
 	{
 	  if (node->flags & NODE_WARN)
 	    cpp_error (pfile, CPP_DL_WARNING,
 		       "undefining \"%s\"", NODE_NAME (node));
+	  else if ((node->flags & NODE_BUILTIN)
+		   && CPP_OPTION (pfile, warn_builtin_macro_redefined))
+	    cpp_warning_with_line (pfile, CPP_W_BUILTIN_MACRO_REDEFINED,
+				   pfile->directive_line, 0,
+				   "undefining \"%s\"", NODE_NAME (node));
 
 	  if (CPP_OPTION (pfile, warn_unused_macros))
 	    _cpp_warn_if_unused_macro (pfile, node, NULL);
 
 	  _cpp_free_definition (node);
Index: libcpp/init.c
===================================================================
--- libcpp/init.c	(revision 214396)
+++ libcpp/init.c	(working copy)
@@ -465,12 +465,11 @@  cpp_init_special_builtins (cpp_reader *p
   for (b = builtin_array; b < builtin_array + n; b++)
     {
       cpp_hashnode *hp = cpp_lookup (pfile, b->name, b->len);
       hp->type = NT_MACRO;
       hp->flags |= NODE_BUILTIN;
-      if (b->always_warn_if_redefined
-          || CPP_OPTION (pfile, warn_builtin_macro_redefined))
+      if (b->always_warn_if_redefined)
 	hp->flags |= NODE_WARN;
       hp->value.builtin = (enum cpp_builtin_type) b->value;
     }
 }