Re: [C++ Patch] Use declarator->id_loc in three additional places
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
Re: [C++ Patch] Use declarator->id_loc in three additional places
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.
Re: [C++ Patch] Use declarator->id_loc in three additional places
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
Re: [C++ Patch] Use declarator->id_loc in three additional places
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.
Re: [C++ Patch] Use declarator->id_loc in three additional places
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
[C++ Patch] Use declarator->id_loc in three additional places
Hi,
tested x86_64-linux, as usual.
Thanks, Paolo.
///
/cp
2019-06-04 Paolo Carlini
* decl.c (grokdeclarator): Use declarator->id_loc in three
additional places.
/testsuite
2019-06-04 Paolo Carlini
* 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.
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 friend void A::foo() = delete; // { dg-error "" }
+ template friend void A::foo() = delete; // { dg-error "34:cannot
define" }
};
Index: testsuite/g++.dg/other/friend4.C
===
--- testsuite/g++.dg/other/frie
