diff mbox series

[v2] Capture source location of dtors (PR c++/89390)

Message ID 1550716412-45071-1-git-send-email-dmalcolm@redhat.com
State New
Headers show
Series [v2] Capture source location of dtors (PR c++/89390) | expand

Commit Message

David Malcolm Feb. 21, 2019, 2:33 a.m. UTC 
On Tue, 2019-02-19 at 20:37 +0100, Jakub Jelinek wrote:
> On Tue, Feb 19, 2019 at 02:44:55PM -0500, David Malcolm wrote:
> > How about something like this? (on top of Jakub's patch)
> 
> I had following queued for regtest, so if you want to go for the
> make_location ^~, you should change more spots.
> 
> > pr89390.C: In function 'void foo()':
> > pr89390.C:9:6: error: '~A' is not a member of 'A'
> >     9 |   A::~A ();    // { dg-error "6: '~A' is not a member of
> > 'A'" }
> >       |      ^~
> > pr89390.C: In function 'void test_2()':
> > pr89390.C:17:10: error: '~ns::P' is not a member of 'ns::P'
> >    17 |   ns::P::~P ();    // { dg-error "10: '~ns::P' is not a
> > member of 'ns::P'" }
> >       |          ^~
> > 
> > (Presumably gcc 10 material at this point; not yet bootstrapped).
> 
> I wouldn't say so, it actually is even a regression:
> $ /usr/src/gcc-6/obj/gcc/cc1plus -quiet pr89390.C 
> pr89390.C: In function ‘void foo()’:
> pr89390.C:9:3: error: ‘~A’ is not a member of ‘A’
>    A::~A (); // { dg-error "'~A' is not a member of 'A'" }
>    ^
> 
> $ /usr/src/gcc-7/obj/gcc/cc1plus -quiet pr89390.C 
> In function ‘void foo()’:
> cc1plus: error: ‘~A’ is not a member of ‘A’
> 
> Feel free to take this over though.
> 
> 2019-02-19  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/89390
> 	* parser.c (cp_parser_unqualified_id): For BIT_NOT_EXPR
> remember
> 	location of the ~ token and use it to build cp_expr.
> 
> 	* g++.dg/diagnostic/pr89390.C (foo): Expect diagnostics at the
> right
> 	line.

Thanks.

Here's an updated version of the patch which use make_location, and
merges in the changes from yours, and uses the loc in some other places
(adding test coverage for them); hope that's not stretching things
too far.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.

OK for trunk?

gcc/cp/ChangeLog:
	PR c++/89390
	* parser.c (cp_parser_unqualified_id): Capture and use locations
	for destructors.

gcc/testsuite/ChangeLog:
	PR c++/89390
	* g++.dg/diagnostic/pr89390.C: Update expected location of error,
	renaming to a multicharacter name, so that start != finish.  Add
	tests for dtor locations.
---
 gcc/cp/parser.c                           | 26 +++++++++++++------
 gcc/testsuite/g++.dg/diagnostic/pr89390.C | 42 +++++++++++++++++++++++++++++--
 2 files changed, 58 insertions(+), 10 deletions(-)

Comments

Jason Merrill Feb. 21, 2019, 6:03 a.m. UTC | #1 
On 2/20/19 4:33 PM, David Malcolm wrote:
> On Tue, 2019-02-19 at 20:37 +0100, Jakub Jelinek wrote:
>> On Tue, Feb 19, 2019 at 02:44:55PM -0500, David Malcolm wrote:
>>> How about something like this? (on top of Jakub's patch)
>>
>> I had following queued for regtest, so if you want to go for the
>> make_location ^~, you should change more spots.
>>
>>> pr89390.C: In function 'void foo()':
>>> pr89390.C:9:6: error: '~A' is not a member of 'A'
>>>      9 |   A::~A ();    // { dg-error "6: '~A' is not a member of
>>> 'A'" }
>>>        |      ^~
>>> pr89390.C: In function 'void test_2()':
>>> pr89390.C:17:10: error: '~ns::P' is not a member of 'ns::P'
>>>     17 |   ns::P::~P ();    // { dg-error "10: '~ns::P' is not a
>>> member of 'ns::P'" }
>>>        |          ^~
>>>
>>> (Presumably gcc 10 material at this point; not yet bootstrapped).
>>
>> I wouldn't say so, it actually is even a regression:
>> $ /usr/src/gcc-6/obj/gcc/cc1plus -quiet pr89390.C
>> pr89390.C: In function ‘void foo()’:
>> pr89390.C:9:3: error: ‘~A’ is not a member of ‘A’
>>     A::~A (); // { dg-error "'~A' is not a member of 'A'" }
>>     ^
>>
>> $ /usr/src/gcc-7/obj/gcc/cc1plus -quiet pr89390.C
>> In function ‘void foo()’:
>> cc1plus: error: ‘~A’ is not a member of ‘A’
>>
>> Feel free to take this over though.
>>
>> 2019-02-19  Jakub Jelinek  <jakub@redhat.com>
>>
>> 	PR c++/89390
>> 	* parser.c (cp_parser_unqualified_id): For BIT_NOT_EXPR
>> remember
>> 	location of the ~ token and use it to build cp_expr.
>>
>> 	* g++.dg/diagnostic/pr89390.C (foo): Expect diagnostics at the
>> right
>> 	line.
> 
> Thanks.
> 
> Here's an updated version of the patch which use make_location, and
> merges in the changes from yours, and uses the loc in some other places
> (adding test coverage for them); hope that's not stretching things
> too far.
> 
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
> 
> OK for trunk?

OK.

Jason
diff mbox series

Patch

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index ffecce4..f800360 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -5976,6 +5976,7 @@  cp_parser_unqualified_id (cp_parser* parser,
 	tree object_scope;
 	tree scope;
 	bool done;
+	location_t tilde_loc = token->location;
 
 	/* Consume the `~' token.  */
 	cp_lexer_consume_token (parser->lexer);
@@ -6038,9 +6039,18 @@  cp_parser_unqualified_id (cp_parser* parser,
 	  }
 	gcc_assert (!scope || TYPE_P (scope));
 
+	token = cp_lexer_peek_token (parser->lexer);
+
+	/* Create a location with caret == start at the tilde,
+	   finishing at the end of the peeked token, e.g:
+	   ~token
+	   ^~~~~~.  */
+	location_t loc
+	  = make_location (tilde_loc, tilde_loc, token->location);
+
 	/* If the name is of the form "X::~X" it's OK even if X is a
 	   typedef.  */
-	token = cp_lexer_peek_token (parser->lexer);
+
 	if (scope
 	    && token->type == CPP_NAME
 	    && (cp_lexer_peek_nth_token (parser->lexer, 2)->type
@@ -6050,18 +6060,18 @@  cp_parser_unqualified_id (cp_parser* parser,
 		    && constructor_name_p (token->u.value, scope))))
 	  {
 	    cp_lexer_consume_token (parser->lexer);
-	    return build_nt (BIT_NOT_EXPR, scope);
+	    return cp_expr (build_nt (BIT_NOT_EXPR, scope), loc);
 	  }
 
 	/* ~auto means the destructor of whatever the object is.  */
 	if (cp_parser_is_keyword (token, RID_AUTO))
 	  {
 	    if (cxx_dialect < cxx14)
-	      pedwarn (input_location, 0,
+	      pedwarn (loc, 0,
 		       "%<~auto%> only available with "
 		       "-std=c++14 or -std=gnu++14");
 	    cp_lexer_consume_token (parser->lexer);
-	    return build_nt (BIT_NOT_EXPR, make_auto ());
+	    return cp_expr (build_nt (BIT_NOT_EXPR, make_auto (), loc));
 	  }
 
 	/* If there was an explicit qualification (S::~T), first look
@@ -6152,7 +6162,7 @@  cp_parser_unqualified_id (cp_parser* parser,
 		type_decl = cp_parser_identifier (parser);
 		if (type_decl != error_mark_node)
 		  type_decl = build_nt (BIT_NOT_EXPR, type_decl);
-		return type_decl;
+		return cp_expr (type_decl, loc);
 	      }
 	  }
 	/* If an error occurred, assume that the name of the
@@ -6168,7 +6178,7 @@  cp_parser_unqualified_id (cp_parser* parser,
 	if (declarator_p && scope && !check_dtor_name (scope, type_decl))
 	  {
 	    if (!cp_parser_uncommitted_to_tentative_parse_p (parser))
-	      error_at (token->location,
+	      error_at (loc,
 			"declaration of %<~%T%> as member of %qT",
 			type_decl, scope);
 	    cp_parser_simulate_error (parser);
@@ -6183,11 +6193,11 @@  cp_parser_unqualified_id (cp_parser* parser,
 	    && !DECL_IMPLICIT_TYPEDEF_P (type_decl)
 	    && !DECL_SELF_REFERENCE_P (type_decl)
 	    && !cp_parser_uncommitted_to_tentative_parse_p (parser))
-	  error_at (token->location,
+	  error_at (loc,
 		    "typedef-name %qD used as destructor declarator",
 		    type_decl);
 
-	return build_nt (BIT_NOT_EXPR, TREE_TYPE (type_decl));
+	return cp_expr (build_nt (BIT_NOT_EXPR, TREE_TYPE (type_decl), loc));
       }
 
     case CPP_KEYWORD:
diff --git a/gcc/testsuite/g++.dg/diagnostic/pr89390.C b/gcc/testsuite/g++.dg/diagnostic/pr89390.C
index df35fcc..2e8c95a 100644
--- a/gcc/testsuite/g++.dg/diagnostic/pr89390.C
+++ b/gcc/testsuite/g++.dg/diagnostic/pr89390.C
@@ -1,10 +1,48 @@ 
 // PR c++/89390
 // { dg-do compile { target c++11 } }
+// { dg-options "-fdiagnostics-show-caret" }
 
-enum class A { B, C };
+enum class bar { A, B, C };
 
 void
 foo ()
 {
-  A::~A ();    // { dg-error "'~A' is not a member of 'A'" "" { target *-*-* } 0 }
+  bar::~bar ();    // { dg-error "8: '~bar' is not a member of 'bar'" }
+  /* { dg-begin-multiline-output "" }
+   bar::~bar ();
+        ^~~~
+     { dg-end-multiline-output "" } */
 }
+
+namespace ns { enum class baz { P, Q, R }; }
+
+void
+test_2 ()
+{
+  ns::baz::~baz ();    // { dg-error "12: '~ns::baz' is not a member of 'ns::baz'" }
+  /* { dg-begin-multiline-output "" }
+   ns::baz::~baz ();
+            ^~~~
+     { dg-end-multiline-output "" } */
+}
+
+struct first;
+struct second;
+second::~first() {} // { dg-error "9: declaration of '~first' as member of 'second'" }
+  /* { dg-begin-multiline-output "" }
+ second::~first() {}
+         ^~~~~~
+     { dg-end-multiline-output "" } */
+
+struct test { ~test(); };
+typedef test test_t;
+~test_t();  // { dg-error "typedef-name 'test_t' used as destructor declarator" }
+// { dg-error "expected" "" { target *-*-* } .-1 }
+  /* { dg-begin-multiline-output "" }
+ ~test_t();
+ ^~~~~~~
+     { dg-end-multiline-output "" } */
+  /* { dg-begin-multiline-output "" }
+ ~test_t();
+          ^
+     { dg-end-multiline-output "" } */