This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PR c/25880 improve message of warning for discarding qualifiers


I sometimes forget that humans cannot read minds. Now with patch.

Manuel.

On 24 May 2010 11:38, Manuel López-Ibáñez <lopezibanez@gmail.com> wrote:
> Before updating testcases, I would like to know if the current patch is ok.
>
> Bootstrapped and regression tested on x86_64-linux-gnu. Failing
> testcases are due to the change of wording.
>
> Cheers,
>
> Manuel.
>
>
> 2010-05-24 ?Manuel López-Ibáñez ?<manu@gcc.gnu.org>
>
> ? ? ? ?PR c/25880
> ? ? ? ?* c-objc-common.c (c_tree_printer): Handle %V, %v and %#v.
> ? ? ? ?* c-format.c (gcc_diag_flag_specs): Add hash.
> ? ? ? ?(gcc_cxxdiag_flag_specs): Use gcc_diag_flag_specs directly.
> ? ? ? ?(gcc_tdiag_char_table,gcc_cdiag_char_table): Handle %V and %v.
> ? ? ? ?* c-pretty-print.c (pp_c_cv_qualifier): Rename as
> ? ? ? ?pp_c_cv_qualifiers. Handle qualifiers spelling here.
> ? ? ? ?(pp_c_type_qualifier_list): Call the function above.
> ? ? ? ?* c-pretty-print.h (pp_c_cv_qualifiers): Declare.
> ? ? ? ?* c-typeck.c (handle_warn_cast_qual): Print qualifiers.
> ? ? ? ?(WARN_FOR_QUALIFIERS): New macro.
> ? ? ? ?(convert_for_assignment): Use it.
>
Index: gcc/c-objc-common.c
===================================================================
--- gcc/c-objc-common.c	(revision 159762)
+++ gcc/c-objc-common.c	(working copy)
@@ -77,13 +77,14 @@ c_objc_common_init (void)
    is as follows:
    %D: a general decl,
    %E: an identifier or expression,
    %F: a function declaration,
    %T: a type.
+   %V: a list of type qualifiers from a tree.
+   %v: an explicit list of type qualifiers
+   %#v: an explicit list of type qualifiers of a function type.
 
