diff mbox series

[C++] Use declarator->id_loc in three additional places

Message ID 59ac4b84-5400-8c6f-7666-95e5e900faf6@oracle.com
State New
Headers show
Series [C++] Use declarator->id_loc in three additional places | expand

Commit Message

Paolo Carlini June 4, 2019, 2:31 p.m. UTC 
Hi,

tested x86_64-linux, as usual.

Thanks, Paolo.

///////////////////////
/cp
2019-06-04  Paolo Carlini  <paolo.carlini@oracle.com>

	* decl.c (grokdeclarator): Use declarator->id_loc in three
	additional places.

/testsuite
2019-06-04  Paolo Carlini  <paolo.carlini@oracle.com>

	* g++.dg/concepts/pr60573.C: Test locations too.
	* g++.dg/cpp0x/deleted13.C: Likewise.
	* g++.dg/other/friend4.C: Likewise.
	* g++.dg/other/friend5.C: Likewise.
	* g++.dg/other/friend7.C: Likewise.
	* g++.dg/parse/error29.C: Likewise.
	* g++.dg/parse/friend7.C: Likewise.
	* g++.dg/parse/qualified4.C: Likewise.
	* g++.dg/template/crash96.C Likewise.
	* g++.old-deja/g++.brendan/crash22.C Likewise.
	* g++.old-deja/g++.brendan/crash23.C Likewise.
	* g++.old-deja/g++.law/visibility10.C Likewise.
	* g++.old-deja/g++.other/decl5.C: Likewise.

Comments

Jason Merrill June 4, 2019, 2:50 p.m. UTC | #1 
On 6/4/19 10:31 AM, Paolo Carlini wrote:
> +	      permerror (loc, "member functions are implicitly "
> +			 "friends of their class");

Wouldn't it be better to use the location of "friend" in this diagnostic?

The rest of the patch is OK.

Jason
Paolo Carlini June 4, 2019, 3:57 p.m. UTC | #2 
Hi,

On 04/06/19 16:50, Jason Merrill wrote:
> On 6/4/19 10:31 AM, Paolo Carlini wrote:
>> +          permerror (loc, "member functions are implicitly "
>> +             "friends of their class");
>
> Wouldn't it be better to use the location of "friend" in this diagnostic?

Yes, however doing that fully correctly seems a bit tricky, I thought 
that pointing to the id_loc it's still better than a rather meaningless 
place near the end of the line, eg, before the semicolon. Note this is a 
more general issue, I'll give it some thought... I'm leaving those 
friends alone for the time being.

Thanks, Paolo.
Jason Merrill June 5, 2019, 5:45 p.m. UTC | #3 
On 6/4/19 11:57 AM, Paolo Carlini wrote:
> Hi,
> 
> On 04/06/19 16:50, Jason Merrill wrote:
>> On 6/4/19 10:31 AM, Paolo Carlini wrote:
>>> +          permerror (loc, "member functions are implicitly "
>>> +             "friends of their class");
>>
>> Wouldn't it be better to use the location of "friend" in this diagnostic?
> 
> Yes, however doing that fully correctly seems a bit tricky

Why tricky?  Doesn't declspecs->locations[ds_friend] work?

Jason
Paolo Carlini June 5, 2019, 6:09 p.m. UTC | #4 
Hi,

On 05/06/19 19:45, Jason Merrill wrote:
> On 6/4/19 11:57 AM, Paolo Carlini wrote:
>> Hi,
>>
>> On 04/06/19 16:50, Jason Merrill wrote:
>>> On 6/4/19 10:31 AM, Paolo Carlini wrote:
>>>> +          permerror (loc, "member functions are implicitly "
>>>> +             "friends of their class");
>>>
>>> Wouldn't it be better to use the location of "friend" in this 
>>> diagnostic?
>>
>> Yes, however doing that fully correctly seems a bit tricky
>
> Why tricky?  Doesn't declspecs->locations[ds_friend] work?

To be honest, here I wasn't considering ds_friend at all. Indeed it 
gives us the location of 'friend', but, say, for a testcase like 
parse/friend4.C:

struct A
{
   friend void A::foo();  // { dg-error "implicitly friends" }
   friend A::~A();        // { dg-error "implicitly friends" }
};

