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®rtested 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
