[PATCH 02/10 v2] Fix g++.dg/cpp0x/nsdmi-template14.C (v2)

[David Malcolm <dmalcolm at redhat dot com>]

On Thu, 2015-12-03 at 17:17 -0500, Jason Merrill wrote:
> On 12/03/2015 04:43 PM, David Malcolm wrote:
> > On Thu, 2015-12-03 at 15:33 -0500, Jason Merrill wrote:
> >> On 12/03/2015 09:55 AM, David Malcolm wrote:
> >>> This patch adds bulletproofing to detect purged tokens, and avoid using
> >>> them.
> >>>
> >>> Alternatively, is it OK to access purged tokens for this kind of thing?
> >>> If so, would it make more sense to instead leave their locations untouched
> >>> when purging them?
> >>
> >> I think cp_lexer_previous_token should skip past purged tokens.
> >
> > Sorry if this is a silly question, but should I limit the iteration e.g.
> > by detecting a sentinel value?  e.g.
> >    parser->lexer->buffer->address () ?
> >
> > Or is there guaranteed to be an unpurged token somewhere beforehand?
> 
> There should always be an unpurged token.

Thanks.

> > Out of interest, the prior tokens here are:
> >
> > (gdb) p end_tok[0]
> > $25 = {type = CPP_GREATER, keyword = RID_MAX, flags = 0 '\000',
> > pragma_kind = PRAGMA_NONE, implicit_extern_c = 0,
> >    error_reported = 0, purged_p = 1, location = 0, u = {tree_check_value
> > = 0x0, value = <tree 0x0>}}
> >
> > (gdb) p end_tok[-1]
> > $26 = {type = CPP_NAME, keyword = RID_MAX, flags = 0 '\000', pragma_kind
> > = PRAGMA_NONE, implicit_extern_c = 0,
> >    error_reported = 0, purged_p = 1, location = 0, u = {tree_check_value
> > = 0x0, value = <tree 0x0>}}
> >
> > (gdb) p end_tok[-2]
> > $27 = {type = CPP_LESS, keyword = RID_MAX, flags = 0 '\000', pragma_kind
> > = PRAGMA_NONE, implicit_extern_c = 0,
> >    error_reported = 0, purged_p = 1, location = 0, u = {tree_check_value
> > = 0x0, value = <tree 0x0>}}
> >
> > (gdb) p end_tok[-3]
> > $28 = {type = 86, keyword = RID_MAX, flags = 1 '\001', pragma_kind =
> > PRAGMA_NONE, implicit_extern_c = 0, error_reported = 0,
> >    purged_p = 0, location = 202016, u = {tree_check_value =
> > 0x7ffff19dfd98, value = <bdver1-direct,bdver1-agu 0x7ffff19dfd98>}}
> >
> > (gdb) p end_tok[-4]
> > $29 = {type = CPP_KEYWORD, keyword = RID_NEW, flags = 1 '\001',
> > pragma_kind = PRAGMA_NONE, implicit_extern_c = 0,
> >    error_reported = 0, purged_p = 0, location = 201890, u =
> > {tree_check_value = 0x7ffff18a8318,
> >      value = <identifier_node 0x7ffff18a8318 new>}}
> >
> > where the previous unpurged token is:
> >
> > (gdb) p end_tok[-3].purged_p
> > $31 = 0
> >
> > (gdb) call inform (end_tok[-3].location, "")
> > ../../src/gcc/testsuite/g++.dg/cpp0x/nsdmi-template14.C:11:14: note:
> >     B* p = new B<N>;
> >                ^
> >
> > which would give a range of:
> >
> >     B* p = new B<N>;
> >            ^~~~~
> >
> > for the erroneous new expression, rather than:
> >
> >
> >     B* p = new B<N>;
> >            ^~~~~~~~
> >
> > if we used the location of the purged token (the CPP_GREATER).
> > I prefer the latter, hence my suggestion about not zero-ing out the
> > locations of tokens when purging them.
> 
> The unpurged token you're finding is the artificial CPP_TEMPLATE_ID 
> token, which seems to need to have its location adjusted to reflect the 
> full range of the template-id.

Aha!  Thanks.

Here's an updated fix for g++.dg/cpp0x/nsdmi-template14.C,
which generates meaningful locations/ranges when converting tokens
to CPP_TEMPLATE_ID:

(gdb) call inform (end_tok[-3].location, "")
../../src/gcc/testsuite/g++.dg/cpp0x/nsdmi-template14.C:11:14: note:
   B* p = new B<N>; // { dg-error "recursive instantiation of non-static data" }
              ^~~~