I thought we wanted a precise caret under the 'f' of 'foo' and the '~' 
of the destructor - both clang and icc do that - I wasn't even 
considering pointing at 'friend'. If you think that would be an 
improvement wrt the current closed parenthesis - I agreee it would! - I 
can do that!

Paolo.
Jason Merrill June 5, 2019, 6:37 p.m. UTC | #5 
On 6/5/19 2:09 PM, Paolo Carlini wrote:
> Hi,
> 
> On 05/06/19 19:45, Jason Merrill wrote:
>> On 6/4/19 11:57 AM, Paolo Carlini wrote:
>>> Hi,
>>>
>>> On 04/06/19 16:50, Jason Merrill wrote:
>>>> On 6/4/19 10:31 AM, Paolo Carlini wrote:
>>>>> +          permerror (loc, "member functions are implicitly "
>>>>> +             "friends of their class");
>>>>
>>>> Wouldn't it be better to use the location of "friend" in this 
>>>> diagnostic?
>>>
>>> Yes, however doing that fully correctly seems a bit tricky
>>
>> Why tricky?  Doesn't declspecs->locations[ds_friend] work?
> 
> To be honest, here I wasn't considering ds_friend at all. Indeed it 
> gives us the location of 'friend', but, say, for a testcase like 
> parse/friend4.C:
> 
> struct A
> {
>    friend void A::foo();  // { dg-error "implicitly friends" }
>    friend A::~A();        // { dg-error "implicitly friends" }
> };
> 
> I thought we wanted a precise caret under the 'f' of 'foo' and the '~' 
> of the destructor - both clang and icc do that - I wasn't even 
> considering pointing at 'friend'. If you think that would be an 
> improvement wrt the current closed parenthesis - I agreee it would! - I 
> can do that!

Please do.

Jason
diff mbox series

Patch

Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 271899)
+++ cp/decl.c	(working copy)
@@ -11873,6 +11873,8 @@  grokdeclarator (const cp_declarator *declarator,
       unqualified_id = dname;
     }
 
+  location_t loc = declarator ? declarator->id_loc : input_location;
+
   /* If TYPE is a FUNCTION_TYPE, but the function name was explicitly
      qualified with a class-name, turn it into a METHOD_TYPE, unless
      we know that the function is static.  We take advantage of this
@@ -11893,13 +11895,12 @@  grokdeclarator (const cp_declarator *declarator,
 	{
 	  if (friendp)
 	    {
-	      permerror (input_location, "member functions are implicitly "
-					 "friends of their class");
+	      permerror (loc, "member functions are implicitly "
+			 "friends of their class");
 	      friendp = 0;
 	    }
 	  else
-	    permerror (declarator->id_loc, 
-		       "extra qualification %<%T::%> on member %qs",
+	    permerror (loc, "extra qualification %<%T::%> on member %qs",
 		       ctype, name);
 	}
       else if (/* If the qualifying type is already complete, then we
@@ -11928,19 +11929,19 @@  grokdeclarator (const cp_declarator *declarator,
 	  if (current_class_type
 	      && (!friendp || funcdef_flag || initialized))
 	    {
-	      error (funcdef_flag || initialized
-		     ? G_("cannot define member function %<%T::%s%> "
-			  "within %qT")
-		     : G_("cannot declare member function %<%T::%s%> "
-			  "within %qT"),
-		     ctype, name, current_class_type);
+	      error_at (loc, funcdef_flag || initialized
+			? G_("cannot define member function %<%T::%s%> "
+			     "within %qT")
+			: G_("cannot declare member function %<%T::%s%> "
+			     "within %qT"),
+			ctype, name, current_class_type);
 	      return error_mark_node;
 	    }
 	}
       else if (typedef_p && current_class_type)
 	{
-	  error ("cannot declare member %<%T::%s%> within %qT",
-		 ctype, name, current_class_type);
+	  error_at (loc, "cannot declare member %<%T::%s%> within %qT",
+		    ctype, name, current_class_type);
 	  return error_mark_node;
 	}
     }
@@ -12053,8 +12054,6 @@  grokdeclarator (const cp_declarator *declarator,
 	}
     }
 
-  location_t loc = declarator ? declarator->id_loc : input_location;
-
   /* If this is declaring a typedef name, return a TYPE_DECL.  */
   if (typedef_p && decl_context != TYPENAME)
     {
Index: testsuite/g++.dg/concepts/pr60573.C
===================================================================
--- testsuite/g++.dg/concepts/pr60573.C	(revision 271899)
+++ testsuite/g++.dg/concepts/pr60573.C	(working copy)
@@ -9,7 +9,7 @@  struct A
     void foo(auto);
   };
 