-   These format specifiers form a subset of the format specifiers set used
-   by the C++ front-end.
    Please notice when called, the `%' part was already skipped by the
    diagnostic machinery.  */
 static bool
 c_tree_printer (pretty_printer *pp, text_info *text, const char *spec,
 		int precision, bool wide, bool set_locus, bool hash)
@@ -91,23 +92,25 @@ c_tree_printer (pretty_printer *pp, text
   tree t;
   tree name;
   c_pretty_printer *cpp = (c_pretty_printer *) pp;
   pp->padding = pp_none;
 
-  if (precision != 0 || wide || hash)
+  if (precision != 0 || wide)
     return false;
 
   if (*spec == 'K')
     {
       percent_K_format (text);
       return true;
     }
 
-  t = va_arg (*text->args_ptr, tree);
-
-  if (set_locus && text->locus)
-    *text->locus = DECL_SOURCE_LOCATION (t);
+  if (*spec != 'v')
+    {
+      t = va_arg (*text->args_ptr, tree);
+      if (set_locus && text->locus)
+	*text->locus = DECL_SOURCE_LOCATION (t);
+    }
 
   switch (*spec)
     {
     case 'D':
       if (DECL_DEBUG_EXPR_IS_FROM (t) && DECL_DEBUG_EXPR (t))
@@ -153,10 +156,18 @@ c_tree_printer (pretty_printer *pp, text
 	pp_identifier (cpp, IDENTIFIER_POINTER (t));
       else
 	pp_expression (cpp, t);
       return true;
 
+    case 'V':
+      pp_c_type_qualifier_list (cpp, t);
+      return true;
+
+    case 'v':
+      pp_c_cv_qualifiers (cpp, va_arg (*text->args_ptr, int), hash);
+      return true;
+
     default:
       return false;
     }
 
   pp_string (cpp, _("({anonymous})"));
Index: gcc/c-format.c
===================================================================
--- gcc/c-format.c	(revision 159762)
+++ gcc/c-format.c	(working copy)
@@ -418,28 +418,20 @@ static const format_flag_pair gcc_gfc_fl
 };
 
 static const format_flag_spec gcc_diag_flag_specs[] =
 {
   { '+',  0, 0, N_("'+' flag"),        N_("the '+' printf flag"),              STD_C89 },
+  { '#',  0, 0, N_("'#' flag"),        N_("the '#' printf flag"),              STD_C89 },
   { 'q',  0, 0, N_("'q' flag"),        N_("the 'q' diagnostic flag"),          STD_C89 },
   { 'p',  0, 0, N_("precision"),       N_("precision in printf format"),       STD_C89 },
   { 'L',  0, 0, N_("length modifier"), N_("length modifier in printf format"), STD_C89 },
   { 0, 0, 0, NULL, NULL, STD_C89 }
 };
 
 #define gcc_tdiag_flag_specs gcc_diag_flag_specs
 #define gcc_cdiag_flag_specs gcc_diag_flag_specs
-
-static const format_flag_spec gcc_cxxdiag_flag_specs[] =
-{
-  { '+',  0, 0, N_("'+' flag"),        N_("the '+' printf flag"),              STD_C89 },
-  { '#',  0, 0, N_("'#' flag"),        N_("the '#' printf flag"),              STD_C89 },
-  { 'q',  0, 0, N_("'q' flag"),        N_("the 'q' diagnostic flag"),          STD_C89 },
-  { 'p',  0, 0, N_("precision"),       N_("precision in printf format"),       STD_C89 },
-  { 'L',  0, 0, N_("length modifier"), N_("length modifier in printf format"), STD_C89 },
-  { 0, 0, 0, NULL, NULL, STD_C89 }
-};
+#define gcc_cxxdiag_flag_specs gcc_diag_flag_specs
 
 static const format_flag_spec scanf_flag_specs[] =
 {
   { '*',  0, 0, N_("assignment suppression"), N_("the assignment suppression scanf feature"), STD_C89 },
   { 'a',  0, 0, N_("'a' flag"),               N_("the 'a' scanf flag"),                       STD_EXT },
@@ -582,11 +574,13 @@ static const format_char_info gcc_tdiag_
   { "p",   1, STD_C89, { T89_V,   BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN  }, "q",  "c",  NULL },
 
   /* Custom conversion specifiers.  */
 
   /* These will require a "tree" at runtime.  */
-  { "DFKTE", 0, STD_C89, { T89_V,   BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN  }, "q+", "",   NULL },
+  { "DFKTEV", 0, STD_C89, { T89_V,   BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN  }, "q+", "",   NULL },
+
+  { "v", 0,STD_C89, { T89_I,   BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN  }, "q#",  "",   NULL },
 
   { "<>'", 0, STD_C89, NOARGUMENTS, "",      "",   NULL },
   { "m",   0, STD_C89, NOARGUMENTS, "q",     "",   NULL },
   { NULL,  0, STD_C89, NOLENGTHS, NULL, NULL, NULL }
 };
@@ -602,11 +596,13 @@ static const format_char_info gcc_cdiag_
   { "p",   1, STD_C89, { T89_V,   BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN  }, "q",  "c",  NULL },
 
   /* Custom conversion specifiers.  */
 
   /* These will require a "tree" at runtime.  */
-  { "DEFKT", 0, STD_C89, { T89_V,   BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN  }, "q+", "",   NULL },
+  { "DEFKTV", 0, STD_C89, { T89_V,   BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN  }, "q+", "",   NULL },
+
+  { "v", 0,STD_C89, { T89_I,   BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN  }, "q#",  "",   NULL },
 
   { "<>'", 0, STD_C89, NOARGUMENTS, "",      "",   NULL },
   { "m",   0, STD_C89, NOARGUMENTS, "q",     "",   NULL },
   { NULL,  0, STD_C89, NOLENGTHS, NULL, NULL, NULL }
 };
@@ -719,23 +715,23 @@ static const format_kind_info format_typ
     asm_fprintf_flag_specs, asm_fprintf_flag_pairs,
     FMT_FLAG_ARG_CONVERT|FMT_FLAG_EMPTY_PREC_OK,
     'w', 0, 'p', 0, 'L', 0,
     NULL, NULL
   },
-  { "gcc_diag",   gcc_diag_length_specs,  gcc_diag_char_table, "q+", NULL,
+  { "gcc_diag",   gcc_diag_length_specs,  gcc_diag_char_table, "q+#", NULL,
     gcc_diag_flag_specs, gcc_diag_flag_pairs,
     FMT_FLAG_ARG_CONVERT,
     0, 0, 'p', 0, 'L', 0,
     NULL, &integer_type_node
   },
-  { "gcc_tdiag",   gcc_tdiag_length_specs,  gcc_tdiag_char_table, "q+", NULL,
+  { "gcc_tdiag",   gcc_tdiag_length_specs,  gcc_tdiag_char_table, "q+#", NULL,
     gcc_tdiag_flag_specs, gcc_tdiag_flag_pairs,
     FMT_FLAG_ARG_CONVERT,
     0, 0, 'p', 0, 'L', 0,
     NULL, &integer_type_node
   },
-  { "gcc_cdiag",   gcc_cdiag_length_specs,  gcc_cdiag_char_table, "q+", NULL,
+  { "gcc_cdiag",   gcc_cdiag_length_specs,  gcc_cdiag_char_table, "q+#", NULL,
     gcc_cdiag_flag_specs, gcc_cdiag_flag_pairs,
     FMT_FLAG_ARG_CONVERT,
     0, 0, 'p', 0, 'L', 0,
     NULL, &integer_type_node
   },
Index: gcc/c-pretty-print.c
===================================================================
--- gcc/c-pretty-print.c	(revision 159762)
+++ gcc/c-pretty-print.c	(working copy)
@@ -169,22 +169,47 @@ pp_c_exclamation (c_pretty_printer *pp)
 {
   pp_exclamation (pp);
   pp_base (pp)->padding = pp_none;
 }
 
-/* Print out the external representation of CV-QUALIFIER.  */
+/* Print out the external representation of QUALIFIERS.  */
 
-static void
-pp_c_cv_qualifier (c_pretty_printer *pp, const char *cv)
+void
+pp_c_cv_qualifiers (c_pretty_printer *pp, int qualifiers, bool func_type)
 {
   const char *p = pp_last_position_in_text (pp);
+  bool previous = false;
+
+  if (!qualifiers)
+    return;
+
   /* The C programming language does not have references, but it is much
      simpler to handle those here rather than going through the same
      logic in the C++ pretty-printer.  */
   if (p != NULL && (*p == '*' || *p == '&'))
     pp_c_whitespace (pp);
-  pp_c_ws_string (pp, cv);
+
+  if (qualifiers & TYPE_QUAL_CONST)
+    {
+      pp_c_ws_string (pp, "const");
+      previous = true;
+    }
+
+  if (qualifiers & TYPE_QUAL_VOLATILE)
+    {
+      if (previous)
+        pp_c_whitespace (pp);
+      pp_c_ws_string (pp, func_type ? "__atribute__((noreturn))" : "volatile");
+      previous = true;
+    }
+
+  if (qualifiers & TYPE_QUAL_RESTRICT)
+    {
+      if (previous)
+        pp_c_whitespace (pp);
+      pp_c_ws_string (pp, flag_isoc99 ? "restrict" : "__restrict__");
+    }
 }
 
 /* Pretty-print T using the type-cast notation '( type-name )'.  */
 
 static void
@@ -241,16 +266,12 @@ pp_c_type_qualifier_list (c_pretty_print
 
   if (!TYPE_P (t))
     t = TREE_TYPE (t);
 
   qualifiers = TYPE_QUALS (t);
-  if (qualifiers & TYPE_QUAL_CONST)
-    pp_c_cv_qualifier (pp, "const");
-  if (qualifiers & TYPE_QUAL_VOLATILE)
-    pp_c_cv_qualifier (pp, "volatile");
-  if (qualifiers & TYPE_QUAL_RESTRICT)
-    pp_c_cv_qualifier (pp, flag_isoc99 ? "restrict" : "__restrict__");
+  pp_c_cv_qualifiers (pp, qualifiers,
+		      TREE_CODE (t) == FUNCTION_TYPE);
 
   if (!ADDR_SPACE_GENERIC_P (TYPE_ADDR_SPACE (t)))
     {
       const char *as = c_addr_space_name (TYPE_ADDR_SPACE (t));
       pp_c_identifier (pp, as);
Index: gcc/c-pretty-print.h
===================================================================
--- gcc/c-pretty-print.h	(revision 159762)
+++ gcc/c-pretty-print.h	(working copy)
@@ -174,10 +174,11 @@ void pp_c_space_for_pointer_operator (c_
 
 /* Declarations.  */
 void pp_c_tree_decl_identifier (c_pretty_printer *, tree);
 void pp_c_function_definition (c_pretty_printer *, tree);
 void pp_c_attributes (c_pretty_printer *, tree);
+void pp_c_cv_qualifiers (c_pretty_printer *pp, int qualifiers, bool func_type);
 void pp_c_type_qualifier_list (c_pretty_printer *, tree);
 void pp_c_parameter_type_list (c_pretty_printer *, tree);
 void pp_c_declaration (c_pretty_printer *, tree);
 void pp_c_declaration_specifiers (c_pretty_printer *, tree);
 void pp_c_declarator (c_pretty_printer *, tree);
Index: gcc/c-typeck.c
===================================================================
--- gcc/c-typeck.c	(revision 159762)
+++ gcc/c-typeck.c	(working copy)
@@ -4379,17 +4379,19 @@ handle_warn_cast_qual (tree type, tree o
     }
   while (TREE_CODE (in_type) == POINTER_TYPE
 	 && TREE_CODE (in_otype) == POINTER_TYPE);
 
   if (added)
-    warning (OPT_Wcast_qual, "cast adds new qualifiers to function type");
+    warning (OPT_Wcast_qual, "cast adds %q#v qualifier to function type",
+	     added);
 
   if (discarded)
     /* There are qualifiers present in IN_OTYPE that are not present
        in IN_TYPE.  */
     warning (OPT_Wcast_qual,
-	     "cast discards qualifiers from pointer target type");
+	     "cast discards %q#v qualifier from pointer target type",
+	     discarded);
 
   if (added || discarded)
     return;
 
   /* A cast from **T to const **T is unsafe, because it can cause a
@@ -4418,13 +4420,14 @@ handle_warn_cast_qual (tree type, tree o
       in_type = TREE_TYPE (in_type);
       in_otype = TREE_TYPE (in_otype);
       if ((TYPE_QUALS (in_type) &~ TYPE_QUALS (in_otype)) != 0
 	  && !is_const)
 	{
+	  int added = TYPE_QUALS (in_type) &~ TYPE_QUALS (in_otype);
 	  warning (OPT_Wcast_qual,
-		   ("new qualifiers in middle of multi-level non-const cast "
-		    "are unsafe"));
+		   ("new %qv qualifier in middle of multi-level non-const cast "
+		    "is unsafe"), added);
 	  break;
 	}
       if (is_const)
 	is_const = TYPE_READONLY (in_type);
     }
@@ -4946,10 +4949,40 @@ convert_for_assignment (location_t locat
       default:                                                           \
         gcc_unreachable ();                                              \
       }                                                                  \
   } while (0)
 
+  /* This macro is used to emit diagnostics to ensure that all format
+     strings are complete sentences, visible to gettext and checked at
+     compile time.  It is the same as WARN_FOR_ASSIGNMENT but with an
+     extra parameter to enumerate qualifiers.  */
+
+#define WARN_FOR_QUALIFIERS(LOCATION, OPT, AR, AS, IN, RE, QUALS)        \
+  do {                                                                   \
+    switch (errtype)                                                     \
+      {                                                                  \
+      case ic_argpass:                                                   \
+        if (pedwarn (LOCATION, OPT, AR, parmnum, rname, QUALS))          \
+          inform ((fundecl && !DECL_IS_BUILTIN (fundecl))	         \
+	      	  ? DECL_SOURCE_LOCATION (fundecl) : LOCATION,		 \
+                  "expected %qT but argument is of type %qT",            \
+                  type, rhstype);                                        \
+        break;                                                           \
+      case ic_assign:                                                    \
+        pedwarn (LOCATION, OPT, AS, QUALS);                          \
+        break;                                                           \
+      case ic_init:                                                      \
+        pedwarn (LOCATION, OPT, IN, QUALS);                          \
+        break;                                                           \
+      case ic_return:                                                    \
+        pedwarn (LOCATION, OPT, RE, QUALS);                        	 \
+        break;                                                           \
+      default:                                                           \
+        gcc_unreachable ();                                              \
+      }                                                                  \
+  } while (0)
+
   if (TREE_CODE (rhs) == EXCESS_PRECISION_EXPR)
     rhs = TREE_OPERAND (rhs, 0);
 
   rhstype = TREE_TYPE (rhs);
   coder = TREE_CODE (rhstype);
@@ -5153,34 +5186,36 @@ convert_for_assignment (location_t locat
 		     certain things, it is okay to use a const or volatile
 		     function where an ordinary one is wanted, but not
 		     vice-versa.  */
 		  if (TYPE_QUALS_NO_ADDR_SPACE (ttl)
 		      & ~TYPE_QUALS_NO_ADDR_SPACE (ttr))
-		    WARN_FOR_ASSIGNMENT (location, 0,
+		    WARN_FOR_QUALIFIERS (location, 0,
 					 G_("passing argument %d of %qE "
-					    "makes qualified function "
+					    "makes %q#v qualified function "
 					    "pointer from unqualified"),
-					 G_("assignment makes qualified "
+					 G_("assignment makes %q#v qualified "
 					    "function pointer from "
 					    "unqualified"),
-					 G_("initialization makes qualified "
+					 G_("initialization makes %q#v qualified "
 					    "function pointer from "
 					    "unqualified"),
-					 G_("return makes qualified function "
-					    "pointer from unqualified"));
+					 G_("return makes %q#v qualified function "
+					    "pointer from unqualified"),
+					 TYPE_QUALS (ttl) & ~TYPE_QUALS (ttr));
 		}
 	      else if (TYPE_QUALS_NO_ADDR_SPACE (ttr)
 		       & ~TYPE_QUALS_NO_ADDR_SPACE (ttl))
-		WARN_FOR_ASSIGNMENT (location, 0,
+		WARN_FOR_QUALIFIERS (location, 0,
 				     G_("passing argument %d of %qE discards "
-					"qualifiers from pointer target type"),
-				     G_("assignment discards qualifiers "
+					"%qv qualifier from pointer target type"),
+				     G_("assignment discards %qv qualifier "
 					"from pointer target type"),
-				     G_("initialization discards qualifiers "
+				     G_("initialization discards %qv qualifier "
 					"from pointer target type"),
-				     G_("return discards qualifiers from "
-					"pointer target type"));
+				     G_("return discards %qv qualifier from "
+					"pointer target type"),
+				     TYPE_QUALS (ttr) & ~TYPE_QUALS (ttl));
 
 	      memb = marginal_memb;
 	    }
 
 	  if (!fundecl || !DECL_IN_SYSTEM_HEADER (fundecl))
@@ -5322,19 +5357,20 @@ convert_for_assignment (location_t locat
 		{
 		  /* Types differing only by the presence of the 'volatile'
 		     qualifier are acceptable if the 'volatile' has been added
 		     in by the Objective-C EH machinery.  */
 		  if (!objc_type_quals_match (ttl, ttr))
-		    WARN_FOR_ASSIGNMENT (location, 0,
+		    WARN_FOR_QUALIFIERS (location, 0,
 					 G_("passing argument %d of %qE discards "
-					    "qualifiers from pointer target type"),
-					 G_("assignment discards qualifiers "
+					    "%qv qualifier from pointer target type"),
+					 G_("assignment discards %qv qualifier "
 					    "from pointer target type"),
-					 G_("initialization discards qualifiers "
+					 G_("initialization discards %qv qualifier "
 					    "from pointer target type"),
-					 G_("return discards qualifiers from "
-					    "pointer target type"));
+					 G_("return discards %qv qualifier from "
+					    "pointer target type"),
+					 TYPE_QUALS (ttr) & ~TYPE_QUALS (ttl));
 		}
 	      /* If this is not a case of ignoring a mismatch in signedness,
 		 no warning.  */
 	      else if (VOID_TYPE_P (ttl) || VOID_TYPE_P (ttr)
 		       || target_cmp)
@@ -5358,20 +5394,21 @@ convert_for_assignment (location_t locat
 		 that say the function will not do certain things,
 		 it is okay to use a const or volatile function
 		 where an ordinary one is wanted, but not vice-versa.  */
 	      if (TYPE_QUALS_NO_ADDR_SPACE (ttl)
 		  & ~TYPE_QUALS_NO_ADDR_SPACE (ttr))
-		WARN_FOR_ASSIGNMENT (location, 0,
+		WARN_FOR_QUALIFIERS (location, 0,
 				     G_("passing argument %d of %qE makes "
-					"qualified function pointer "
+					"%q#v qualified function pointer "
 					"from unqualified"),
-				     G_("assignment makes qualified function "
+				     G_("assignment makes %q#v qualified function "
 					"pointer from unqualified"),
-				     G_("initialization makes qualified "
+				     G_("initialization makes %q#v qualified "
 					"function pointer from unqualified"),
-				     G_("return makes qualified function "
-					"pointer from unqualified"));
+				     G_("return makes %q#v qualified function "
+					"pointer from unqualified"),
+				     TYPE_QUALS (ttl) & ~TYPE_QUALS (ttr));
 	    }
 	}
       else
 	/* Avoid warning about the volatile ObjC EH puts on decls.  */
 	if (!objc_ok)

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]