Re: [PATCH] c++: error message for dependent template members [PR70417]
On Sat, Jan 15, 2022 at 3:28 AM Anthony Sharp
wrote:
> Hi Jason,
>
> Hope you are well. Apologies, I've not had time to sit down and look at
> this since last month I quit my old job, then I had family around for the
> whole of the Christmas period, and then even more recently I've had to
> start my new job.
>
> In any case happy that you managed to figure it all out. I noticed the
> small regression at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104025.
> To be honest I wouldn't even know how to begin to go about fixing that so
> perhaps it's best if I leave that to you? I guess it's not playing well
> with the use of warn_missing_template_keyword. Maybe I didn't set that
> variable up properly or something.
>
FYI it seems to be a latent bug that cp_rollback_tokens doesn't also roll
back input_location, not a bug in the new code.
> Kind regards,
> Anthony
>
> On Thu, 13 Jan 2022 at 21:01, Jason Merrill wrote:
>
>> On 12/9/21 10:51, Jason Merrill wrote:
>> > On 12/4/21 12:23, Anthony Sharp wrote:
>> >> Hi Jason,
>> >>
>> >> Hope you are well. Apologies for not coming back sooner.
>> >>
>> >> >I'd put it just above the definition of saved_token_sentinel in
>> >> parser.c.
>> >>
>> >> Sounds good, done.
>> >>
>> >> >Maybe cp_parser_require_end_of_template_parameter_list? Either way
>> >> is fine.
>> >>
>> >> Even better, have changed it.
>> >>
>> >> >Hmm, good point; operators that are member functions must be
>> >> non-static,
>> >> >so we couldn't be doing a comparison of the address of the function.
>> >>
>> >> In that case I have set it to return early there.
>> >>
>> >> >So declarator_p should be true there. I'll fix that.
>> >>
>> >> Thank you.
>> >>
>> >> >> + if (next_token->keyword == RID_TEMPLATE)
>> >> >> +{
>> >> >> + /* But at least make sure it's properly formed (e.g. see
>> >> PR19397). */
>> >> >> + if (cp_lexer_peek_nth_token (parser->lexer, 2)->type ==
>> >> CPP_NAME)
>> >> >> + return 1;
>> >> >> +
>> >> >> + return -1;
>> >> >> +}
>> >> >> +
>> >> >> + /* Could be a ~ referencing the destructor of a class template.
>> >> */
>> >> >> + if (next_token->type == CPP_COMPL)
>> >> >> +{
>> >> >> + /* It could only be a template. */
>> >> >> + if (cp_lexer_peek_nth_token (parser->lexer, 2)->type ==
>> >> CPP_NAME)
>> >> >> + return 1;
>> >> >> +
>> >> >> + return -1;
>> >> >> +}
>> >> >
>> >> >Why don't these check for the < ?
>> >>
>> >> I think perhaps I could have named the function better; instead of
>> >> next_token_begins_template_id, how's about
>> >> next_token_begins_template_name?
>> >> That's all I intended to check for.
>> >
>> > You're using it to control whether we try to parse a template-id, and
>> > it's used to initialize variables named looks_like_template_id, so I
>> > think better to keep the old name.
>> >
>> >> In the first case, something like "->template some_name" will always be
>> >> intended as a template, so no need to check for the <. If there were
>> >> default
>> >> template arguments you could also validly omit the <> completely, I
>> think
>> >> (could be wrong).
>> >
>> > Or if the template arguments can be deduced, yes:
>> >
>> > template struct A
>> > {
>> >template void f(U u);
>> > };
>> >
>> > template void g(A a)
>> > {
>> >a->template f(42);
>> > }
>> >
>> > But 'f' is still not a template-id.
>> >
>> > ...
>> >
>> > Actually, it occurs to me that you might be better off handling this in
>> > cp_parser_template_name, something like the below, to avoid the complex
>> > duplicate logic in the id-expression handling.
>> >
>> > Note that in this patch I'm using "any_object_scope" as a proxy for
>> "I'm
>> > parsing an expression", since !is_declaration doesn't work for that; as
>> > a result, this doesn't handle the static member function template case.
>> > For that you'd probably still need to pass down a flag so that
>> > cp_parser_template_name knows it's being called from
>> > cp_parser_id_expression.
>> >
>> > Your patch has a false positive on
>> >
>> > template struct A { };
>> > template void f()
>> > {
>> >A();
>> > };
>> >
>> > which my patch checks in_template_argument_list_p to avoid, though
>> > checking any_object_scope also currently avoids it.
>> >
>> > What do you think?
>>
>> I decided that it made more sense to keep the check in
>> cp_parser_id_expression like you had it, but I moved it to the end to
>> simplify the logic. Here's what I'm applying, thanks!
>
>
Re: [PATCH] c++: error message for dependent template members [PR70417]
Hi Jason,
Hope you are well. Apologies, I've not had time to sit down and look at
this since last month I quit my old job, then I had family around for the
whole of the Christmas period, and then even more recently I've had to
start my new job.
In any case happy that you managed to figure it all out. I noticed the
small regression at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104025. To
be honest I wouldn't even know how to begin to go about fixing that so
perhaps it's best if I leave that to you? I guess it's not playing well
with the use of warn_missing_template_keyword. Maybe I didn't set that
variable up properly or something.
Kind regards,
Anthony
On Thu, 13 Jan 2022 at 21:01, Jason Merrill wrote:
> On 12/9/21 10:51, Jason Merrill wrote:
> > On 12/4/21 12:23, Anthony Sharp wrote:
> >> Hi Jason,
> >>
> >> Hope you are well. Apologies for not coming back sooner.
> >>
> >> >I'd put it just above the definition of saved_token_sentinel in
> >> parser.c.
> >>
> >> Sounds good, done.
> >>
> >> >Maybe cp_parser_require_end_of_template_parameter_list? Either way
> >> is fine.
> >>
> >> Even better, have changed it.
> >>
> >> >Hmm, good point; operators that are member functions must be
> >> non-static,
> >> >so we couldn't be doing a comparison of the address of the function.
> >>
> >> In that case I have set it to return early there.
> >>
> >> >So declarator_p should be true there. I'll fix that.
> >>
> >> Thank you.
> >>
> >> >> + if (next_token->keyword == RID_TEMPLATE)
> >> >> +{
> >> >> + /* But at least make sure it's properly formed (e.g. see
> >> PR19397). */
> >> >> + if (cp_lexer_peek_nth_token (parser->lexer, 2)->type ==
> >> CPP_NAME)
> >> >> + return 1;
> >> >> +
> >> >> + return -1;
> >> >> +}
> >> >> +
> >> >> + /* Could be a ~ referencing the destructor of a class template.
> >> */
> >> >> + if (next_token->type == CPP_COMPL)
> >> >> +{
> >> >> + /* It could only be a template. */
> >> >> + if (cp_lexer_peek_nth_token (parser->lexer, 2)->type ==
> >> CPP_NAME)
> >> >> + return 1;
> >> >> +
> >> >> + return -1;
> >> >> +}
> >> >
> >> >Why don't these check for the < ?
> >>
> >> I think perhaps I could have named the function better; instead of
> >> next_token_begins_template_id, how's about
> >> next_token_begins_template_name?
> >> That's all I intended to check for.
> >
> > You're using it to control whether we try to parse a template-id, and
> > it's used to initialize variables named looks_like_template_id, so I
> > think better to keep the old name.
> >
> >> In the first case, something like "->template some_name" will always be
> >> intended as a template, so no need to check for the <. If there were
> >> default
> >> template arguments you could also validly omit the <> completely, I
> think
> >> (could be wrong).
> >
> > Or if the template arguments can be deduced, yes:
> >
> > template struct A
> > {
> >template void f(U u);
> > };
> >
> > template void g(A a)
> > {
> >a->template f(42);
> > }
> >
> > But 'f' is still not a template-id.
> >
> > ...
> >
> > Actually, it occurs to me that you might be better off handling this in
> > cp_parser_template_name, something like the below, to avoid the complex
> > duplicate logic in the id-expression handling.
> >
> > Note that in this patch I'm using "any_object_scope" as a proxy for "I'm
> > parsing an expression", since !is_declaration doesn't work for that; as
> > a result, this doesn't handle the static member function template case.
> > For that you'd probably still need to pass down a flag so that
> > cp_parser_template_name knows it's being called from
> > cp_parser_id_expression.
> >
> > Your patch has a false positive on
> >
> > template struct A { };
> > template void f()
> > {
> >A();
> > };
> >
> > which my patch checks in_template_argument_list_p to avoid, though
> > checking any_object_scope also currently avoids it.
> >
> > What do you think?
>
> I decided that it made more sense to keep the check in
> cp_parser_id_expression like you had it, but I moved it to the end to
> simplify the logic. Here's what I'm applying, thanks!
Re: [PATCH] c++: error message for dependent template members [PR70417]
On 12/9/21 10:51, Jason Merrill wrote:
On 12/4/21 12:23, Anthony Sharp wrote:
Hi Jason,
Hope you are well. Apologies for not coming back sooner.
>I'd put it just above the definition of saved_token_sentinel in
parser.c.
Sounds good, done.
>Maybe cp_parser_require_end_of_template_parameter_list? Either way
is fine.
Even better, have changed it.
>Hmm, good point; operators that are member functions must be
non-static,
>so we couldn't be doing a comparison of the address of the function.
In that case I have set it to return early there.
>So declarator_p should be true there. I'll fix that.
Thank you.
>> + if (next_token->keyword == RID_TEMPLATE)
>> + {
>> + /* But at least make sure it's properly formed (e.g. see
PR19397). */
>> + if (cp_lexer_peek_nth_token (parser->lexer, 2)->type ==
CPP_NAME)
>> + return 1;
>> +
>> + return -1;
>> + }
>> +
>> + /* Could be a ~ referencing the destructor of a class template.
*/
>> + if (next_token->type == CPP_COMPL)
>> + {
>> + /* It could only be a template. */
>> + if (cp_lexer_peek_nth_token (parser->lexer, 2)->type ==
CPP_NAME)
>> + return 1;
>> +
>> + return -1;
>> + }
>
>Why don't these check for the < ?
I think perhaps I could have named the function better; instead of
next_token_begins_template_id, how's about
next_token_begins_template_name?
That's all I intended to check for.
You're using it to control whether we try to parse a template-id, and
it's used to initialize variables named looks_like_template_id, so I
think better to keep the old name.
In the first case, something like "->template some_name" will always be
intended as a template, so no need to check for the <. If there were
default
template arguments you could also validly omit the <> completely, I think
(could be wrong).
Or if the template arguments can be deduced, yes:
template struct A
{
template void f(U u);
};
template void g(A a)
{
a->template f(42);
}
But 'f' is still not a template-id.
...
Actually, it occurs to me that you might be better off handling this in
cp_parser_template_name, something like the below, to avoid the complex
duplicate logic in the id-expression handling.
Note that in this patch I'm using "any_object_scope" as a proxy for "I'm
parsing an expression", since !is_declaration doesn't work for that; as
a result, this doesn't handle the static member function template case.
For that you'd probably still need to pass down a flag so that
cp_parser_template_name knows it's being called from
cp_parser_id_expression.
Your patch has a false positive on
template struct A { };
template void f()
{
A();
};
which my patch checks in_template_argument_list_p to avoid, though
checking any_object_scope also currently avoids it.
What do you think?
I decided that it made more sense to keep the check in
cp_parser_id_expression like you had it, but I moved it to the end to
simplify the logic. Here's what I'm applying, thanks!From 1978f05716133b934de0fca7c3d64089b62e3e78 Mon Sep 17 00:00:00 2001
From: Anthony Sharp
Date: Sat, 4 Dec 2021 17:23:22 +
Subject: [PATCH] c++: warning for dependent template members [PR70417]
To: gcc-patches@gcc.gnu.org
Add a helpful warning message for when the user forgets to
include the "template" keyword after ., -> or :: when
accessing a member in a dependent context, where the member is a
template.
PR c++/70417
gcc/c-family/ChangeLog:
* c.opt: Added -Wmissing-template-keyword.
gcc/cp/ChangeLog:
* parser.c (cp_parser_id_expression): Handle
-Wmissing-template-keyword.
(struct saved_token_sentinel): Add modes to control what happens
on destruction.
(cp_parser_statement): Adjust.
(cp_parser_skip_entire_template_parameter_list): New function that
skips an entire template parameter list.
(cp_parser_require_end_of_template_parameter_list): Rename old
cp_parser_skip_to_end_of_template_parameter_list.
(cp_parser_skip_to_end_of_template_parameter_list): Refactor to be
called from one of the above two functions.
(cp_parser_lambda_declarator_opt)
(cp_parser_explicit_template_declaration)
(cp_parser_enclosed_template_argument_list): Adjust.
gcc/ChangeLog:
* doc/invoke.texi: Documentation for Wmissing-template-keyword.
gcc/testsuite/ChangeLog:
* g++.dg/cpp0x/variadic-mem_fn2.C: Catch warning about missing
template keyword.
* g++.dg/template/dependent-name17.C: New test.
* g++.dg/template/dependent-name18.C: New test.
Co-authored-by: Jason Merrill
---
gcc/doc/invoke.texi | 33
gcc/c-family/c.opt| 4 +
gcc/cp/parser.c | 178 +-
gcc/testsuite/g++.dg/cpp0x/variadic-mem_fn2.C | 1 +
.../g++.dg/template/dependent-name17.C| 49 +
.../g++.dg/template/dependent-name18.C| 5 +
6 files changed, 223 insertions(+), 47 deletions(-)
create mode 100644
Re: [PATCH] c++: error message for dependent template members [PR70417]
On 12/4/21 12:23, Anthony Sharp wrote:
Hi Jason,
Hope you are well. Apologies for not coming back sooner.
>I'd put it just above the definition of saved_token_sentinel in parser.c.
Sounds good, done.
>Maybe cp_parser_require_end_of_template_parameter_list? Either way is
fine.
Even better, have changed it.
>Hmm, good point; operators that are member functions must be non-static,
>so we couldn't be doing a comparison of the address of the function.
In that case I have set it to return early there.
>So declarator_p should be true there. I'll fix that.
Thank you.
>> + if (next_token->keyword == RID_TEMPLATE)
>> + {
>> + /* But at least make sure it's properly formed (e.g. see
PR19397). */
>> + if (cp_lexer_peek_nth_token (parser->lexer, 2)->type == CPP_NAME)
>> + return 1;
>> +
>> + return -1;
>> + }
>> +
>> + /* Could be a ~ referencing the destructor of a class template. */
>> + if (next_token->type == CPP_COMPL)
>> + {
>> + /* It could only be a template. */
>> + if (cp_lexer_peek_nth_token (parser->lexer, 2)->type == CPP_NAME)
>> + return 1;
>> +
>> + return -1;
>> + }
>
>Why don't these check for the < ?
I think perhaps I could have named the function better; instead of
next_token_begins_template_id, how's about next_token_begins_template_name?
That's all I intended to check for.
You're using it to control whether we try to parse a template-id, and
it's used to initialize variables named looks_like_template_id, so I
think better to keep the old name.
In the first case, something like "->template some_name" will always be
intended as a template, so no need to check for the <. If there were default
template arguments you could also validly omit the <> completely, I think
(could be wrong).
Or if the template arguments can be deduced, yes:
template struct A
{
template void f(U u);
};
template void g(A a)
{
a->template f(42);
}
But 'f' is still not a template-id.
...
Actually, it occurs to me that you might be better off handling this in
cp_parser_template_name, something like the below, to avoid the complex
duplicate logic in the id-expression handling.
Note that in this patch I'm using "any_object_scope" as a proxy for "I'm
parsing an expression", since !is_declaration doesn't work for that; as
a result, this doesn't handle the static member function template case.
For that you'd probably still need to pass down a flag so that
cp_parser_template_name knows it's being called from
cp_parser_id_expression.
Your patch has a false positive on
template struct A { };
template void f()
{
A();
};
which my patch checks in_template_argument_list_p to avoid, though
checking any_object_scope also currently avoids it.
What do you think?
JasonFrom 3022a600514f8adf8706fd286387a5f9f34c06ba Mon Sep 17 00:00:00 2001
From: Jason Merrill
Date: Wed, 8 Dec 2021 17:24:21 -0500
Subject: [PATCH] handle in cp_parser_template_name
To: gcc-patches@gcc.gnu.org
---
gcc/cp/parser.c | 41 -
1 file changed, 36 insertions(+), 5 deletions(-)
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index ed0bae33fc1..6ec9da646cf 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -18546,6 +18546,8 @@ cp_parser_template_name (cp_parser* parser,
/* cp_parser_lookup_name clears OBJECT_TYPE. */
tree scope = (parser->scope ? parser->scope
: parser->context->object_type);
+ bool any_object_scope = (parser->context->object_type
+ || parser->object_scope);
/* Look up the name. */
decl = cp_parser_lookup_name (parser, identifier,
@@ -18625,12 +18627,41 @@ cp_parser_template_name (cp_parser* parser,
found = true;
}
- /* "in a type-only context" */
if (!found && scope
- && tag_type != none_type
- && dependentish_scope_p (scope)
- && cp_parser_nth_token_starts_template_argument_list_p (parser, 1))
- found = true;
+ && processing_template_decl
+ && cp_parser_nth_token_starts_template_argument_list_p (parser, 1)
+ && dependentish_scope_p (scope))
+ {
+ /* "in a type-only context" */
+ if (tag_type != none_type)
+ found = true;
+ else if (warn_missing_template_keyword
+ /* If there's an object scope we know we're in an
+ expression, not a declarator-id. FIXME is_declaration
+ ought to make that distinction for us. */
+ && any_object_scope
+ /* In a template argument list a > could be closing
+ the enclosing targs. */
+ && !parser->in_template_argument_list_p
+ && !token->error_reported
+ && warning_enabled_at (token->location,
+ OPT_Wmissing_template_keyword))
+ {
+ saved_token_sentinel toks (parser->lexer, STS_ROLLBACK);
+ if (cp_parser_skip_entire_template_parameter_list (parser)
+ /* An operator after the > suggests that the > ends a
+ template-id; a name or literal suggests that the > is an
+ operator. */
+ &&
Re: [PATCH] c++: error message for dependent template members [PR70417]
Hi Jason,
Hope you are well. Apologies for not coming back sooner.
>I'd put it just above the definition of saved_token_sentinel in parser.c.
Sounds good, done.
>Maybe cp_parser_require_end_of_template_parameter_list? Either way is
fine.
Even better, have changed it.
>Hmm, good point; operators that are member functions must be non-static,
>so we couldn't be doing a comparison of the address of the function.
In that case I have set it to return early there.
>So declarator_p should be true there. I'll fix that.
Thank you.
>> + if (next_token->keyword == RID_TEMPLATE)
>> +{
>> + /* But at least make sure it's properly formed (e.g. see
PR19397). */
>> + if (cp_lexer_peek_nth_token (parser->lexer, 2)->type == CPP_NAME)
>> + return 1;
>> +
>> + return -1;
>> +}
>> +
>> + /* Could be a ~ referencing the destructor of a class template. */
>> + if (next_token->type == CPP_COMPL)
>> +{
>> + /* It could only be a template. */
>> + if (cp_lexer_peek_nth_token (parser->lexer, 2)->type == CPP_NAME)
>> + return 1;
>> +
>> + return -1;
>> +}
>
>Why don't these check for the < ?
I think perhaps I could have named the function better; instead of
next_token_begins_template_id, how's about next_token_begins_template_name?
That's all I intended to check for.
In the first case, something like "->template some_name" will always be
intended as a template, so no need to check for the <. If there were default
template arguments you could also validly omit the <> completely, I think
(could be wrong).
In the second case I think you are correct. I have added a check for the
"<".
>> + /* E.g. "::operator- <>". */
>> + if (next_token->keyword == RID_OPERATOR)
>> +{
>> + /* Consume it so it doesn't get in our way. */
>> + cp_lexer_consume_token (parser->lexer);
>> + next_token = cp_lexer_peek_token (parser->lexer);
>> + found_operator_keyword = true;
>> +}
>> +
>> + if (next_token->type == CPP_TEMPLATE_ID)
>> + return 1;
>> +
>> + /* By now the next token should be a name/operator and the one after
that
>> + should be a "<". */
>> + if (!cp_parser_nth_token_starts_template_argument_list_p (parser, 2))
>> +return -1;
>> +
>> + if (!found_operator_keyword && next_token->type != CPP_NAME)
>> +return -1;
>
>These could be if/else if so you don't need the found_operator_keyword
>variable.
Hmm yes I think so. But that's been refactored now anyways; it returns if
it sees
RID_OPERATOR.
>> + if (next_token->type == CPP_TEMPLATE_ID)
>> +return 1;
>
>This could move above the saved_token_sentinel; you won't have a
>CPP_TEMPLATE_ID after RID_OPERATOR.
Yes thanks, done.
>> + /* If this is a function template then we would see a "(" after the
>> + final ">". It could also be a class template constructor. */
>> + if (next_token->type == CPP_OPEN_PAREN
>> + /* If this is a class template then we could see a scope token
after
>> + the final ">". */
>> + || next_token->type == CPP_SCOPE
>> + /* We could see a ";" after a variable template. */
>> + || next_token->type == CPP_SEMICOLON
>> + /* We could see something like
>> +friend vect (::operator- <>)( const vect&, const vect&
);
>> +*/
>> + || next_token->type == CPP_CLOSE_PAREN)
>> +return 1;
>
>This seems too limited. As I was saying before,
>
>> I guess any operator-or-punctuator after the > would suggest a
>> template-id.
>
>For instance, the patch doesn't currently warn about
>
>template struct A
>{
> template static U m;
>};
>
>template int f (T t)
>{
> return t.m + 42;
>}
>
>This is especially a problem since we use the return value of -1 to skip
>calling cp_parser_template_id_expr.
Maybe a better approach then as you hinted at before would be to check for
what
we would not expect to see (i.e. a name), rather than what we would. I've
updated it. Seems to work?
>> +++ b/gcc/testsuite/g++.dg/cpp0x/decltype34.C
>> @@ -7,13 +7,13 @@ struct impl
>> };
>>
>> template> -class = decltype(impl::create()->impl::create())>
>> +class = decltype(impl::create()->impl::template create())>
>
>As I was saying in earlier discussion, this is currently a false
>positive, because impl:: is a non-dependent scope. If parser->scope is
>set, we don't need to look at parser->context->object_type...
>
>> + && (dependent_scope_p (parser->context->object_type)
>> + /* :: access expressions. */
>> + || (dependent_scope_p (parser->scope)
>
>...here.
Ah yes I see now. Sorry, I misunderstood what you had said before. I
thought you
were saying that there is no need to check whether the entire expression is
dependent, merely the class we are trying to access (although my hunch is
that
if any part of an expression is dependent, the whole thing must be). But yes
that regression test confused me quite a bit, having read the issue that it
was trying to solve it makes a lot
Re: [PATCH] c++: error message for dependent template members [PR70417]
On 10/10/21 07:28, Anthony Sharp wrote:
Hi Jason,
Hope you are well. Thanks for the feedback.
I like adding the configurability, but I think let's keep committing as
the default behavior. And adding the parameter to the rollback function
seems unnecessary. For the behavior argument, let's use an enum to be
clearer.
Sure, all done. The declaration of the enum could have gone in many
places it seems but I put it in cp-tree.h.
I'd put it just above the definition of saved_token_sentinel in parser.c.
I've removed the awkward call to cp_parser_require and put it (along with the
call to cp_parser_skip_to_end_of_template_parameter_list) in its own new
function called cp_parser_ensure_reached_end_of_template_parameter_list.
Maybe cp_parser_require_end_of_template_parameter_list? Either way is fine.
I think we don't want to return early here, we want to go through the
same ( check that we do for regular names.
I have changed it to do that, but could something like "operator- <"
ever be intended as something other than the start of a template-id?
Hmm, good point; operators that are member functions must be non-static,
so we couldn't be doing a comparison of the address of the function.
+// { dg-warning "expected \"template\" keyword before dependent template name"
{ target c++20 } .-1 }
+// ^ bogus warning caused by the above being treated as an expression, not a
declaration >
Hmm, that's a problem. Can you avoid it by checking declarator_p?
We actually already check declarator_p in cp_parser_id_expression in
that case. The reason it throws a warning is because typename14.C is
intentionally malformed; in the eyes of the compiler it's written like
an expression because it's missing the return type (although, even
adding a return type would not make it valid). I'm not sure there's any
worthwhile way around this really.
But it isn't written like an expression: the error comes when trying to
diagnose it as an invalid type in a declaration context.
So declarator_p should be true there. I'll fix that.
Also, some more missing template keywords seemed to crop up in the
regression tests, so I had to sprinkle some more template keywords in a
few. I guess there was a hidden bug that was missing a few scenarios.
Just out of curiosity, how pedantic would you say I should be when
submitting patches to GCC etc? I regression test and bootstrap all my
changes, but I'm always worrying about what might happen if I somehow
forgot to stage a file in git, or attached the wrong patch file, or
interpreted the .sum file wrong etc. Do patches go through another round
of testing after submitting?
Not automatically (yet), but often I test other people's patches before
applying them.
Sometimes I wonder whether I should be
applying the patch locally and then bootstrapping and regression testing
it again, although that's hardly fun since that usually takes around 2-3
hours even with -j 12.
Maybe I ought to think about getting a dedicated Linux PC.
You could also apply for a GCC Compile Farm account:
https://gcc.gnu.org/wiki/CompileFarm
+ if (next_token->keyword == RID_TEMPLATE)
+{
+ /* But at least make sure it's properly formed (e.g. see PR19397). */
+ if (cp_lexer_peek_nth_token (parser->lexer, 2)->type == CPP_NAME)
+ return 1;
+
+ return -1;
+}
+
+ /* Could be a ~ referencing the destructor of a class template. */
+ if (next_token->type == CPP_COMPL)
+{
+ /* It could only be a template. */
+ if (cp_lexer_peek_nth_token (parser->lexer, 2)->type == CPP_NAME)
+ return 1;
+
+ return -1;
+}
Why don't these check for the < ?
+ /* E.g. "::operator- <>". */
+ if (next_token->keyword == RID_OPERATOR)
+{
+ /* Consume it so it doesn't get in our way. */
+ cp_lexer_consume_token (parser->lexer);
+ next_token = cp_lexer_peek_token (parser->lexer);
+ found_operator_keyword = true;
+}
...
+ if (!found_operator_keyword && next_token->type != CPP_NAME)
+return -1;
These could be if/else if so you don't need the found_operator_keyword
variable.
+ if (next_token->type == CPP_TEMPLATE_ID)
+return 1;
This could move above the saved_token_sentinel; you won't have a
CPP_TEMPLATE_ID after RID_OPERATOR.
+ /* If this is a function template then we would see a "(" after the
+ final ">". It could also be a class template constructor. */
+ if (next_token->type == CPP_OPEN_PAREN
+ /* If this is a class template then we could see a scope token after
+ the final ">". */
+ || next_token->type == CPP_SCOPE
+ /* We could see a ";" after a variable template. */
+ || next_token->type == CPP_SEMICOLON
+ /* We could see something like
+friend vect (::operator- <>)( const vect&, const vect& );
+*/
+ || next_token->type == CPP_CLOSE_PAREN)
+return 1;
This seems too limited. As I was saying before,
Re: [PATCH] c++: error message for dependent template members [PR70417]
r it's written like an
expression because it's missing the return type (although, even adding a
return type would not make it valid). I'm not sure there's any worthwhile
way around this really.
You could tell DejaGnu to silence the warning, but I think it's probably
more informative to just include the warning inline.
Also, some more missing template keywords seemed to crop up in the
regression tests, so I had to sprinkle some more template keywords in a
few. I guess there was a hidden bug that was missing a few scenarios.
Just out of curiosity, how pedantic would you say I should be when
submitting patches to GCC etc? I regression test and bootstrap all my
changes, but I'm always worrying about what might happen if I somehow
forgot to stage a file in git, or attached the wrong patch file, or
interpreted the .sum file wrong etc. Do patches go through another round of
testing after submitting? Sometimes I wonder whether I should be applying
the patch locally and then bootstrapping and regression testing it again,
although that's hardly fun since that usually takes around 2-3 hours even
with -j 12. Maybe I ought to think about getting a dedicated Linux PC.
Patch should be attached.
Kind regards,
Anthony
On Wed, 22 Sept 2021 at 21:04, Jason Merrill wrote:
> On 9/17/21 18:22, Anthony Sharp wrote:
> > And also re-attaching the patch!
> >
> > On Fri, 17 Sept 2021 at 23:17, Anthony Sharp
> wrote:
> >>
> >> Re-adding gcc-patches@gcc.gnu.org.
> >>
> >> -- Forwarded message -
> >> From: Anthony Sharp
> >> Date: Fri, 17 Sept 2021 at 23:11
> >> Subject: Re: [PATCH] c++: error message for dependent template members
> [PR70417]
> >> To: Jason Merrill
> >>
> >>
> >> Hi Jason! Apologies for the delay.
> >>
> >>> This is basically core issue 1835, http://wg21.link/cwg1835
> >>
> >>> This was changed for C++23 by the paper "Declarations and where to find
> >>> them", http://wg21.link/p1787
> >>
> >> Interesting, I was not aware of that. I was very vaguely aware that a
> >> template-id in a class member access expression could be found by
> >> ordinary lookup (very bottom of here
> >> https://en.cppreference.com/w/cpp/language/dependent_name), but it's
> >> interesting to see it is deeper than I realised.
> >>
> >>> But in either case, whether create is in a dependent scope depends
> on
> >>> how we resolve impl::, we don't need to remember further back in the
> >>> expression. So your dependent_expression_p parameter seems like the
> >>> wrong approach. Note that when we're looking up the name after ->, the
> >>> type of the object expression is in parser->context->object_type.
> >>
> >> That's true. I think my thinking was that since it already got figured
> >> out in cp_parser_postfix_dot_deref_expression, which is where . and ->
> >> access expressions come from, I thought I might as well pass it
> >> through, since it seemed to work. But looking again, you're right,
> >> it's not really worth the hassle; might as well just call
> >> dependent_scope_p again.
> >>
> >>> The cases you fixed in symbol-summary.h are indeed mistakes, but not
> >>> ill-formed, so giving an error on them is wrong. For example, here is
> a
> >>> well-formed program that is rejected with your patch:
> >>
> >>> template void f(T t) { t.m(0); }
> >>> struct A { int m; } a;
> >>> int main() { f(a); }
> >>
> >> I suppose there was always going to be edge-cases when doing it the
> >> way I've done. But yes, it can be worked-around by making it a warning
> >> instead. Interestingly Clang doesn't trip up on that example, so I
> >> guess they must be examining it some other way (e.g. at instantiation
> >> time) - but that approach perhaps misses out on the slight performance
> >> improvement this seems to bring.
> >>
> >>> Now that we're writing C++, I'd prefer to avoid this kind of pattern in
> >>> favor of RAII, such as saved_token_sentinel. If this is still relevant
> >>> after addressing the above comments.
> >>
> >> Sorry, it's the junior developer in me showing! So this confused me at
> >> first. After having mucked around a bit I tried using
> >> saved_token_sentinel but didn't see any benefit since it doesn't
> >> rollback on going out of scope, and I'll always want to rollback. I
> >> can call rollback directly, but then I might as well save and restore
> >
Re: [PATCH] c++: error message for dependent template members [PR70417]
On 9/17/21 18:22, Anthony Sharp wrote:
And also re-attaching the patch!
On Fri, 17 Sept 2021 at 23:17, Anthony Sharp wrote:
Re-adding gcc-patches@gcc.gnu.org.
-- Forwarded message -
From: Anthony Sharp
Date: Fri, 17 Sept 2021 at 23:11
Subject: Re: [PATCH] c++: error message for dependent template members [PR70417]
To: Jason Merrill
Hi Jason! Apologies for the delay.
This is basically core issue 1835, http://wg21.link/cwg1835
This was changed for C++23 by the paper "Declarations and where to find
them", http://wg21.link/p1787
Interesting, I was not aware of that. I was very vaguely aware that a
template-id in a class member access expression could be found by
ordinary lookup (very bottom of here
https://en.cppreference.com/w/cpp/language/dependent_name), but it's
interesting to see it is deeper than I realised.
But in either case, whether create is in a dependent scope depends on
how we resolve impl::, we don't need to remember further back in the
expression. So your dependent_expression_p parameter seems like the
wrong approach. Note that when we're looking up the name after ->, the
type of the object expression is in parser->context->object_type.
That's true. I think my thinking was that since it already got figured
out in cp_parser_postfix_dot_deref_expression, which is where . and ->
access expressions come from, I thought I might as well pass it
through, since it seemed to work. But looking again, you're right,
it's not really worth the hassle; might as well just call
dependent_scope_p again.
The cases you fixed in symbol-summary.h are indeed mistakes, but not
ill-formed, so giving an error on them is wrong. For example, here is a
well-formed program that is rejected with your patch:
template void f(T t) { t.m(0); }
struct A { int m; } a;
int main() { f(a); }
I suppose there was always going to be edge-cases when doing it the
way I've done. But yes, it can be worked-around by making it a warning
instead. Interestingly Clang doesn't trip up on that example, so I
guess they must be examining it some other way (e.g. at instantiation
time) - but that approach perhaps misses out on the slight performance
improvement this seems to bring.
Now that we're writing C++, I'd prefer to avoid this kind of pattern in
favor of RAII, such as saved_token_sentinel. If this is still relevant
after addressing the above comments.
Sorry, it's the junior developer in me showing! So this confused me at
first. After having mucked around a bit I tried using
saved_token_sentinel but didn't see any benefit since it doesn't
rollback on going out of scope, and I'll always want to rollback. I
can call rollback directly, but then I might as well save and restore
myself. So what I did was use it but also modify it slightly to
rollback by default on going out of scope (in my mind that makes more
sense, since if something goes wrong you wouldn't normally want to
commit anything that happened [edit 1: unless committing was part of
the whole sanity checking thing] [edit 2: well I guess you could also
argue that since this is a parser after all, we like to KEEP things
sometimes]). But anyways, I made this configurable; it now has three
modes - roll-back, commit or do nothing. Let me know if you think
that's not the way to go.
I like adding the configurability, but I think let's keep committing as
the default behavior. And adding the parameter to the rollback function
seems unnecessary. For the behavior argument, let's use an enum to be
clearer.
This code doesn't handle skipping matched ()/{}/[] in the
template-argument-list. You probably want to involve
cp_parser_skip_to_end_of_template_parameter_list somehow.
Good point. It required some refactoring, but I have used it. Also,
just putting it out there, this line from
cp_parser_skip_to_end_of_template_parameter_list makes zero sense to
me (why throw an error OR immediately return?), but I have worked
around it, since it seems to break without it:
/* Are we ready, yet? If not, issue error message. */
if (cp_parser_require (parser, CPP_GREATER, RT_GREATER))
return false;
If the next token is >, we're already at the end of the template
argument list. If not, then something has gone wrong, so give an error
and skip ahead.
Last thing - I initially made a mistake. I put something like:
(next_token->type == CPP_NAME
&& MAYBE_CLASS_TYPE_P (parser->scope)
&& !constructor_name_p (cp_expr (next_token->u.value,
next_token->location),
parser->scope))
Instead of:
!(next_token->type == CPP_NAME
&& MAYBE_CLASS_TYPE_P (parser->scope)
&& constructor_name_p (cp_expr (next_token->u.value,
next_token->location),
parser->sc
Re: [PATCH] c++: error message for dependent template members [PR70417]
And also re-attaching the patch!
On Fri, 17 Sept 2021 at 23:17, Anthony Sharp wrote:
>
> Re-adding gcc-patches@gcc.gnu.org.
>
> -- Forwarded message -
> From: Anthony Sharp
> Date: Fri, 17 Sept 2021 at 23:11
> Subject: Re: [PATCH] c++: error message for dependent template members
> [PR70417]
> To: Jason Merrill
>
>
> Hi Jason! Apologies for the delay.
>
> > This is basically core issue 1835, http://wg21.link/cwg1835
>
> > This was changed for C++23 by the paper "Declarations and where to find
> > them", http://wg21.link/p1787
>
> Interesting, I was not aware of that. I was very vaguely aware that a
> template-id in a class member access expression could be found by
> ordinary lookup (very bottom of here
> https://en.cppreference.com/w/cpp/language/dependent_name), but it's
> interesting to see it is deeper than I realised.
>
> > But in either case, whether create is in a dependent scope depends on
> > how we resolve impl::, we don't need to remember further back in the
> > expression. So your dependent_expression_p parameter seems like the
> > wrong approach. Note that when we're looking up the name after ->, the
> > type of the object expression is in parser->context->object_type.
>
> That's true. I think my thinking was that since it already got figured
> out in cp_parser_postfix_dot_deref_expression, which is where . and ->
> access expressions come from, I thought I might as well pass it
> through, since it seemed to work. But looking again, you're right,
> it's not really worth the hassle; might as well just call
> dependent_scope_p again.
>
> > The cases you fixed in symbol-summary.h are indeed mistakes, but not
> > ill-formed, so giving an error on them is wrong. For example, here is a
> > well-formed program that is rejected with your patch:
>
> > template void f(T t) { t.m(0); }
> > struct A { int m; } a;
> > int main() { f(a); }
>
> I suppose there was always going to be edge-cases when doing it the
> way I've done. But yes, it can be worked-around by making it a warning
> instead. Interestingly Clang doesn't trip up on that example, so I
> guess they must be examining it some other way (e.g. at instantiation
> time) - but that approach perhaps misses out on the slight performance
> improvement this seems to bring.
>
> > Now that we're writing C++, I'd prefer to avoid this kind of pattern in
> > favor of RAII, such as saved_token_sentinel. If this is still relevant
> > after addressing the above comments.
>
> Sorry, it's the junior developer in me showing! So this confused me at
> first. After having mucked around a bit I tried using
> saved_token_sentinel but didn't see any benefit since it doesn't
> rollback on going out of scope, and I'll always want to rollback. I
> can call rollback directly, but then I might as well save and restore
> myself. So what I did was use it but also modify it slightly to
> rollback by default on going out of scope (in my mind that makes more
> sense, since if something goes wrong you wouldn't normally want to
> commit anything that happened [edit 1: unless committing was part of
> the whole sanity checking thing] [edit 2: well I guess you could also
> argue that since this is a parser afterall, we like to KEEP things
> sometimes]). But anyways, I made this configurable; it now has three
> modes - roll-back, commit or do nothing. Let me know if you think
> that's not the way to go.
>
> > This code doesn't handle skipping matched ()/{}/[] in the
> > template-argument-list. You probably want to involve
> > cp_parser_skip_to_end_of_template_parameter_list somehow.
>
> Good point. It required some refactoring, but I have used it. Also,
> just putting it out there, this line from
> cp_parser_skip_to_end_of_template_parameter_list makes zero sense to
> me (why throw an error OR immediately return?), but I have worked
> around it, since it seems to break without it:
>
> > /* Are we ready, yet? If not, issue error message. */
> > if (cp_parser_require (parser, CPP_GREATER, RT_GREATER))
> > return false;
>
> Last thing - I initially made a mistake. I put something like:
>
> (next_token->type == CPP_NAME
> && MAYBE_CLASS_TYPE_P (parser->scope)
> && !constructor_name_p (cp_expr (next_token->u.value,
>
> next_token->location),
>parser->scope))
>
> Instead of:
>
> !(next_token->type == CPP_NAME
> && MAYBE_CLASS_TYPE_P (parser->scope)
> && constructor_name_p (cp_expr (next_token->u.value,
>
Re: [PATCH] c++: error message for dependent template members [PR70417]
Re-adding gcc-patches@gcc.gnu.org.
-- Forwarded message -
From: Anthony Sharp
Date: Fri, 17 Sept 2021 at 23:11
Subject: Re: [PATCH] c++: error message for dependent template members [PR70417]
To: Jason Merrill
Hi Jason! Apologies for the delay.
> This is basically core issue 1835, http://wg21.link/cwg1835
> This was changed for C++23 by the paper "Declarations and where to find
> them", http://wg21.link/p1787
Interesting, I was not aware of that. I was very vaguely aware that a
template-id in a class member access expression could be found by
ordinary lookup (very bottom of here
https://en.cppreference.com/w/cpp/language/dependent_name), but it's
interesting to see it is deeper than I realised.
> But in either case, whether create is in a dependent scope depends on
> how we resolve impl::, we don't need to remember further back in the
> expression. So your dependent_expression_p parameter seems like the
> wrong approach. Note that when we're looking up the name after ->, the
> type of the object expression is in parser->context->object_type.
That's true. I think my thinking was that since it already got figured
out in cp_parser_postfix_dot_deref_expression, which is where . and ->
access expressions come from, I thought I might as well pass it
through, since it seemed to work. But looking again, you're right,
it's not really worth the hassle; might as well just call
dependent_scope_p again.
> The cases you fixed in symbol-summary.h are indeed mistakes, but not
> ill-formed, so giving an error on them is wrong. For example, here is a
> well-formed program that is rejected with your patch:
> template void f(T t) { t.m(0); }
> struct A { int m; } a;
> int main() { f(a); }
I suppose there was always going to be edge-cases when doing it the
way I've done. But yes, it can be worked-around by making it a warning
instead. Interestingly Clang doesn't trip up on that example, so I
guess they must be examining it some other way (e.g. at instantiation
time) - but that approach perhaps misses out on the slight performance
improvement this seems to bring.
> Now that we're writing C++, I'd prefer to avoid this kind of pattern in
> favor of RAII, such as saved_token_sentinel. If this is still relevant
> after addressing the above comments.
Sorry, it's the junior developer in me showing! So this confused me at
first. After having mucked around a bit I tried using
saved_token_sentinel but didn't see any benefit since it doesn't
rollback on going out of scope, and I'll always want to rollback. I
can call rollback directly, but then I might as well save and restore
myself. So what I did was use it but also modify it slightly to
rollback by default on going out of scope (in my mind that makes more
sense, since if something goes wrong you wouldn't normally want to
commit anything that happened [edit 1: unless committing was part of
the whole sanity checking thing] [edit 2: well I guess you could also
argue that since this is a parser afterall, we like to KEEP things
sometimes]). But anyways, I made this configurable; it now has three
modes - roll-back, commit or do nothing. Let me know if you think
that's not the way to go.
> This code doesn't handle skipping matched ()/{}/[] in the
> template-argument-list. You probably want to involve
> cp_parser_skip_to_end_of_template_parameter_list somehow.
Good point. It required some refactoring, but I have used it. Also,
just putting it out there, this line from
cp_parser_skip_to_end_of_template_parameter_list makes zero sense to
me (why throw an error OR immediately return?), but I have worked
around it, since it seems to break without it:
> /* Are we ready, yet? If not, issue error message. */
> if (cp_parser_require (parser, CPP_GREATER, RT_GREATER))
> return false;
Last thing - I initially made a mistake. I put something like:
(next_token->type == CPP_NAME
&& MAYBE_CLASS_TYPE_P (parser->scope)
&& !constructor_name_p (cp_expr (next_token->u.value,
next_token->location),
parser->scope))
Instead of:
!(next_token->type == CPP_NAME
&& MAYBE_CLASS_TYPE_P (parser->scope)
&& constructor_name_p (cp_expr (next_token->u.value,
next_token->location),
parser->scope))
This meant a lot of things were being excluded that weren't supposed
to be. Oops! Changing this opened up a whole new can of worms, so I
had to make some changes to the main logic, but it just a little bit
in the end.
Regtested everything again and all seems fine. Bootstraps fine. Patch
attached. Let me know if it needs anything else.
Thanks for the help,
Anthony
On Fri, 27 Aug 2021 at 22:33, Jason Merrill wrote:
>
>
Re: [PATCH] c++: error message for dependent template members [PR70417]
On 8/20/21 12:56 PM, Anthony Sharp via Gcc-patches wrote:
Hi, hope everyone is well. I have a patch here for issue 70417
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70417). I'm still a GCC
noob, and this is probably the hardest thing I have ever coded in my
life, so please forgive any initial mistakes!
TLDR
This patch introduces a helpful error message when the user attempts
to put a template-id after a member access token (., -> or ::) in a
dependent context without having put the "template" keyword after the
access token.
Great, thanks!
CONTEXT
In C++, when referencing a class member (using ., -> or ::) in a
dependent context, if that member is a template, the template keyword
is required after the access token. For example:
template void MyDependentTemplate(T t)
{
t.DoSomething(); /* Error - t is dependent. "<" is treated as a
less-than sign because DoSomething is assumed to be a non-template
identifier. */
t.template DoSomething(); // Good.
}
PATCH EXPLANATION
In order to throw an appropriate error message we need to identify
and/or create a point in the compiler where the following conditions
are all simultaneously satisfied:
A) a class member access token (., ->, ::)
B) a dependent context
C) the thing being accessed is a template-id
D) No template keyword
A, B and D are relatively straightforward and I won't go into detail
about how that was achieved. The real challenge is C - figuring out
whether a name is a template-id.
Usually, we would look up the name and use that to determine whether
the name is a template or not. But, we cannot do that. We are in a
dependent context, so lookup cannot (in theory) find anything at the
point the access expression is parsed.
We (maybe) could defer checking until the template is actually
instantiated. But this is not the approach I have gone for, since this
to me sounded unnecessarily awkward and clunky. In my mind this also
seems like a syntax error, and IMO it seems more natural that syntax
errors should get caught as soon as they are parsed.
So, the solution I went for was to figure out whether the name was a
template by looking at the surrounding tokens. The key to this lies in
being able to differentiate between the start and end of a template
parameter list (< and >) and greater-than and less-than tokens (and
perhaps also things like << or >>). If the final > (if we find one at
all) does indeed mark the end of a class or function template, then it
will be followed by something like (, ::, or even just ;. On the other
hand, a greater-than operator would be followed by a name.
PERFORMANCE IMPACT
My concern with this approach was that checking this would actually
slow the compiler down. In the end it seemed the opposite was true. By
parsing the tokens to determine whether the name looks like a
template-id, we can cut out a lot of the template-checking code that
gets run trying to find a template when there is none, making compile
time generally faster. That being said, I'm not sure if the
improvement will be that substantial, so I did not feel it necessary
to introduce the template checking method everywhere where we could
possibly have run into a template.
I ran an ABAB test with the following code repeated enough times to
fill up about 10,000 lines:
ai = 1;
bi = 2;
ci = 3;
c.x = 4;
()->x = 5;
mytemplateclass::y = 6;
c.template class_func();
()->template class_func();
mytemplateclass::template class_func_static();
c2.class_func();
()->class_func();
myclass::class_func_static();
ai = 6;
bi = 5;
ci = 4;
c.x = 3;
()->x = 2;
mytemplateclass::y = 1;
c.template class_func();
()->template class_func();
mytemplateclass::template class_func_static();
c2.class_func();
()->class_func();
myclass::class_func_static();
It basically just contains a mixture of class access expressions plus
some regular assignments for good measure. The compiler was compiled
without any optimisations (and so slower than usual). In hindsight,
perhaps this test case assumes the worst-ish-case scenario since it
contains a lot of templates; most of the benefits of this patch occur
when parsing non-templates.
The compiler (clean) and the compiler with the patch applied (patched)
compiled the above code 20 times each in ABAB fashion. On average, the
clean compiler took 2.26295 seconds and the patched compiler took
2.25145 seconds (about 0.508% faster). Whether this improvement
remains or disappears when the compiler is built with optimisations
turned on I haven't tested. My main concern was just making sure it
didn't get any slower.
AWKWARD TEST CASES
You will see from the patch that it required the updating of a few
testcases (as well as one place within the compiler). I believe this
is because up until now, the compiler allowed the omission of the
template keyword in some places it shouldn't have. Take decltype34.C
for example:
struct impl
{
template static T create();
};
template()->impl::create())>
struct tester{};
GCC currently accepts this code. My
[PATCH] c++: error message for dependent template members [PR70417]
h causes it to be rejected. For what it's worth, Clang also rejects this code: 1824786093/source.cpp:6:70: error: use 'template' keyword to treat 'create' as a dependent template name template()->impl::create())> ^ template I think Clang is correct since from what I understand it should be written as impl::create()->impl::template create(). Here, create() is dependent, so it makes sense that we would need "template" before the scope operator. Then again, I could well be wrong. The rules around dependent template names are pretty crazy. REGRESSION TESTING No differences between clean/patched g++.sum, gcc.sum and libstdc++.sum. Bootstraps fine. Built on x86_64-pc-linux-gnu for x86_64-pc-linux-gnu. CONCLUSION Some of the changes are a bit debatable, e.g. passing and adding new arguments to various functions just for the sake of a slight performance improvement - I don't really have the high-level experience to be able to judge whether that is worth the increased code complexity or not, so I just went with my gut. Please find patch attached :) Kind regards, Anthony Sharp From 0316e821607f6875293a664ab181747188ef3e73 Mon Sep 17 00:00:00 2001 From: Anthony Sharp Date: Thu, 19 Aug 2021 17:51:38 +0100 Subject: [PATCH] c++: error message for dependent template members [PR70417] Add a helpful error message for when the user forgets to include the "template" keyword after ., -> or :: when accessing a member in a dependent context, where the member is a template. Fix some cases where the compiler was allowing the exclusion of the template keyword when it shouldn't have been. gcc/cp/ChangeLog: 2021-08-15 Anthony Sharp * parser.c (next_token_begins_template_id): New method to check whether we are looking at what syntactically looks like a template-id. (cp_parser_id_expression): Added dependent_expression_p as function argument. Check whether the id looks like a template; only attempt to parse it as one if so. Throw error if missing "template" keyword. (cp_parser_unqualified_id): Pass looks_like_template through this function to cp_parser_template_id_expr to avoid repeating logic unnecessarily. (cp_parser_postfix_dot_deref_expression): Pass dependent_p to cp_parser_id_expression. (cp_parser_template_id): Pass looks_like_template to avoid repeating logic. (cp_parser_template_id_expr): As above. (cp_parser_parse_and_diagnose_invalid_type_name): Minor update to a function call. (cp_parser_decltype_expr): As above. (cp_parser_default_template_template_argument): As above. (cp_parser_template_argument): As above. (cp_parser_primary_expression): As above. (cp_parser_simple_type_specifier): As above. (cp_parser_pseudo_destructor_name): As above. (cp_parser_type_name): As above. (cp_parser_elaborated_type_specifier): As above. (cp_parser_using_declaration): As above. (cp_parser_declarator_id): As above. (cp_parser_class_name): As above. (cp_parser_class_head): AS above. (cp_parser_type_requirement): As above. (cp_parser_constructor_declarator_p): As above. (cp_parser_omp_var_list_no_open): As above. (cp_parser_omp_clause_reduction): As above. (cp_parser_omp_clause_linear): As above. (cp_parser_omp_clause_detach): As above. (cp_parser_omp_for_loop_init): As above. (cp_finish_omp_declare_variant): As above. (cp_parser_omp_declare_reduction_exprs): As above. (cp_parser_oacc_routine): As above. 2021-08-15 Anthony Sharp gcc/ChangeLog: * symbol-summary.h: Added missing template keyword. 2021-08-15 Anthony Sharp gcc/testsuite/ChangeLog: * g++.dg/cpp0x/decltype34.C: Add missing template keyword(s). * g++.dg/parse/template26.C: As above. * g++.old-deja/g++.pt/explicit81.C: As above. * g++.dg/template/dependent-name15.C: New test for pr70417. --- gcc/cp/parser.c | 376 +- gcc/symbol-summary.h | 4 +- gcc/testsuite/g++.dg/cpp0x/decltype34.C | 4 +- gcc/testsuite/g++.dg/parse/template26.C | 6 +- .../g++.dg/template/dependent-name15.C| 46 +++ .../g++.old-deja/g++.pt/explicit81.C | 4 +- 6 files changed, 341 insertions(+), 99 deletions(-) create mode 100644 gcc/testsuite/g++.dg/template/dependent-name15.C diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index 45216f0a222..12fa945c1e5 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -2108,9 +2108,9 @@ static void cp_parser_translation_unit (cp_parser *); static cp_expr cp_parser_primary_expression (cp_parser *, bool, bool, bool, cp_id_kind *); static cp_expr cp_parser_id_expression - (cp_parser *, bool, bool, bool *, bool, bool); + (cp_parser *, bool, bool, bool *, bool, bool, bool *); static cp_expr cp_parser_unqualified_id - (cp_parser *, bool, bool, bool, bool); + (cp_parser *, bool, bool, bool, bool, int); static tree cp_parser_nested_name_specifier_opt (cp_parser *, bool, bool, bool, bool, bool = fals
