[C++ Patch] Fix three additional locations

[C++ Patch] Fix three additional locations

[Paolo Carlini]

[-- Attachment #1: Type: text/plain, Size: 154 bytes --]

Hi,

a few additional easy to fix cases where we thought that a plain error 
was good enough. Tested x86_64-linux.

Thanks, Paolo.

////////////////////


[-- Attachment #2: CL_locs_21 --]
[-- Type: text/plain, Size: 314 bytes --]

/cp
2019-01-08  Paolo Carlini  <paolo.carlini@oracle.com>

	* decl.c (grok_reference_init): Improve error location.
	(grokdeclarator): Likewise, improve two locations.

/testsuite
2019-01-08  Paolo Carlini  <paolo.carlini@oracle.com>

	* g++.dg/diagnostic/constexpr2.C: New.
	* g++.dg/diagnostic/ref3.C: Likewise.

[-- Attachment #3: patch_locs_21 --]
[-- Type: text/plain, Size: 2194 bytes --]

Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 267675)
+++ cp/decl.c	(working copy)
@@ -5357,7 +5357,8 @@ grok_reference_init (tree decl, tree type, tree in
       if ((DECL_LANG_SPECIFIC (decl) == 0
 	   || DECL_IN_AGGR_P (decl) == 0)
 	  && ! DECL_THIS_EXTERN (decl))
-	error ("%qD declared as reference but not initialized", decl);
+	error_at (DECL_SOURCE_LOCATION (decl),
+		  "%qD declared as reference but not initialized", decl);
       return NULL_TREE;
     }
 
@@ -12517,8 +12518,9 @@ grokdeclarator (const cp_declarator *declarator,
 			    unqualified_id);
 		else if (constexpr_p && !initialized)
 		  {
-		    error ("%<constexpr%> static data member %qD must have an "
-			   "initializer", decl);
+		    error_at (DECL_SOURCE_LOCATION (decl),
+			      "%<constexpr%> static data member %qD must "
+			      "have an initializer", decl);
 		    constexpr_p = false;
 		  }
 
@@ -12756,8 +12758,9 @@ grokdeclarator (const cp_declarator *declarator,
 	  }
 	else if (constexpr_p && DECL_EXTERNAL (decl))
 	  {
-	    error ("declaration of %<constexpr%> variable %qD "
-		   "is not a definition", decl);
+	    error_at (DECL_SOURCE_LOCATION (decl),
+		      "declaration of %<constexpr%> variable %qD "
+		      "is not a definition", decl);
 	    constexpr_p = false;
 	  }
 
Index: testsuite/g++.dg/diagnostic/constexpr2.C
===================================================================
--- testsuite/g++.dg/diagnostic/constexpr2.C	(nonexistent)
+++ testsuite/g++.dg/diagnostic/constexpr2.C	(working copy)
@@ -0,0 +1,8 @@
+// { dg-do compile { target c++11 } }
+
+extern constexpr int i __attribute__((unused));  // { dg-error "22:declaration of .constexpr. variable .i." }
+
+struct S
+{
+  constexpr static int i __attribute__((unused));  // { dg-error "24:.constexpr. static data member .i." }
+};
Index: testsuite/g++.dg/diagnostic/ref3.C
===================================================================
--- testsuite/g++.dg/diagnostic/ref3.C	(nonexistent)
+++ testsuite/g++.dg/diagnostic/ref3.C	(working copy)
@@ -0,0 +1 @@
+int& i __attribute__((unused));  // { dg-error "6:.i. declared as reference" }

Re: [C++ Patch] Fix three additional locations

[Jason Merrill]

On 1/8/19 8:13 AM, Paolo Carlini wrote:
> Hi,
> 
> a few additional easy to fix cases where we thought that a plain error 
> was good enough. Tested x86_64-linux.
> 
> Thanks, Paolo.
> 
> ////////////////////
> 
OK.

Re: [C++ Patch] Fix three additional locations

[Jason Merrill]

On 1/11/19 4:56 PM, Paolo Carlini wrote:
> Hi,
> 
> On 11/01/19 22:22, Jason Merrill wrote:
>> On 1/11/19 4:13 PM, Paolo Carlini wrote:
>>> Hi,
>>>
>>> On 11/01/19 19:58, Jason Merrill wrote:
>>>> On 1/10/19 9:24 AM, Paolo Carlini wrote:
>>>>> Hi again,
>>>>>
>>>>> this one is also matter of consistency with, say, the precise 
>>>>> location that we use for the error message at the beginning of 
>>>>> check_methods. Indeed, the sequence of error messages of 
>>>>> g++.dg/inherit/pure1.C reflect that. Tested x86_64-linux.
>>>>>
>>>>> Thanks, Paolo.
>>>>>
>>>>> PS: minor issues anyway, but I'm almost done with these low hanging 
>>>>> fruits which I'm proposing to fix for 9 too....
>>>>
>>>> Hmm, wouldn't it be preferable to use the location of the 
>>>> initializer when the initializer is the problem?
>>>
>>> I see what you mean and indeed yesterday I gave that some thought. In 
>>> practice, we have the usual issue that currently constants don't have 
>>> a location
>>
>> They do now in a lot more cases, with location wrappers.  If not, we 
>> could fall back on the decl location with EXPR_LOC_OR_LOC.
> 
> Yes. And that's what we are in fact already doing in all the other uses 
> of cp_expr_loc_or_loc in decl.c. Seems a good solution to me too. I'm 
> finishing testing the below.

OK.

Jason

Re: [C++ Patch] Fix three additional locations

[Paolo Carlini]

[-- Attachment #1: Type: text/plain, Size: 1263 bytes --]

Hi,

On 11/01/19 22:22, Jason Merrill wrote:
> On 1/11/19 4:13 PM, Paolo Carlini wrote:
>> Hi,
>>
>> On 11/01/19 19:58, Jason Merrill wrote:
>>> On 1/10/19 9:24 AM, Paolo Carlini wrote:
>>>> Hi again,
>>>>
>>>> this one is also matter of consistency with, say, the precise 
>>>> location that we use for the error message at the beginning of 
>>>> check_methods. Indeed, the sequence of error messages of 
>>>> g++.dg/inherit/pure1.C reflect that. Tested x86_64-linux.
>>>>
>>>> Thanks, Paolo.
>>>>
>>>> PS: minor issues anyway, but I'm almost done with these low hanging 
>>>> fruits which I'm proposing to fix for 9 too....
>>>
>>> Hmm, wouldn't it be preferable to use the location of the 
>>> initializer when the initializer is the problem?
>>
>> I see what you mean and indeed yesterday I gave that some thought. In 
>> practice, we have the usual issue that currently constants don't have 
>> a location
>
> They do now in a lot more cases, with location wrappers.  If not, we 
> could fall back on the decl location with EXPR_LOC_OR_LOC.

Yes. And that's what we are in fact already doing in all the other uses 
of cp_expr_loc_or_loc in decl.c. Seems a good solution to me too. I'm 
finishing testing the below.

Thanks, Paolo.

////////////////////



[-- Attachment #2: patch_locs_23_2 --]
[-- Type: text/plain, Size: 3467 bytes --]

Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 267858)
+++ cp/decl.c	(working copy)
@@ -7293,7 +7293,10 @@ cp_finish_decl (tree decl, tree init, bool init_co
 		    synthesize_method (decl);
 		}
 	      else
-		error ("function %q#D is initialized like a variable", decl);
+		error_at (cp_expr_loc_or_loc (init,
+					      DECL_SOURCE_LOCATION (decl)),
+			  "function %q#D is initialized like a variable",
+			  decl);
 	    }
 	  /* else no initialization required.  */
 	}
Index: cp/decl2.c
===================================================================
--- cp/decl2.c	(revision 267858)
+++ cp/decl2.c	(working copy)
@@ -924,12 +924,14 @@ grokfield (const cp_declarator *declarator,
 	  else
 	    {
 	      gcc_assert (TREE_CODE (TREE_TYPE (value)) == FUNCTION_TYPE);
+	      location_t iloc
+		= cp_expr_loc_or_loc (init, DECL_SOURCE_LOCATION (value));
 	      if (friendp)
-		error ("initializer specified for friend function %qD",
-		       value);
+		error_at (iloc, "initializer specified for friend "
+			  "function %qD", value);
 	      else
-		error ("initializer specified for static member function %qD",
-		       value);
+		error_at (iloc, "initializer specified for static "
+			  "member function %qD", value);
 	    }
 	}
       else if (TREE_CODE (value) == FIELD_DECL)
Index: testsuite/g++.dg/cpp0x/pr62101.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr62101.C	(revision 267858)
+++ testsuite/g++.dg/cpp0x/pr62101.C	(working copy)
@@ -3,7 +3,7 @@
 
 struct X
 {
-  friend void g(X, int) = 0; // { dg-error "initializer specified for friend function" }
+  friend void g(X, int) = 0; // { dg-error "15:initializer specified for friend function" }
   friend void g(X, int) = default; // { dg-error "cannot be defaulted" }
   // { dg-prune-output "note" }
   friend void f(X, int) = delete;
Index: testsuite/g++.dg/inherit/pure1.C
===================================================================
--- testsuite/g++.dg/inherit/pure1.C	(revision 267858)
+++ testsuite/g++.dg/inherit/pure1.C	(working copy)
@@ -2,13 +2,13 @@
 // Origin: Volker Reichelt  <reichelt@igpm.rwth-aachen.de>
 // { dg-do compile }
 
-void foo0() = 0;                   // { dg-error "like a variable" }
+void foo0() = 0;                   // { dg-error "6:function .void foo0\\(\\). is initialized like a variable" }
 virtual void foo1() = 0;           // { dg-error "1:'virtual' outside class" }
-// { dg-error "like a variable" "" { target *-*-* } .-1 }
+// { dg-error "14:function .void foo1\\(\\). is initialized like a variable" "" { target *-*-* } .-1 }
 struct A
 {
-  void foo2() = 0;                 // { dg-error "non-virtual" }
-  static void foo3() = 0;          // { dg-error "static member" }
+  void foo2() = 0;                 // { dg-error "8:initializer specified for non-virtual method" }
+  static void foo3() = 0;          // { dg-error "15:initializer specified for static member function" }
   virtual static void foo4() = 0;  // { dg-error "both 'virtual' and 'static'" }
   virtual void foo5() = 0;         // { dg-error "base class" }
 };
@@ -15,5 +15,6 @@ struct A
 
 struct B : A
 {
-  static void foo5() = 0;          // { dg-error "static member|declared" }
+  static void foo5() = 0;          // { dg-error "15:initializer specified for static member function" }
+// { dg-error "declared" "" { target *-*-* } .-1 }  
 };

Re: [C++ Patch] Fix three additional locations

[Jason Merrill]

On 1/11/19 4:13 PM, Paolo Carlini wrote:
> Hi,
> 
> On 11/01/19 19:58, Jason Merrill wrote:
>> On 1/10/19 9:24 AM, Paolo Carlini wrote:
>>> Hi again,
>>>
>>> this one is also matter of consistency with, say, the precise 
>>> location that we use for the error message at the beginning of 
>>> check_methods. Indeed, the sequence of error messages of 
>>> g++.dg/inherit/pure1.C reflect that. Tested x86_64-linux.
>>>
>>> Thanks, Paolo.
>>>
>>> PS: minor issues anyway, but I'm almost done with these low hanging 
>>> fruits which I'm proposing to fix for 9 too....
>>
>> Hmm, wouldn't it be preferable to use the location of the initializer 
>> when the initializer is the problem?
> 
> I see what you mean and indeed yesterday I gave that some thought. In 
> practice, we have the usual issue that currently constants don't have a 
> location

They do now in a lot more cases, with location wrappers.  If not, we 
could fall back on the decl location with EXPR_LOC_OR_LOC.  But I 
suppose indicating the decl is fine too.

Jason

Re: [C++ Patch] Fix three additional locations

[Paolo Carlini]

Hi,

On 11/01/19 19:58, Jason Merrill wrote:
> On 1/10/19 9:24 AM, Paolo Carlini wrote:
>> Hi again,
>>
>> this one is also matter of consistency with, say, the precise 
>> location that we use for the error message at the beginning of 
>> check_methods. Indeed, the sequence of error messages of 
>> g++.dg/inherit/pure1.C reflect that. Tested x86_64-linux.
>>
>> Thanks, Paolo.
>>
>> PS: minor issues anyway, but I'm almost done with these low hanging 
>> fruits which I'm proposing to fix for 9 too....
>
> Hmm, wouldn't it be preferable to use the location of the initializer 
> when the initializer is the problem?

I see what you mean and indeed yesterday I gave that some thought. In 
practice, we have the usual issue that currently constants don't have a 
location thus the only - brittle - way to achieve that is relying on 
input_location. That appears to work for the cases in decl.c and decl2.c 
but, as far as I can see, nothing similar can be cooked up for the 
check_methods case (we don't have the initializer at all, input_location 
doesn't make any sense). On the other hand, front-ends like clang just 
consistently use the location of the decl, that's why I decided to go 
ahead and propose a simple solution using everywhere that location.

Looks like for the time being we can't make much progress, because, in 
most of the error messages related to the initializers, thanks to 
input_location we can point to the initializers and resolving the 
inconsistency with check_methods seems tough.

Paolo.

Re: [C++ Patch] Fix three additional locations

[Jason Merrill]

On 1/10/19 9:24 AM, Paolo Carlini wrote:
> Hi again,
> 
> this one is also matter of consistency with, say, the precise location 
> that we use for the error message at the beginning of check_methods. 
> Indeed, the sequence of error messages of g++.dg/inherit/pure1.C reflect 
> that. Tested x86_64-linux.
> 
> Thanks, Paolo.
> 
> PS: minor issues anyway, but I'm almost done with these low hanging 
> fruits which I'm proposing to fix for 9 too....

Hmm, wouldn't it be preferable to use the location of the initializer 
when the initializer is the problem?

Jason

[C++ Patch] Fix three additional locations

[Paolo Carlini]

[-- Attachment #1: Type: text/plain, Size: 412 bytes --]

Hi again,

this one is also matter of consistency with, say, the precise location 
that we use for the error message at the beginning of check_methods. 
Indeed, the sequence of error messages of g++.dg/inherit/pure1.C reflect 
that. Tested x86_64-linux.

Thanks, Paolo.

PS: minor issues anyway, but I'm almost done with these low hanging 
fruits which I'm proposing to fix for 9 too....

/////////////////////


[-- Attachment #2: CL_locs_23 --]
[-- Type: text/plain, Size: 316 bytes --]

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

	* decl.c (cp_finish_decl): Improve error location.
	(grokfield): Likewise, improve two locations.

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

	* g++.dg/g++.dg/cpp0x/pr62101.C: Test locations too.
	* g++.dg/inherit/pure1.C: Likewise.

[-- Attachment #3: patch_locs_23 --]
[-- Type: text/plain, Size: 3389 bytes --]

Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 267807)
+++ cp/decl.c	(working copy)
@@ -7292,7 +7293,9 @@ cp_finish_decl (tree decl, tree init, bool init_co
 		    synthesize_method (decl);
 		}
 	      else
-		error ("function %q#D is initialized like a variable", decl);
+		error_at (DECL_SOURCE_LOCATION (decl),
+			  "function %q#D is initialized like a variable",
+			  decl);
 	    }
 	  /* else no initialization required.  */
 	}
Index: cp/decl2.c
===================================================================
--- cp/decl2.c	(revision 267807)
+++ cp/decl2.c	(working copy)
@@ -925,11 +925,13 @@ grokfield (const cp_declarator *declarator,
 	    {
 	      gcc_assert (TREE_CODE (TREE_TYPE (value)) == FUNCTION_TYPE);
 	      if (friendp)
-		error ("initializer specified for friend function %qD",
-		       value);
+		error_at (DECL_SOURCE_LOCATION (value),
+			  "initializer specified for friend function %qD",
+			  value);
 	      else
-		error ("initializer specified for static member function %qD",
-		       value);
+		error_at (DECL_SOURCE_LOCATION (value),
+			  "initializer specified for static member "
+			  "function %qD", value);
 	    }
 	}
       else if (TREE_CODE (value) == FIELD_DECL)
Index: testsuite/g++.dg/cpp0x/pr62101.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr62101.C	(revision 267807)
+++ testsuite/g++.dg/cpp0x/pr62101.C	(working copy)
@@ -3,7 +3,7 @@
 
 struct X
 {
-  friend void g(X, int) = 0; // { dg-error "initializer specified for friend function" }
+  friend void g(X, int) = 0; // { dg-error "15:initializer specified for friend function" }
   friend void g(X, int) = default; // { dg-error "cannot be defaulted" }
   // { dg-prune-output "note" }
   friend void f(X, int) = delete;
Index: testsuite/g++.dg/inherit/pure1.C
===================================================================
--- testsuite/g++.dg/inherit/pure1.C	(revision 267807)
+++ testsuite/g++.dg/inherit/pure1.C	(working copy)
@@ -2,13 +2,13 @@
 // Origin: Volker Reichelt  <reichelt@igpm.rwth-aachen.de>
 // { dg-do compile }
 
-void foo0() = 0;                   // { dg-error "like a variable" }
+void foo0() = 0;                   // { dg-error "6:function .void foo0\\(\\). is initialized like a variable" }
 virtual void foo1() = 0;           // { dg-error "1:'virtual' outside class" }
-// { dg-error "like a variable" "" { target *-*-* } .-1 }
+// { dg-error "14:function .void foo1\\(\\). is initialized like a variable" "" { target *-*-* } .-1 }
 struct A
 {
-  void foo2() = 0;                 // { dg-error "non-virtual" }
-  static void foo3() = 0;          // { dg-error "static member" }
+  void foo2() = 0;                 // { dg-error "8:initializer specified for non-virtual method" }
+  static void foo3() = 0;          // { dg-error "15:initializer specified for static member function" }
   virtual static void foo4() = 0;  // { dg-error "both 'virtual' and 'static'" }
   virtual void foo5() = 0;         // { dg-error "base class" }
 };
@@ -15,5 +15,6 @@ struct A
 
 struct B : A
 {
-  static void foo5() = 0;          // { dg-error "static member|declared" }
+  static void foo5() = 0;          // { dg-error "15:initializer specified for static member function" }
+// { dg-error "declared" "" { target *-*-* } .-1 }  
 };