I added a test case for this to
g++.dg/plugin/diagnostic-test-expressions-1.C.

The updated patch also updates cp_lexer_previous_token to skip
past purged tokens, so that we use the above token when determining
the end of the new-expression, giving:

../../src/gcc/testsuite/g++.dg/cpp0x/nsdmi-template14.C:11:10: error: recursive instantiation of non-static data member initializer for âB<1>::pâ
   B* p = new B<N>; // { dg-error "recursive instantiation of non-static data" }
          ^~~~~~~~

and hence we no longer need the "bulletproofing" from the previous
iteration of the patch.

As before, the patch also updates the location of a dg-error directive
in the testcase to reflect improved location information.

Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu (in
combination with the other patches from the kit).

gcc/cp/ChangeLog:
	* parser.c (cp_lexer_previous_token): Skip past purged tokens.
	(cp_parser_template_id): When converting a token to
	CPP_TEMPLATE_ID, update the location.

gcc/testsuite/ChangeLog:
	* g++.dg/cpp0x/nsdmi-template14.C: Move dg-error directive.
	* g++.dg/plugin/diagnostic-test-expressions-1.C
	(test_template_id): New function.
---
 gcc/cp/parser.c                                       | 19 +++++++++++++++++++
 gcc/testsuite/g++.dg/cpp0x/nsdmi-template14.C         |  4 ++--
 .../g++.dg/plugin/diagnostic-test-expressions-1.C     |  9 +++++++++
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index d859a89..1e3ada5 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -733,6 +733,13 @@ cp_lexer_previous_token (cp_lexer *lexer)
 {
   cp_token_position tp = cp_lexer_previous_token_position (lexer);
 
+  /* Skip past purged tokens.  */
+  while (tp->purged_p)
+    {
+      gcc_assert (tp != lexer->buffer->address ());
+      tp--;
+    }
+
   return cp_lexer_token_at (lexer, tp);
 }
 
@@ -14733,6 +14740,18 @@ cp_parser_template_id (cp_parser *parser,
 
       /* Reset the contents of the START_OF_ID token.  */
       token->type = CPP_TEMPLATE_ID;
+
+      /* Update the location to be of the form:
+	   template-name < template-argument-list [opt] >
+	   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+	 with caret == start at the start of the template-name,
+	 ranging until the closing '>'.  */
+      location_t finish_loc
+	= get_finish (cp_lexer_previous_token (parser->lexer)->location);
+      location_t combined_loc
+	= make_location (token->location, token->location, finish_loc);
+      token->location = combined_loc;
+
       /* Retrieve any deferred checks.  Do not pop this access checks yet
 	 so the memory will not be reclaimed during token replacing below.  */
       token->u.tree_check_value = ggc_cleared_alloc<struct tree_check> ();
diff --git a/gcc/testsuite/g++.dg/cpp0x/nsdmi-template14.C b/gcc/testsuite/g++.dg/cpp0x/nsdmi-template14.C
index 9cb01f1..47f5b63 100644
--- a/gcc/testsuite/g++.dg/cpp0x/nsdmi-template14.C
+++ b/gcc/testsuite/g++.dg/cpp0x/nsdmi-template14.C
@@ -8,10 +8,10 @@ template<int> struct A // { dg-error "has been parsed" }
 
 template<int N> struct B
 {
-  B* p = new B<N>;
+  B* p = new B<N>; // { dg-error "recursive instantiation of non-static data" }
 };
 
-B<1> x; // { dg-error "recursive instantiation of non-static data" }
+B<1> x;
 
 struct C
 {
diff --git a/gcc/testsuite/g++.dg/plugin/diagnostic-test-expressions-1.C b/gcc/testsuite/g++.dg/plugin/diagnostic-test-expressions-1.C
index 826e835..4d3b07c 100644
--- a/gcc/testsuite/g++.dg/plugin/diagnostic-test-expressions-1.C
+++ b/gcc/testsuite/g++.dg/plugin/diagnostic-test-expressions-1.C
@@ -697,6 +697,15 @@ public:
   example_template (TYPENAME v);
 };
 
+void test_template_id (void)
+{
+  example_template <int>; /* { dg-warning "declaration does not declare anything" } */
+/* { dg-begin-multiline-output "" }
+   example_template <int>;
+   ^~~~~~~~~~~~~~~~~~~~~~
+   { dg-end-multiline-output "" } */
+}
+
 void test_new (void)
 {
   __emit_expression_range (0, ::new base); /* { dg-warning "range" } */
-- 
1.8.5.3