-  void B::foo(auto) {}  // { dg-error "cannot define" }
+  void B::foo(auto) {}  // { dg-error "8:cannot define" }
 
   struct X
   {
@@ -21,8 +21,8 @@  struct A
       };
     };
 
-    void Y::Z::foo(auto) {}  // { dg-error "cannot define" }
+    void Y::Z::foo(auto) {}  // { dg-error "10:cannot define" }
   };
 
-  void X::Y::Z::foo(auto) {}  // { dg-error "cannot define" }
+  void X::Y::Z::foo(auto) {}  // { dg-error "8:cannot define" }
 };
Index: testsuite/g++.dg/cpp0x/deleted13.C
===================================================================
--- testsuite/g++.dg/cpp0x/deleted13.C	(revision 271899)
+++ testsuite/g++.dg/cpp0x/deleted13.C	(working copy)
@@ -8,5 +8,5 @@  struct A
 
 struct B
 {
-  template<typename> friend void A::foo() = delete; // { dg-error "" }
+  template<typename> friend void A::foo() = delete; // { dg-error "34:cannot define" }
 };
Index: testsuite/g++.dg/other/friend4.C
===================================================================
--- testsuite/g++.dg/other/friend4.C	(revision 271899)
+++ testsuite/g++.dg/other/friend4.C	(working copy)
@@ -3,6 +3,6 @@ 
 
 struct A
 {
-  friend void A::foo();  // { dg-error "implicitly friends" }
-  friend A::~A();        // { dg-error "implicitly friends" }
+  friend void A::foo();  // { dg-error "15:member functions are implicitly friends" }
+  friend A::~A();        // { dg-error "10:member functions are implicitly friends" }
 };
Index: testsuite/g++.dg/other/friend5.C
===================================================================
--- testsuite/g++.dg/other/friend5.C	(revision 271899)
+++ testsuite/g++.dg/other/friend5.C	(working copy)
@@ -5,5 +5,5 @@ 
 
 struct A
 {
-  friend A::~A() {} /* { dg-error "implicitly friends of their class" } */
+  friend A::~A() {} /* { dg-error "10:member functions are implicitly friends of their class" } */
 };
Index: testsuite/g++.dg/other/friend7.C
===================================================================
--- testsuite/g++.dg/other/friend7.C	(revision 271899)
+++ testsuite/g++.dg/other/friend7.C	(working copy)
@@ -5,5 +5,5 @@ 
 
 struct A
 {
-  friend A::~A() {} // { dg-error "implicitly friends of their class" }
+  friend A::~A() {} // { dg-error "10:member functions are implicitly friends of their class" }
 };
Index: testsuite/g++.dg/parse/error29.C
===================================================================
--- testsuite/g++.dg/parse/error29.C	(revision 271899)
+++ testsuite/g++.dg/parse/error29.C	(working copy)
@@ -7,7 +7,7 @@  struct A {
   void operator delete(void *);
 };
 struct B { 
-  friend void A::foo() {} // { dg-error "22:cannot define member function 'A::foo' within 'B'" }
-  friend void A::operator delete(void*) {} // { dg-error "39:cannot define member function 'A::operator delete' within 'B'" }
-  friend A::A() {} // { dg-error "15:cannot define member function 'A::A' within 'B'" }
+  friend void A::foo() {} // { dg-error "15:cannot define member function 'A::foo' within 'B'" }
+  friend void A::operator delete(void*) {} // { dg-error "15:cannot define member function 'A::operator delete' within 'B'" }
+  friend A::A() {} // { dg-error "10:cannot define member function 'A::A' within 'B'" }
 };
