diff mbox series

[C++] Remove most uses of in_system_header_at

Message ID 23d0f763-658b-31e5-6ca0-4411c6285610@oracle.com
State New
Headers show
Series [C++] Remove most uses of in_system_header_at | expand

Commit Message

Paolo Carlini Oct. 11, 2019, 9:48 p.m. UTC
Hi,

as promised, this removes most uses of the predicate. I left alone the 
two occurrences in decl.c which guard permerrors, not warnings, and two 
more in parser.c which should be a (minor) optimization (particularly 
that in cp_parser_cast_expression).

Tested x86_64-linux.

Thanks, Paolo.

///////////////////////
2019-10-11  Paolo Carlini  <paolo.carlini@oracle.com>

	* decl.c (grokfndecl): Remove redundant use of in_system_header_at.
	(compute_array_index_type_loc): Likewise.
	(grokdeclarator): Likewise.
	* error.c (cp_printer): Likewise.
	* lambda.c (add_default_capture): Likewise.
	* parser.c (cp_parser_primary_expression): Likewise.
	(cp_parser_selection_statement): Likewise.
	(cp_parser_toplevel_declaration): Likewise.
	(cp_parser_enumerator_list): Likewise.
	(cp_parser_using_declaration): Likewise.
	(cp_parser_member_declaration): Likewise.
	(cp_parser_exception_specification_opt): Likewise.
	(cp_parser_std_attribute_spec): Likewise.
	* pt.c (do_decl_instantiation): Likewise.
	(do_type_instantiation): Likewise.
	* typeck.c (cp_build_unary_op): Likewise.

Comments

Jakub Jelinek Oct. 11, 2019, 10:28 p.m. UTC | #1
On Fri, Oct 11, 2019 at 11:48:27PM +0200, Paolo Carlini wrote:
> --- decl.c	(revision 276903)
> +++ decl.c	(working copy)
> @@ -9328,7 +9328,6 @@ grokfndecl (tree ctype,
>  	    }
>  	  /* 17.6.3.3.5  */
>  	  if (suffix[0] != '_'
> -	      && !in_system_header_at (location)
>  	      && !current_function_decl && !(friendp && !funcdef_flag))
>  	    warning_at (location, OPT_Wliteral_suffix,
>  			"literal operator suffixes not preceded by %<_%>"

This should make no functional difference.

> @@ -10036,8 +10035,6 @@ compute_array_index_type_loc (location_t name_loc,
>  	       indicated by the state of complain), so that
>  	       another substitution can be found.  */
>  	    return error_mark_node;
> -	  else if (in_system_header_at (input_location))
> -	    /* Allow them in system headers because glibc uses them.  */;
>  	  else if (name)
>  	    pedwarn (loc, OPT_Wpedantic,
>  		     "ISO C++ forbids zero-size array %qD", name);

But this one is unclear, in_system_header_at is testing a different location
from what will be used by pedwarn, so if input_location is in system header
and loc is not, it didn't pedwarn before and now it will.
Similarly various other spots in the patch.  I haven't tried to check in
detail what exactly we want in each case, all I want to say is that some
cases are obvious, other cases are not.

	Jakub
Paolo Carlini Oct. 14, 2019, 8:51 a.m. UTC | #2
Hi,

On 12/10/19 00:28, Jakub Jelinek wrote:
>> @@ -10036,8 +10035,6 @@ compute_array_index_type_loc (location_t name_loc,
>>   	       indicated by the state of complain), so that
>>   	       another substitution can be found.  */
>>   	    return error_mark_node;
>> -	  else if (in_system_header_at (input_location))
>> -	    /* Allow them in system headers because glibc uses them.  */;
>>   	  else if (name)
>>   	    pedwarn (loc, OPT_Wpedantic,
>>   		     "ISO C++ forbids zero-size array %qD", name);
> But this one is unclear, in_system_header_at is testing a different location
> from what will be used by pedwarn, so if input_location is in system header
> and loc is not, it didn't pedwarn before and now it will.
> Similarly various other spots in the patch.  I haven't tried to check in
> detail what exactly we want in each case, all I want to say is that some
> cases are obvious, other cases are not.

Thanks for noticiing. We already discussed that a bit with Marek: we 
believe it should not make a difference, in practice, because the 
difference should only be quite small and if the first location is in 
the header the second one is supposed to be there too. More importantly, 
as far as I know, the difference is just an historical artifact and many 
actually I created myself ;) when I started changing decl.c, decl2.c, 
etc, to pass locations more consistently: I simply left the 
in_system_header bits alone for simplicity (as discussed with Marek).

Thanks, Paolo.
Paolo Carlini Oct. 16, 2019, 3:59 p.m. UTC | #3
... the below, slightly extended patch: 1- Makes sure the 
in_system_header_at calls surviving in decl.c get the same location used 
for the corresponding diagnostic; exploit locations[ds_typedef] in an 
error_at in grokdeclarator.

Tested, as usual, on x86_64-linux.

Thanks, Paolo.

/////////////////////////
/cp
2019-10-16  Paolo Carlini  <paolo.carlini@oracle.com>

	* decl.c (grokfndecl): Remove redundant use of in_system_header_at.
	(compute_array_index_type_loc): Likewise.
	(grokdeclarator): Likewise.
	* error.c (cp_printer): Likewise.
	* lambda.c (add_default_capture): Likewise.
	* parser.c (cp_parser_primary_expression): Likewise.
	(cp_parser_selection_statement): Likewise.
	(cp_parser_toplevel_declaration): Likewise.
	(cp_parser_enumerator_list): Likewise.
	(cp_parser_using_declaration): Likewise.
	(cp_parser_member_declaration): Likewise.
	(cp_parser_exception_specification_opt): Likewise.
	(cp_parser_std_attribute_spec): Likewise.
	* pt.c (do_decl_instantiation): Likewise.
	(do_type_instantiation): Likewise.
	* typeck.c (cp_build_unary_op): Likewise.

	* decl.c (check_tag_decl): Pass to in_system_header_at the same
	location used for the permerror.
	(grokdeclarator): Likewise.

	* decl.c (check_tag_decl): Use locations[ds_typedef] in error_at.

