[C++] Fix three additional locations

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....

/////////////////////
/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.

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

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.

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

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.

////////////////////
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 }  
 };

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