Index: testsuite/g++.dg/parse/friend7.C
===================================================================
--- testsuite/g++.dg/parse/friend7.C	(revision 271899)
+++ testsuite/g++.dg/parse/friend7.C	(working copy)
@@ -32,7 +32,7 @@  struct D
 
 struct E
 {
-  friend A::A () {}		// { dg-error "cannot define member" }
-  friend A::~A () {}		// { dg-error "cannot define member" }
-  friend A::A (const A &) {}	// { dg-error "cannot define member" }
+  friend A::A () {}		// { dg-error "10:cannot define member" }
+  friend A::~A () {}		// { dg-error "10:cannot define member" }
+  friend A::A (const A &) {}	// { dg-error "10:cannot define member" }
 };
Index: testsuite/g++.dg/parse/qualified4.C
===================================================================
--- testsuite/g++.dg/parse/qualified4.C	(revision 271899)
+++ testsuite/g++.dg/parse/qualified4.C	(working copy)
@@ -2,5 +2,5 @@ 
 // { dg-options "" }
 
 struct X { 
-  void X::bar() {} // { dg-error "" }
+  void X::bar() {} // { dg-error "8:extra qualification" }
 }; 
Index: testsuite/g++.dg/template/crash96.C
===================================================================
--- testsuite/g++.dg/template/crash96.C	(revision 271899)
+++ testsuite/g++.dg/template/crash96.C	(working copy)
@@ -2,5 +2,5 @@ 
 
 template<int> struct A
 {
-  template<int> template<int> void A::foo() {} // { dg-error "extra qualification" }
+  template<int> template<int> void A::foo() {} // { dg-error "36:extra qualification" }
 };
Index: testsuite/g++.old-deja/g++.brendan/crash22.C
===================================================================
--- testsuite/g++.old-deja/g++.brendan/crash22.C	(revision 271899)
+++ testsuite/g++.old-deja/g++.brendan/crash22.C	(working copy)
@@ -6,6 +6,6 @@  struct A {
 };
 
 struct B {
-    void A::a1(); // this used to die in chainon(), now grokdeclarator should// { dg-error "" }  cannot declare.*
-    void A::a2(); // should be fixed by the 930629 change.// { dg-error "" }  cannot declare.*
+    void A::a1(); // this used to die in chainon(), now grokdeclarator should// { dg-error "10:cannot declare" }  cannot declare.*
+    void A::a2(); // should be fixed by the 930629 change.// { dg-error "10:cannot declare" }  cannot declare.*
 };
Index: testsuite/g++.old-deja/g++.brendan/crash23.C
===================================================================
--- testsuite/g++.old-deja/g++.brendan/crash23.C	(revision 271899)
+++ testsuite/g++.old-deja/g++.brendan/crash23.C	(working copy)
@@ -10,6 +10,6 @@  class A
     void f ();
     void g (int);
   };
-  void B::f () {}// { dg-error "" } .*
-  void B::g (int val) {}// { dg-error "" } .*
+  void B::f () {}// { dg-error "8:cannot define" } .*
+  void B::g (int val) {}// { dg-error "8:cannot define" } .*
 };
Index: testsuite/g++.old-deja/g++.law/visibility10.C
===================================================================
--- testsuite/g++.old-deja/g++.law/visibility10.C	(revision 271899)
+++ testsuite/g++.old-deja/g++.law/visibility10.C	(working copy)
@@ -10,7 +10,7 @@  class base {
 };
 
 class deriv : public base {
-  void base :: f1();// { dg-error "" } .*
+  void base :: f1();// { dg-error "8:cannot declare" } .*
 };
 
 int main ()
Index: testsuite/g++.old-deja/g++.other/decl5.C
===================================================================
--- testsuite/g++.old-deja/g++.other/decl5.C	(revision 271899)
+++ testsuite/g++.old-deja/g++.other/decl5.C	(working copy)
@@ -35,7 +35,7 @@  struct B {
   struct ::Q {        // { dg-error "global qual" } ::Q not a member of B
     int m;
   };
-  int A::fn() {       // { dg-error "cannot define member" } A::fn not a member of B
+  int A::fn() {       // { dg-error "7:cannot define member" } A::fn not a member of B
     return 0;
   }
   void fn(struct ::Q &);