/testsuite
2019-10-16  Paolo Carlini  <paolo.carlini@oracle.com>

	* g++.old-deja/g++.other/decl9.C: Check locations too.
Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 276984)
+++ cp/decl.c	(working copy)
@@ -4933,9 +4933,9 @@ check_tag_decl (cp_decl_specifier_seq *declspecs,
 	      "multiple types in one declaration");
   else if (declspecs->redefined_builtin_type)
     {
-      if (!in_system_header_at (input_location))
-	permerror (declspecs->locations[ds_redefined_builtin_type_spec],
-		   "redeclaration of C++ built-in type %qT",
+      location_t loc = declspecs->locations[ds_redefined_builtin_type_spec];
+      if (!in_system_header_at (loc))
+	permerror (loc, "redeclaration of C++ built-in type %qT",
 		   declspecs->redefined_builtin_type);
       return NULL_TREE;
     }
@@ -4984,7 +4984,8 @@ check_tag_decl (cp_decl_specifier_seq *declspecs,
 	 --end example]  */
       if (saw_typedef)
 	{
-	  error ("missing type-name in typedef-declaration");
+	  error_at (declspecs->locations[ds_typedef],
+		    "missing type-name in typedef-declaration");
 	  return NULL_TREE;
 	}
       /* Anonymous unions are objects, so they can have specifiers.  */;
@@ -9328,7 +9329,6 @@ grokfndecl (tree ctype,
 	    }
 	  /* 17.6.3.3.5  */
 	  if (suffix[0] != '_'
-	      && !in_system_header_at (location)
 	      && !current_function_decl && !(friendp && !funcdef_flag))
 	    warning_at (location, OPT_Wliteral_suffix,
 			"literal operator suffixes not preceded by %<_%>"
@@ -10036,8 +10036,6 @@ compute_array_index_type_loc (location_t name_loc,
 	       indicated by the state of complain), so that
 	       another substitution can be found.  */
 	    return error_mark_node;
-	  else if (in_system_header_at (input_location))
-	    /* Allow them in system headers because glibc uses them.  */;
 	  else if (name)
 	    pedwarn (loc, OPT_Wpedantic,
 		     "ISO C++ forbids zero-size array %qD", name);
@@ -11004,7 +11002,7 @@ grokdeclarator (const cp_declarator *declarator,
 
       if (type_was_error_mark_node)
 	/* We've already issued an error, don't complain more.  */;
-      else if (in_system_header_at (input_location) || flag_ms_extensions)
+      else if (in_system_header_at (id_loc) || flag_ms_extensions)
 	/* Allow it, sigh.  */;
       else if (! is_main)
 	permerror (id_loc, "ISO C++ forbids declaration of %qs with no type",
@@ -11037,7 +11035,7 @@ grokdeclarator (const cp_declarator *declarator,
 	}
       /* Don't pedwarn if the alternate "__intN__" form has been used instead
 	 of "__intN".  */
-      else if (!int_n_alt && pedantic && ! in_system_header_at (input_location))
+      else if (!int_n_alt && pedantic)
 	pedwarn (declspecs->locations[ds_type_spec], OPT_Wpedantic,
 		 "ISO C++ does not support %<__int%d%> for %qs",
 		 int_n_data[declspecs->int_n_idx].bitsize, name);
@@ -12695,10 +12693,7 @@ grokdeclarator (const cp_declarator *declarator,
 	    else
 	      {
 		/* Array is a flexible member.  */
-		if (in_system_header_at (input_location))
-		  /* Do not warn on flexible array members in system
-		     headers because glibc uses them.  */;
-		else if (name)
+		if (name)
 		  pedwarn (id_loc, OPT_Wpedantic,
 			   "ISO C++ forbids flexible array member %qs", name);
 		else
Index: cp/error.c
===================================================================
--- cp/error.c	(revision 276984)
+++ cp/error.c	(working copy)
@@ -4317,10 +4317,7 @@ cp_printer (pretty_printer *pp, text_info *text, c
 void
 maybe_warn_cpp0x (cpp0x_warn_str str)
 {
-  if ((cxx_dialect == cxx98) && !in_system_header_at (input_location))
-    /* We really want to suppress this warning in system headers,
-       because libstdc++ uses variadic templates even when we aren't
-       in C++0x mode. */
+  if (cxx_dialect == cxx98)
     switch (str)
       {
       case CPP0X_INITIALIZER_LISTS:
Index: cp/lambda.c
===================================================================
--- cp/lambda.c	(revision 276984)
+++ cp/lambda.c	(working copy)
@@ -697,8 +697,7 @@ add_default_capture (tree lambda_stack, tree id, t
       /* Warn about deprecated implicit capture of this via [=].  */
       if (cxx_dialect >= cxx2a
 	  && this_capture_p
-	  && LAMBDA_EXPR_DEFAULT_CAPTURE_MODE (lambda) == CPLD_COPY
-	  && !in_system_header_at (LAMBDA_EXPR_LOCATION (lambda)))
+	  && LAMBDA_EXPR_DEFAULT_CAPTURE_MODE (lambda) == CPLD_COPY)
 	{
 	  if (warning_at (LAMBDA_EXPR_LOCATION (lambda), OPT_Wdeprecated,
 			  "implicit capture of %qE via %<[=]%> is deprecated "
Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 276984)
+++ cp/parser.c	(working copy)
@@ -5364,8 +5364,7 @@ cp_parser_primary_expression (cp_parser *parser,
 	  {
 	    expr = cp_parser_fold_expression (parser, expr);
 	    if (expr != error_mark_node
-		&& cxx_dialect < cxx17
-		&& !in_system_header_at (input_location))
+		&& cxx_dialect < cxx17)
 	      pedwarn (input_location, 0, "fold-expressions only available "
 		       "with %<-std=c++17%> or %<-std=gnu++17%>");
 	  }
@@ -11817,7 +11816,7 @@ cp_parser_selection_statement (cp_parser* parser,
 	  {
 	    cx = true;
 	    cp_token *tok = cp_lexer_consume_token (parser->lexer);
-	    if (cxx_dialect < cxx17 && !in_system_header_at (tok->location))
+	    if (cxx_dialect < cxx17)
 	      pedwarn (tok->location, 0, "%<if constexpr%> only available "
 		       "with %<-std=c++17%> or %<-std=gnu++17%>");
 	  }
@@ -13314,8 +13313,7 @@ cp_parser_toplevel_declaration (cp_parser* parser)
       /* A declaration consisting of a single semicolon is
 	 invalid.  Allow it unless we're being pedantic.  */
       cp_lexer_consume_token (parser->lexer);
-      if (!in_system_header_at (input_location))
-	pedwarn (input_location, OPT_Wpedantic, "extra %<;%>");
+      pedwarn (input_location, OPT_Wpedantic, "extra %<;%>");
     }
   else
     /* Parse the declaration itself.  */
@@ -19193,7 +19191,7 @@ cp_parser_enumerator_list (cp_parser* parser, tree
       /* If the next token is a `}', there is a trailing comma.  */
       if (cp_lexer_next_token_is (parser->lexer, CPP_CLOSE_BRACE))
 	{
-	  if (cxx_dialect < cxx11 && !in_system_header_at (input_location))
+	  if (cxx_dialect < cxx11)
 	    pedwarn (input_location, OPT_Wpedantic,
                      "comma at end of enumerator list");
 	  break;
@@ -19655,8 +19653,7 @@ cp_parser_using_declaration (cp_parser* parser,
   else if (cp_lexer_next_token_is (parser->lexer, CPP_ELLIPSIS))
     {
       cp_token *ell = cp_lexer_consume_token (parser->lexer);
-      if (cxx_dialect < cxx17
-	  && !in_system_header_at (ell->location))
+      if (cxx_dialect < cxx17)
 	pedwarn (ell->location, 0,
 		 "pack expansion in using-declaration only available "
 		 "with %<-std=c++17%> or %<-std=gnu++17%>");
@@ -24835,7 +24832,6 @@ cp_parser_member_declaration (cp_parser* parser)
 		  location_t loc
 		    = cp_lexer_peek_token (parser->lexer)->location;
 		  if (cxx_dialect < cxx2a
-		      && !in_system_header_at (loc)
 		      && identifier != NULL_TREE)
 		    pedwarn (loc, 0,
 			     "default member initializers for bit-fields "
@@ -25692,7 +25688,7 @@ cp_parser_exception_specification_opt (cp_parser*
 			 "specifications");
 	  type_id_list = NULL_TREE;
 	}
-      else if (cxx_dialect >= cxx11 && !in_system_header_at (loc))
+      else if (cxx_dialect >= cxx11)
 	warning_at (loc, OPT_Wdeprecated,
 		    "dynamic exception specifications are deprecated in "
 		    "C++11");
@@ -26680,8 +26676,7 @@ cp_parser_std_attribute_spec (cp_parser *parser)
 	  if (attr_ns
 	      && cp_lexer_nth_token_is (parser->lexer, 3, CPP_COLON))
 	    {
-	      if (cxx_dialect < cxx17
-		  && !in_system_header_at (input_location))
+	      if (cxx_dialect < cxx17)
 		pedwarn (input_location, 0,
 			 "attribute using prefix only available "
 			 "with %<-std=c++17%> or %<-std=gnu++17%>");
Index: cp/pt.c
===================================================================
--- cp/pt.c	(revision 276984)
+++ cp/pt.c	(working copy)
@@ -24257,7 +24257,7 @@ do_decl_instantiation (tree decl, tree storage)
     ;
   else if (storage == ridpointers[(int) RID_EXTERN])
     {
-      if (!in_system_header_at (input_location) && (cxx_dialect == cxx98))
+      if (cxx_dialect == cxx98)
 	pedwarn (input_location, OPT_Wpedantic,
 		 "ISO C++ 1998 forbids the use of %<extern%> on explicit "
 		 "instantiations");
@@ -24339,20 +24339,17 @@ do_type_instantiation (tree t, tree storage, tsubs
 
   if (storage != NULL_TREE)
     {
-      if (!in_system_header_at (input_location))
+      if (storage == ridpointers[(int) RID_EXTERN])
 	{
-	  if (storage == ridpointers[(int) RID_EXTERN])
-	    {
-	      if (cxx_dialect == cxx98)
-		pedwarn (input_location, OPT_Wpedantic,
-			 "ISO C++ 1998 forbids the use of %<extern%> on "
-			 "explicit instantiations");
-	    }
-	  else
+	  if (cxx_dialect == cxx98)
 	    pedwarn (input_location, OPT_Wpedantic,
-		     "ISO C++ forbids the use of %qE"
-		     " on explicit instantiations", storage);
+		     "ISO C++ 1998 forbids the use of %<extern%> on "
+		     "explicit instantiations");
 	}
+      else
+	pedwarn (input_location, OPT_Wpedantic,
+		 "ISO C++ forbids the use of %qE"
+		 " on explicit instantiations", storage);
 
       if (storage == ridpointers[(int) RID_INLINE])
 	nomem_p = 1;
Index: cp/typeck.c
===================================================================
--- cp/typeck.c	(revision 276984)
+++ cp/typeck.c	(working copy)
@@ -6533,7 +6533,7 @@ cp_build_unary_op (enum tree_code code, tree xarg,
 		    return error_mark_node;
 		  }
 		/* Otherwise, [depr.incr.bool] says this is deprecated.  */
-		else if (!in_system_header_at (input_location))
+		else
 		  warning (OPT_Wdeprecated, "use of an operand of type %qT "
 			   "in %<operator++%> is deprecated",
 			   boolean_type_node);
Index: testsuite/g++.old-deja/g++.other/decl9.C
===================================================================
--- testsuite/g++.old-deja/g++.other/decl9.C	(revision 276984)
+++ testsuite/g++.old-deja/g++.other/decl9.C	(working copy)
@@ -4,7 +4,7 @@
 // Contributed by Gabriel Dos Reis <gdr@codesourcery.com>
 
 typedef struct { } S;           // OK
-typedef struct { };             // { dg-error "" } Missing type-name
+typedef struct { };             // { dg-error "1:missing type-name" } Missing type-name
 
 typedef union { } U;            // OK
-typedef union { };              // { dg-error "" } Missing type-name
+typedef union { };              // { dg-error "1:missing type-name" } Missing type-name
Jason Merrill Oct. 17, 2019, 3:15 a.m. UTC | #4
On 10/16/19 11:59 AM, Paolo Carlini wrote:
> ... the below, slightly extended patch: 1- Makes sure the 
> in_system_header_at calls surviving in decl.c get the same location used 
> for the corresponding diagnostic

Hmm, we probably want to change permerror to respect warn_system_headers 
like warning and pedwarn.

Jason
Paolo Carlini Oct. 17, 2019, 2:16 p.m. UTC | #5
Hi,

On 17/10/19 05:15, Jason Merrill wrote:
> On 10/16/19 11:59 AM, Paolo Carlini wrote:
>> ... the below, slightly extended patch: 1- Makes sure the 
>> in_system_header_at calls surviving in decl.c get the same location 
>> used for the corresponding diagnostic
> Hmm, we probably want to change permerror to respect 
> warn_system_headers like warning and pedwarn.

I see. Certainly it enables cleaning up those two remaining usages in 
decl.c.

Which is the right place to implement this? The beginning of 
diagnostic_impl figures out if a given permerror boils down to an actual 
error or a warning: if we use in_system_header_at at that level, part of 
the permissive_error_kind macro, then the warning is handled as any 
other other warning, thus by default suppressed in system headers, etc. 
Makes sense? Tested x86_64-linux.

Thanks, Paolo.

////////////////////////////
Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 277097)
+++ cp/decl.c	(working copy)
@@ -4933,10 +4933,9 @@ check_tag_decl (cp_decl_specifier_seq *declspecs,
 	      "multiple types in one declaration");
   else if (declspecs->redefined_builtin_type)
     {
-      if (!in_system_header_at (input_location))
-	permerror (declspecs->locations[ds_redefined_builtin_type_spec],
-		   "redeclaration of C++ built-in type %qT",
-		   declspecs->redefined_builtin_type);
+      permerror (declspecs->locations[ds_redefined_builtin_type_spec],
+		 "redeclaration of C++ built-in type %qT",
+		 declspecs->redefined_builtin_type);
       return NULL_TREE;
     }
 
@@ -4984,7 +4983,8 @@ check_tag_decl (cp_decl_specifier_seq *declspecs,
 	 --end example]  */
       if (saw_typedef)
 	{
-	  error ("missing type-name in typedef-declaration");
+	  error_at (declspecs->locations[ds_typedef],
+		    "missing type-name in typedef-declaration");
 	  return NULL_TREE;
 	}
       /* Anonymous unions are objects, so they can have specifiers.  */;
@@ -9328,7 +9328,6 @@ grokfndecl (tree ctype,
 	    }
 	  /* 17.6.3.3.5  */
 	  if (suffix[0] != '_'
-	      && !in_system_header_at (location)
 	      && !current_function_decl && !(friendp && !funcdef_flag))
 	    warning_at (location, OPT_Wliteral_suffix,
 			"literal operator suffixes not preceded by %<_%>"
@@ -10036,8 +10035,6 @@ compute_array_index_type_loc (location_t name_loc,
 	       indicated by the state of complain), so that
 	       another substitution can be found.  */
 	    return error_mark_node;
-	  else if (in_system_header_at (input_location))
-	    /* Allow them in system headers because glibc uses them.  */;
 	  else if (name)
 	    pedwarn (loc, OPT_Wpedantic,
 		     "ISO C++ forbids zero-size array %qD", name);
@@ -11004,7 +11001,7 @@ grokdeclarator (const cp_declarator *declarator,
 
       if (type_was_error_mark_node)
 	/* We've already issued an error, don't complain more.  */;
-      else if (in_system_header_at (input_location) || flag_ms_extensions)
+      else if (flag_ms_extensions)
 	/* Allow it, sigh.  */;
       else if (! is_main)
 	permerror (id_loc, "ISO C++ forbids declaration of %qs with no type",
@@ -11037,7 +11034,7 @@ grokdeclarator (const cp_declarator *declarator,
 	}
       /* Don't pedwarn if the alternate "__intN__" form has been used instead
 	 of "__intN".  */
-      else if (!int_n_alt && pedantic && ! in_system_header_at (input_location))
+      else if (!int_n_alt && pedantic)
 	pedwarn (declspecs->locations[ds_type_spec], OPT_Wpedantic,
 		 "ISO C++ does not support %<__int%d%> for %qs",
 		 int_n_data[declspecs->int_n_idx].bitsize, name);
@@ -12695,10 +12692,7 @@ grokdeclarator (const cp_declarator *declarator,
 	    else
 	      {
 		/* Array is a flexible member.  */
-		if (in_system_header_at (input_location))
-		  /* Do not warn on flexible array members in system
-		     headers because glibc uses them.  */;
-		else if (name)
+		if (name)
 		  pedwarn (id_loc, OPT_Wpedantic,
 			   "ISO C++ forbids flexible array member %qs", name);
 		else
Index: cp/error.c
===================================================================
--- cp/error.c	(revision 277097)
+++ cp/error.c	(working copy)
@@ -4317,10 +4317,7 @@ cp_printer (pretty_printer *pp, text_info *text, c
 void
 maybe_warn_cpp0x (cpp0x_warn_str str)
 {
-  if ((cxx_dialect == cxx98) && !in_system_header_at (input_location))
-    /* We really want to suppress this warning in system headers,
-       because libstdc++ uses variadic templates even when we aren't
-       in C++0x mode. */
+  if (cxx_dialect == cxx98)
     switch (str)
       {
       case CPP0X_INITIALIZER_LISTS:
Index: cp/lambda.c
===================================================================
--- cp/lambda.c	(revision 277097)
+++ cp/lambda.c	(working copy)
@@ -697,8 +697,7 @@ add_default_capture (tree lambda_stack, tree id, t
       /* Warn about deprecated implicit capture of this via [=].  */
       if (cxx_dialect >= cxx2a
 	  && this_capture_p
-	  && LAMBDA_EXPR_DEFAULT_CAPTURE_MODE (lambda) == CPLD_COPY
-	  && !in_system_header_at (LAMBDA_EXPR_LOCATION (lambda)))
+	  && LAMBDA_EXPR_DEFAULT_CAPTURE_MODE (lambda) == CPLD_COPY)
 	{
 	  if (warning_at (LAMBDA_EXPR_LOCATION (lambda), OPT_Wdeprecated,
 			  "implicit capture of %qE via %<[=]%> is deprecated "
Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 277097)
+++ cp/parser.c	(working copy)
@@ -5364,8 +5364,7 @@ cp_parser_primary_expression (cp_parser *parser,
 	  {
 	    expr = cp_parser_fold_expression (parser, expr);
 	    if (expr != error_mark_node
-		&& cxx_dialect < cxx17
-		&& !in_system_header_at (input_location))
+		&& cxx_dialect < cxx17)
 	      pedwarn (input_location, 0, "fold-expressions only available "
 		       "with %<-std=c++17%> or %<-std=gnu++17%>");
 	  }
@@ -11817,7 +11816,7 @@ cp_parser_selection_statement (cp_parser* parser,
 	  {
 	    cx = true;
 	    cp_token *tok = cp_lexer_consume_token (parser->lexer);
-	    if (cxx_dialect < cxx17 && !in_system_header_at (tok->location))
+	    if (cxx_dialect < cxx17)
 	      pedwarn (tok->location, 0, "%<if constexpr%> only available "
 		       "with %<-std=c++17%> or %<-std=gnu++17%>");
 	  }
@@ -13314,8 +13313,7 @@ cp_parser_toplevel_declaration (cp_parser* parser)
       /* A declaration consisting of a single semicolon is
 	 invalid.  Allow it unless we're being pedantic.  */
       cp_lexer_consume_token (parser->lexer);
-      if (!in_system_header_at (input_location))
-	pedwarn (input_location, OPT_Wpedantic, "extra %<;%>");
+      pedwarn (input_location, OPT_Wpedantic, "extra %<;%>");
     }
   else
     /* Parse the declaration itself.  */
@@ -19193,7 +19191,7 @@ cp_parser_enumerator_list (cp_parser* parser, tree
       /* If the next token is a `}', there is a trailing comma.  */
       if (cp_lexer_next_token_is (parser->lexer, CPP_CLOSE_BRACE))
 	{
-	  if (cxx_dialect < cxx11 && !in_system_header_at (input_location))
+	  if (cxx_dialect < cxx11)
 	    pedwarn (input_location, OPT_Wpedantic,
                      "comma at end of enumerator list");
 	  break;
@@ -19655,8 +19653,7 @@ cp_parser_using_declaration (cp_parser* parser,
   else if (cp_lexer_next_token_is (parser->lexer, CPP_ELLIPSIS))
     {
       cp_token *ell = cp_lexer_consume_token (parser->lexer);
-      if (cxx_dialect < cxx17
-	  && !in_system_header_at (ell->location))
+      if (cxx_dialect < cxx17)
 	pedwarn (ell->location, 0,
 		 "pack expansion in using-declaration only available "
 		 "with %<-std=c++17%> or %<-std=gnu++17%>");
@@ -24835,7 +24832,6 @@ cp_parser_member_declaration (cp_parser* parser)
 		  location_t loc
 		    = cp_lexer_peek_token (parser->lexer)->location;
 		  if (cxx_dialect < cxx2a
-		      && !in_system_header_at (loc)
 		      && identifier != NULL_TREE)
 		    pedwarn (loc, 0,
 			     "default member initializers for bit-fields "
@@ -25692,7 +25688,7 @@ cp_parser_exception_specification_opt (cp_parser*
 			 "specifications");
 	  type_id_list = NULL_TREE;
 	}
-      else if (cxx_dialect >= cxx11 && !in_system_header_at (loc))
+      else if (cxx_dialect >= cxx11)
 	warning_at (loc, OPT_Wdeprecated,
 		    "dynamic exception specifications are deprecated in "
 		    "C++11");
@@ -26680,8 +26676,7 @@ cp_parser_std_attribute_spec (cp_parser *parser)
 	  if (attr_ns
 	      && cp_lexer_nth_token_is (parser->lexer, 3, CPP_COLON))
 	    {
-	      if (cxx_dialect < cxx17
-		  && !in_system_header_at (input_location))
+	      if (cxx_dialect < cxx17)
 		pedwarn (input_location, 0,
 			 "attribute using prefix only available "
 			 "with %<-std=c++17%> or %<-std=gnu++17%>");
Index: cp/pt.c
===================================================================
--- cp/pt.c	(revision 277097)
+++ cp/pt.c	(working copy)
@@ -24257,7 +24257,7 @@ do_decl_instantiation (tree decl, tree storage)
     ;
   else if (storage == ridpointers[(int) RID_EXTERN])
     {
-      if (!in_system_header_at (input_location) && (cxx_dialect == cxx98))
+      if (cxx_dialect == cxx98)
 	pedwarn (input_location, OPT_Wpedantic,
 		 "ISO C++ 1998 forbids the use of %<extern%> on explicit "
 		 "instantiations");
@@ -24339,20 +24339,17 @@ do_type_instantiation (tree t, tree storage, tsubs
 
   if (storage != NULL_TREE)
     {
-      if (!in_system_header_at (input_location))
+      if (storage == ridpointers[(int) RID_EXTERN])
 	{
-	  if (storage == ridpointers[(int) RID_EXTERN])
-	    {
-	      if (cxx_dialect == cxx98)
-		pedwarn (input_location, OPT_Wpedantic,
-			 "ISO C++ 1998 forbids the use of %<extern%> on "
-			 "explicit instantiations");
-	    }
-	  else
+	  if (cxx_dialect == cxx98)
 	    pedwarn (input_location, OPT_Wpedantic,
-		     "ISO C++ forbids the use of %qE"
-		     " on explicit instantiations", storage);
+		     "ISO C++ 1998 forbids the use of %<extern%> on "
+		     "explicit instantiations");
 	}
+      else
+	pedwarn (input_location, OPT_Wpedantic,
+		 "ISO C++ forbids the use of %qE"
+		 " on explicit instantiations", storage);
 
       if (storage == ridpointers[(int) RID_INLINE])
 	nomem_p = 1;
Index: cp/typeck.c
===================================================================
--- cp/typeck.c	(revision 277097)
+++ cp/typeck.c	(working copy)
@@ -6533,7 +6533,7 @@ cp_build_unary_op (enum tree_code code, tree xarg,
 		    return error_mark_node;
 		  }
 		/* Otherwise, [depr.incr.bool] says this is deprecated.  */
-		else if (!in_system_header_at (input_location))
+		else
 		  warning (OPT_Wdeprecated, "use of an operand of type %qT "
 			   "in %<operator++%> is deprecated",
 			   boolean_type_node);
Index: diagnostic.c
===================================================================
--- diagnostic.c	(revision 277097)
+++ diagnostic.c	(working copy)
@@ -54,7 +54,8 @@ along with GCC; see the file COPYING3.  If not see
 
 #define pedantic_warning_kind(DC)			\
   ((DC)->pedantic_errors ? DK_ERROR : DK_WARNING)
-#define permissive_error_kind(DC) ((DC)->permissive ? DK_WARNING : DK_ERROR)
+#define permissive_error_kind(DC, LOC)                  \
+  ((DC)->permissive || in_system_header_at (LOC) ? DK_WARNING : DK_ERROR)
 #define permissive_error_option(DC) ((DC)->opt_permissive)
 
 /* Prototypes.  */
@@ -1188,8 +1189,8 @@ diagnostic_impl (rich_location *richloc, int opt,
   diagnostic_info diagnostic;
   if (kind == DK_PERMERROR)
     {
-      diagnostic_set_info (&diagnostic, gmsgid, ap, richloc,
-			   permissive_error_kind (global_dc));
+      kind = permissive_error_kind (global_dc, richloc->get_loc ());
+      diagnostic_set_info (&diagnostic, gmsgid, ap, richloc, kind);
       diagnostic.option_index = permissive_error_option (global_dc);
     }
   else
Index: testsuite/g++.old-deja/g++.other/decl9.C
===================================================================
--- testsuite/g++.old-deja/g++.other/decl9.C	(revision 277097)
+++ testsuite/g++.old-deja/g++.other/decl9.C	(working copy)
@@ -4,7 +4,7 @@
 // Contributed by Gabriel Dos Reis <gdr@codesourcery.com>
 
 typedef struct { } S;           // OK
-typedef struct { };             // { dg-error "" } Missing type-name
+typedef struct { };             // { dg-error "1:missing type-name" } Missing type-name
 
 typedef union { } U;            // OK
-typedef union { };              // { dg-error "" } Missing type-name
+typedef union { };              // { dg-error "1:missing type-name" } Missing type-name
Paolo Carlini Oct. 17, 2019, 3:30 p.m. UTC | #6
.. hi again.

We have an issue with this idea which I didn't notice earlier today: 
there are some libstdc++ *_neg testcases which rely on permerrors being 
emitted for code in library headers. For example 
20_util/ratio/cons/cons_overflow_neg.cc relies on a permerror for the 
"overflow in constant expression" in constexpr.c. Or, 
20_util/variant/visit_neg.cc relies on the permerror for "invalid 
conversion from .. to .." emitted by convert_like_real.

Something seems a little fishy here but I'm not sure which way we want 
to go: I don't think we want to audit that right here, right now all the 
permerrors in the front-end which could potentially be involved in this 
kind of issue. Even if we, say, promote to plain errors the above two, 
that seems brittle, we got many permerrors which in principle should be 
real errors.

Paolo.
Jason Merrill Oct. 17, 2019, 6:03 p.m. UTC | #7
On 10/17/19 11:30 AM, Paolo Carlini wrote:
> .. hi again.
> 
> We have an issue with this idea which I didn't notice earlier today: 
> there are some libstdc++ *_neg testcases which rely on permerrors being 
> emitted for code in library headers. For example 
> 20_util/ratio/cons/cons_overflow_neg.cc relies on a permerror for the 
> "overflow in constant expression" in constexpr.c. Or, 
> 20_util/variant/visit_neg.cc relies on the permerror for "invalid 
> conversion from .. to .." emitted by convert_like_real.
> 
> Something seems a little fishy here but I'm not sure which way we want 
> to go: I don't think we want to audit that right here, right now all the 
> permerrors in the front-end which could potentially be involved in this 
> kind of issue. Even if we, say, promote to plain errors the above two, 
> that seems brittle, we got many permerrors which in principle should be 
> real errors.

Hmm, true.  So your patch from yesterday is OK.

Jason
diff mbox series

Patch

Index: decl.c
===================================================================
--- decl.c	(revision 276903)
+++ decl.c	(working copy)
@@ -9328,7 +9328,6 @@  grokfndecl (tree ctype,
 	    }
 	  /* 17.6.3.3.5  */
 	  if (suffix[0] != '_'
-	      && !in_system_header_at (location)
 	      && !current_function_decl && !(friendp && !funcdef_flag))
 	    warning_at (location, OPT_Wliteral_suffix,
 			"literal operator suffixes not preceded by %<_%>"
@@ -10036,8 +10035,6 @@  compute_array_index_type_loc (location_t name_loc,
 	       indicated by the state of complain), so that
 	       another substitution can be found.  */
 	    return error_mark_node;
-	  else if (in_system_header_at (input_location))
-	    /* Allow them in system headers because glibc uses them.  */;
 	  else if (name)
 	    pedwarn (loc, OPT_Wpedantic,
 		     "ISO C++ forbids zero-size array %qD", name);
@@ -11037,7 +11034,7 @@  grokdeclarator (const cp_declarator *declarator,
 	}
       /* Don't pedwarn if the alternate "__intN__" form has been used instead
 	 of "__intN".  */
-      else if (!int_n_alt && pedantic && ! in_system_header_at (input_location))
+      else if (!int_n_alt && pedantic)
 	pedwarn (declspecs->locations[ds_type_spec], OPT_Wpedantic,
 		 "ISO C++ does not support %<__int%d%> for %qs",
 		 int_n_data[declspecs->int_n_idx].bitsize, name);
@@ -12695,10 +12692,7 @@  grokdeclarator (const cp_declarator *declarator,
 	    else
 	      {
 		/* Array is a flexible member.  */
-		if (in_system_header_at (input_location))
-		  /* Do not warn on flexible array members in system
-		     headers because glibc uses them.  */;
-		else if (name)
+		if (name)
 		  pedwarn (id_loc, OPT_Wpedantic,
 			   "ISO C++ forbids flexible array member %qs", name);
 		else
Index: error.c
===================================================================
--- error.c	(revision 276903)
+++ error.c	(working copy)
@@ -4317,10 +4317,7 @@  cp_printer (pretty_printer *pp, text_info *text, c
 void
 maybe_warn_cpp0x (cpp0x_warn_str str)
 {
-  if ((cxx_dialect == cxx98) && !in_system_header_at (input_location))
-    /* We really want to suppress this warning in system headers,
-       because libstdc++ uses variadic templates even when we aren't
-       in C++0x mode. */
+  if (cxx_dialect == cxx98)
     switch (str)
       {
       case CPP0X_INITIALIZER_LISTS:
Index: lambda.c
===================================================================
--- lambda.c	(revision 276903)
+++ lambda.c	(working copy)
@@ -697,8 +697,7 @@  add_default_capture (tree lambda_stack, tree id, t
       /* Warn about deprecated implicit capture of this via [=].  */
       if (cxx_dialect >= cxx2a
 	  && this_capture_p
-	  && LAMBDA_EXPR_DEFAULT_CAPTURE_MODE (lambda) == CPLD_COPY
-	  && !in_system_header_at (LAMBDA_EXPR_LOCATION (lambda)))
+	  && LAMBDA_EXPR_DEFAULT_CAPTURE_MODE (lambda) == CPLD_COPY)
 	{
 	  if (warning_at (LAMBDA_EXPR_LOCATION (lambda), OPT_Wdeprecated,
 			  "implicit capture of %qE via %<[=]%> is deprecated "
Index: parser.c
===================================================================
--- parser.c	(revision 276903)
+++ parser.c	(working copy)
@@ -5364,8 +5364,7 @@  cp_parser_primary_expression (cp_parser *parser,
 	  {
 	    expr = cp_parser_fold_expression (parser, expr);
 	    if (expr != error_mark_node
-		&& cxx_dialect < cxx17
-		&& !in_system_header_at (input_location))
+		&& cxx_dialect < cxx17)
 	      pedwarn (input_location, 0, "fold-expressions only available "
 		       "with %<-std=c++17%> or %<-std=gnu++17%>");
 	  }
@@ -11817,7 +11816,7 @@  cp_parser_selection_statement (cp_parser* parser,
 	  {
 	    cx = true;
 	    cp_token *tok = cp_lexer_consume_token (parser->lexer);
-	    if (cxx_dialect < cxx17 && !in_system_header_at (tok->location))
+	    if (cxx_dialect < cxx17)
 	      pedwarn (tok->location, 0, "%<if constexpr%> only available "
 		       "with %<-std=c++17%> or %<-std=gnu++17%>");
 	  }
@@ -13314,8 +13313,7 @@  cp_parser_toplevel_declaration (cp_parser* parser)
       /* A declaration consisting of a single semicolon is
 	 invalid.  Allow it unless we're being pedantic.  */
       cp_lexer_consume_token (parser->lexer);
-      if (!in_system_header_at (input_location))
-	pedwarn (input_location, OPT_Wpedantic, "extra %<;%>");
+      pedwarn (input_location, OPT_Wpedantic, "extra %<;%>");
     }
   else
     /* Parse the declaration itself.  */
@@ -19193,7 +19191,7 @@  cp_parser_enumerator_list (cp_parser* parser, tree
       /* If the next token is a `}', there is a trailing comma.  */
       if (cp_lexer_next_token_is (parser->lexer, CPP_CLOSE_BRACE))
 	{
-	  if (cxx_dialect < cxx11 && !in_system_header_at (input_location))
+	  if (cxx_dialect < cxx11)
 	    pedwarn (input_location, OPT_Wpedantic,
                      "comma at end of enumerator list");
 	  break;
@@ -19655,8 +19653,7 @@  cp_parser_using_declaration (cp_parser* parser,
   else if (cp_lexer_next_token_is (parser->lexer, CPP_ELLIPSIS))
     {
       cp_token *ell = cp_lexer_consume_token (parser->lexer);
-      if (cxx_dialect < cxx17
-	  && !in_system_header_at (ell->location))
+      if (cxx_dialect < cxx17)
 	pedwarn (ell->location, 0,
 		 "pack expansion in using-declaration only available "
 		 "with %<-std=c++17%> or %<-std=gnu++17%>");
@@ -24835,7 +24832,6 @@  cp_parser_member_declaration (cp_parser* parser)
 		  location_t loc
 		    = cp_lexer_peek_token (parser->lexer)->location;
 		  if (cxx_dialect < cxx2a
-		      && !in_system_header_at (loc)
 		      && identifier != NULL_TREE)
 		    pedwarn (loc, 0,
 			     "default member initializers for bit-fields "
@@ -25692,7 +25688,7 @@  cp_parser_exception_specification_opt (cp_parser*
 			 "specifications");
 	  type_id_list = NULL_TREE;
 	}
-      else if (cxx_dialect >= cxx11 && !in_system_header_at (loc))
+      else if (cxx_dialect >= cxx11)
 	warning_at (loc, OPT_Wdeprecated,
 		    "dynamic exception specifications are deprecated in "
 		    "C++11");
@@ -26680,8 +26676,7 @@  cp_parser_std_attribute_spec (cp_parser *parser)
 	  if (attr_ns
 	      && cp_lexer_nth_token_is (parser->lexer, 3, CPP_COLON))
 	    {
-	      if (cxx_dialect < cxx17
-		  && !in_system_header_at (input_location))
+	      if (cxx_dialect < cxx17)
 		pedwarn (input_location, 0,
 			 "attribute using prefix only available "
 			 "with %<-std=c++17%> or %<-std=gnu++17%>");
Index: pt.c
===================================================================
--- pt.c	(revision 276903)
+++ pt.c	(working copy)
@@ -24257,7 +24257,7 @@  do_decl_instantiation (tree decl, tree storage)
     ;
   else if (storage == ridpointers[(int) RID_EXTERN])
     {
-      if (!in_system_header_at (input_location) && (cxx_dialect == cxx98))
+      if (cxx_dialect == cxx98)
 	pedwarn (input_location, OPT_Wpedantic,
 		 "ISO C++ 1998 forbids the use of %<extern%> on explicit "
 		 "instantiations");
@@ -24339,20 +24339,17 @@  do_type_instantiation (tree t, tree storage, tsubs
 
   if (storage != NULL_TREE)
     {
-      if (!in_system_header_at (input_location))
+      if (storage == ridpointers[(int) RID_EXTERN])
 	{
-	  if (storage == ridpointers[(int) RID_EXTERN])
-	    {
-	      if (cxx_dialect == cxx98)
-		pedwarn (input_location, OPT_Wpedantic,
-			 "ISO C++ 1998 forbids the use of %<extern%> on "
-			 "explicit instantiations");
-	    }
-	  else
+	  if (cxx_dialect == cxx98)
 	    pedwarn (input_location, OPT_Wpedantic,
-		     "ISO C++ forbids the use of %qE"
-		     " on explicit instantiations", storage);
+		     "ISO C++ 1998 forbids the use of %<extern%> on "
+		     "explicit instantiations");
 	}
+      else
+	pedwarn (input_location, OPT_Wpedantic,
+		 "ISO C++ forbids the use of %qE"
+		 " on explicit instantiations", storage);
 
       if (storage == ridpointers[(int) RID_INLINE])
 	nomem_p = 1;
Index: typeck.c
===================================================================
--- typeck.c	(revision 276903)
+++ typeck.c	(working copy)
@@ -6533,7 +6533,7 @@  cp_build_unary_op (enum tree_code code, tree xarg,
 		    return error_mark_node;
 		  }
 		/* Otherwise, [depr.incr.bool] says this is deprecated.  */
-		else if (!in_system_header_at (input_location))
+		else
 		  warning (OPT_Wdeprecated, "use of an operand of type %qT "
 			   "in %<operator++%> is deprecated",
 			   boolean_type_node);