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<t> (::operator- <>)( const vect<t>&, const vect<t>&
);
>> + */
>> + || 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 <class T> struct A
>{
> template <class U> static U m;
>};
>
>template <class T> int f (T t)
>{
> return t.m<int> + 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 T, class U,
>> - class = decltype(impl::create<T>()->impl::create<U>())>
>> + class = decltype(impl::create<T>()->impl::template create<U>())>
>
>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 more sense what it's trying to do now.
That's just my lack of experience in reading complex C++ code. Thanks.
>> + int looks_like_template_id
>> + = next_token_begins_template_id (parser);
>
>We probably want this to just be 0, and not call the function, if
>!warn_missing_template_keyword.
Added the check to warn_missing_template_keyword. We'd probably still want
to
call the function either way since it's supposed to speed things up a
little. But we can disable
the warning.
In either case, were next_token_begins_template_id not called, the way it's
set-up currently
I think we'd have to default it to 1, otherwise it will cause it to skip
some of the potentially
necessary parsing logic.
>> + if (looks_like_template_id == 1)
>> + {
>
>Shouldn't this be != -1, as in cp_parser_unqualified_id?
looks_like_template_id will always be 1 or -1 at that point (in
cp_parser_id_expression), so functionally speaking != -1 and == 1 should
hopefully give us the same result there.
The only reason we use != -1 in cp_parser_unqualified_id is because that
function
is called from many places, and so sometimes looks_like_template_id may be
0 there.
So we'd want to check the logic if we saw a 0 or a 1 there, hence the != -1.
But either way is probably just as valid.
>> -Foo<T>::operator Foo<U>() {
>> +Foo<T>::template operator Foo<U>() {
>
>This is wrong; you don't use the template keyword like this in a
declarator.
Had a moment of weakness, that's obviously wrong as you say.
>> - = sizeof(impl::create<T>()->*impl::create<U>())>
>> + = sizeof(impl::create<T>()->*impl::template create<U>())>
>
>As above, this shouldn't be necessary currently.
>
>> - ptr->X::xfn <N::e0> ();
>> + ptr->X::template xfn <N::e0> ();
>
>Or this.
Fixed as discussed.
>> + if (template_p)
>> + *template_p = true;
>
>Since we unconditionally dereference template_p a few lines below, let's
>try removing this condition.
I think I see where you mean. I had tried to refactor these parts but it
turns
out it's trickier than it seems, and was causing a few bugs. The code is a
bit
tangled at the moment; for example, assigning *template_p in circumstances
other than in the strict ways the calling functions are anticipating will
cause issues,
even if merely doing so for the purpose of indicating a found template
keyword (as the
comments above the function declaration hint at). So I've gone and put most
of that
stuff back as it was. Certainly doable to refactor that part, but probably
best left to someone
who can get more to grips with the surrounding context.
>> - n.template Nested<B>::method();
>> + n.template Nested<B>::method(); // { dg-error "is not a template"
"" { target { *-*-* } } }
>
>This is wrong; Nested is indeed a template.
Yes, sorry, this was a bug I caused that I've now fixed. Not sure why I
thought
it was a good idea to try and suppress it...
Bootstraps fine. Re-tested. g++ and gcc tests were perfect. However, for
some reason the libstdc++, libgomp and libitm tests were not running
automatically with this patch with --enable-languages=c,c++ and then a
vanilla make and make check. I had to run them manually with make
check-target-libstdc++-v3; make check-target-libitm-c++; make
check-target-libgomp-c++. And once I had done that it looked like in my
patch the number of successful tests went down and the number of
unsupported tests increased in both libstdc++ and libgomp (although
unexpected failures/successes did not change). Not sure why this was, so
might be worth bearing in mind. (Maybe my source was more/less up to date
with git than it was when I ran the original tests?)
Thanks!
Anthony
On Tue, 19 Oct 2021 at 20:15, Jason Merrill <ja...@redhat.com> wrote:
> 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 <args>( 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<t> (::operator- <>)( const vect<t>&, const vect<t>&
> );
> > + */
> > + || 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 <class T> struct A
> {
> template <class U> static U m;
> };
>
> template <class T> int f (T t)
> {
> return t.m<int> + 42;
> }
>
> This is especially a problem since we use the return value of -1 to skip
> calling cp_parser_template_id_expr.
>
> > +++ b/gcc/testsuite/g++.dg/cpp0x/decltype34.C
> > @@ -7,13 +7,13 @@ struct impl
> > };
> >
> > template<class T, class U,
> > - class = decltype(impl::create<T>()->impl::create<U>())>
> > + class = decltype(impl::create<T>()->impl::template create<U>())>
>
> 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.
>
> > + int looks_like_template_id
> > + = next_token_begins_template_id (parser);
>
> We probably want this to just be 0, and not call the function, if
> !warn_missing_template_keyword.
>
> > + if (template_p)
> > + *template_p = true;
>
> Since we unconditionally dereference template_p a few lines below, let's
> try removing this condition.
>
> > + if (looks_like_template_id == 1)
> > + {
>
> Shouldn't this be != -1, as in cp_parser_unqualified_id?
>
> > -Foo<T>::operator Foo<U>() {
> > +Foo<T>::template operator Foo<U>() {
>
> This is wrong; you don't use the template keyword like this in a
> declarator.
>
> > - n.template Nested<B>::method();
> > + n.template Nested<B>::method(); // { dg-error "is not a template"
> "" { target { *-*-* } } }
>
> This is wrong; Nested is indeed a template.
>
> > - = sizeof(impl::create<T>()->*impl::create<U>())>
> > + = sizeof(impl::create<T>()->*impl::template create<U>())>
>
> As above, this shouldn't be necessary currently.
>
> > - ptr->X::xfn <N::e0> ();
> > + ptr->X::template xfn <N::e0> ();
>
> Or this.
>
> Jason
From ca28b6dc451243f7412815c29e96ab90deb5517e Mon Sep 17 00:00:00 2001
From: Anthony Sharp <anthonysharp15@gmail.com>
Date: Sat, 4 Dec 2021 13:30:40 +0000
Subject: [PATCH] c++: warning for dependent template members [PR70417]
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.
gcc/c-family/ChangeLog:
2021-12-04 Anthony Sharp <anthonysharp15@gmail.com>
* c.opt: Added -Wmissing-template-keyword.
gcc/cp/ChangeLog:
2021-12-04 Anthony Sharp <anthonysharp15@gmail.com>
* parser.c (struct saved_token_sentinel): Added modes to control
what happens on destruction.
(next_token_begins_template_name): New method to
check whether we are looking at what syntactically
looks like a template name.
(cp_parser_id_expression): Check whether the id looks like a
template; only attempt to parse it as one if so. Give
warning 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_lambda_declarator_opt):
cp_parser_skip_to_end_of_template_parameter_list becomes
cp_parser_require_end_of_template_parameter_list.
(cp_parser_statement): Adjust for changes to saved_token_sentinel.
(cp_parser_template_id): Pass looks_like_template to avoid
repeating logic.
(cp_parser_template_id_expr): As above.
(cp_parser_explicit_template_declaration):
cp_parser_skip_to_end_of_template_parameter_list becomes
cp_parser_require_end_of_template_parameter_list.
(cp_parser_enclosed_template_argument_list): Same as above.
(cp_parser_skip_entire_template_parameter_list): New function that
skips an entire template parameter list.
(cp_parser_require_end_of_template_parameter_list): See below.
Ensures we have reached the end of a template parameter list.
(cp_parser_skip_to_end_of_template_parameter_list): Refactored to be
called from one of the above two functions. Returns true on success
and false otherwise.
gcc/ChangeLog:
2021-12-04 Anthony Sharp <anthonysharp15@gmail.com>
* doc/invoke.texi: Documentation for Wmissing-template-keyword.
gcc/testsuite/ChangeLog:
2021-12-04 Anthony Sharp <anthonysharp15@gmail.com>
* g++.dg/cpp0x/variadic-mem_fn2.C: Catch warning about missing
template keyword.
* g++.dg/template/dependent-name17.C: New test for missing
template keywords.
---
gcc/c-family/c.opt | 4 +
gcc/cp/parser.c | 328 ++++++++++++++----
gcc/doc/invoke.texi | 33 ++
gcc/testsuite/g++.dg/cpp0x/variadic-mem_fn2.C | 1 +
.../g++.dg/template/dependent-name17.C | 51 +++
5 files changed, 343 insertions(+), 74 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/template/dependent-name17.C
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 3976fc368db..d09ac499a89 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -876,6 +876,10 @@ Wmissing-requires
C++ ObjC++ Var(warn_missing_requires) Init(1) Warning
Warn about likely missing requires keyword.
+Wmissing-template-keyword
+C++ ObjC++ Var(warn_missing_template_keyword) Init(1) Warning
+Warn when the template keyword is missing after a member access token in a dependent member access expression if that member is a template.
+
Wmultistatement-macros
C ObjC C++ ObjC++ Var(warn_multistatement_macros) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
Warn about unsafe macros expanding to multiple statements used as a body of a clause such as if, else, while, switch, or for.
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index aa7f0e4b526..818d6876970 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -1275,17 +1275,32 @@ cp_lexer_rollback_tokens (cp_lexer* lexer)
lexer->next_token = lexer->saved_tokens.pop ();
}
-/* RAII wrapper around the above functions, with sanity checking. Creating
- a variable saves tokens, which are committed when the variable is
- destroyed unless they are explicitly rolled back by calling the rollback
- member function. */
+/* Determines what saved_token_sentinel does when going out of scope. */
+
+enum saved_token_sentinel_mode {
+ STS_COMMIT,
+ STS_ROLLBACK,
+ STS_DONOTHING
+};
+
+/* RAII wrapper around the above functions, with sanity checking (the token
+ stream should be the same at the point of instantiation as it is at the
+ point of destruction).
+
+ Creating a variable saves tokens. MODE determines what happens when the
+ object is destroyed. STS_COMMIT commits tokens (default),
+ STS_ROLLBACK rolls-back and STS_DONOTHING does nothing. Calling
+ rollback() will immediately roll-back tokens and set MODE to
+ STS_DONOTHING. */
struct saved_token_sentinel
{
cp_lexer *lexer;
unsigned len;
- bool commit;
- saved_token_sentinel(cp_lexer *lexer): lexer(lexer), commit(true)
+ saved_token_sentinel_mode mode;
+ saved_token_sentinel (cp_lexer *_lexer,
+ saved_token_sentinel_mode _mode = STS_COMMIT)
+ : lexer (_lexer), mode (_mode)
{
len = lexer->saved_tokens.length ();
cp_lexer_save_tokens (lexer);
@@ -1293,12 +1308,15 @@ struct saved_token_sentinel
void rollback ()
{
cp_lexer_rollback_tokens (lexer);
- commit = false;
+ mode = STS_DONOTHING;
}
- ~saved_token_sentinel()
+ ~saved_token_sentinel ()
{
- if (commit)
+ if (mode == STS_COMMIT)
cp_lexer_commit_tokens (lexer);
+ else if (mode == STS_ROLLBACK)
+ cp_lexer_rollback_tokens (lexer);
+
gcc_assert (lexer->saved_tokens.length () == len);
}
};
@@ -2137,7 +2155,7 @@ static cp_expr cp_parser_primary_expression
static cp_expr cp_parser_id_expression
(cp_parser *, 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 = 0);
static tree cp_parser_nested_name_specifier_opt
(cp_parser *, bool, bool, bool, bool, bool = false);
static tree cp_parser_nested_name_specifier
@@ -2465,9 +2483,9 @@ static tree cp_parser_template_parameter
static tree cp_parser_type_parameter
(cp_parser *, bool *);
static tree cp_parser_template_id
- (cp_parser *, bool, bool, enum tag_types, bool);
+ (cp_parser *, bool, bool, enum tag_types, bool, int = 0);
static tree cp_parser_template_id_expr
- (cp_parser *, bool, bool, bool);
+ (cp_parser *, bool, bool, bool, int = 0);
static tree cp_parser_template_name
(cp_parser *, bool, bool, bool, enum tag_types, bool *);
static tree cp_parser_template_argument_list
@@ -2770,7 +2788,11 @@ static void cp_parser_skip_to_end_of_block_or_statement
(cp_parser *);
static bool cp_parser_skip_to_closing_brace
(cp_parser *);
-static void cp_parser_skip_to_end_of_template_parameter_list
+static bool cp_parser_skip_entire_template_parameter_list
+ (cp_parser *);
+static void cp_parser_require_end_of_template_parameter_list
+ (cp_parser *);
+static bool cp_parser_skip_to_end_of_template_parameter_list
(cp_parser *);
static void cp_parser_skip_to_pragma_eol
(cp_parser*, cp_token *);
@@ -6061,6 +6083,84 @@ cp_parser_primary_expression (cp_parser *parser,
/*decltype*/false, idk);
}
+/* Check if we are looking at what "looks" like a template-id. Of course,
+ we can't technically know for sure whether it is a template-id without
+ doing lookup (although, the reason we are here is because the context
+ might be dependent and so it might not be possible to do any lookup
+ yet). Return 1 if it does look like a template-id. Return -1 if not.
+
+ Note that this is not perfect - it will generate false positives for
+ things like a.m < n > (0), where m and n are integers, for example. */
+
+static int
+next_token_begins_template_name (cp_parser* parser)
+{
+ cp_token* next_token = cp_lexer_peek_token (parser->lexer);
+
+ /* For performance's sake, quickly return from the most common case. */
+ if (next_token->type == CPP_NAME
+ && !cp_parser_nth_token_starts_template_argument_list_p (parser, 2))
+ return -1;
+
+ /* For these purposes we must believe all "template" keywords; identifiers
+ without <> at the end can still be templates, but they require the
+ template keyword. The template keyword is the only way we can tell those
+ kinds of names are templates. */
+ 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
+ && cp_parser_nth_token_starts_template_argument_list_p (parser, 3))
+ return 1;
+
+ return -1;
+ }
+
+ if (next_token->type == CPP_TEMPLATE_ID)
+ return 1;
+
+ /* E.g. "::operator- <>". */
+ if (next_token->keyword == RID_OPERATOR)
+ {
+ if (cp_parser_nth_token_starts_template_argument_list_p (parser, 3))
+ return 1;
+
+ return -1;
+ }
+
+ /* By now the next token should be a name and the one after that
+ should be a "<". */
+ if (next_token->type != CPP_NAME
+ || !cp_parser_nth_token_starts_template_argument_list_p (parser, 2))
+ return -1;
+
+ saved_token_sentinel saved_tokens (parser->lexer, STS_ROLLBACK);
+
+ /* Consume the name/operator. */
+ cp_lexer_consume_token (parser->lexer);
+
+ /* Skip to the end. If it fails, it wasn't a template. */
+ if (!cp_parser_skip_entire_template_parameter_list (parser))
+ return -1;
+
+ /* We need to distinguish this from a greater-than operator. An easy way
+ to do this is just to check for a name. */
+ if (cp_lexer_peek_token (parser->lexer)->type == CPP_NAME)
+ return -1;
+
+ return 1;
+}
+
/* Parse an id-expression.
id-expression:
@@ -6127,6 +6227,47 @@ cp_parser_id_expression (cp_parser *parser,
template_keyword_p)
!= NULL_TREE);
+ if (cp_lexer_next_token_is_keyword (parser->lexer, RID_TEMPLATE))
+ template_keyword_p = true;
+
+ /* If this doesn't look like a template-id, there is no point
+ trying to parse it as one. By checking first, we usually speed
+ compilation up, and it allows us to spot missing "template"
+ keywords. */
+ int looks_like_template_id = next_token_begins_template_name (parser);
+ cp_token* next_token = cp_lexer_peek_token (parser->lexer);
+
+ /* If this is a dependent member-access expression accessing a template
+ member without the "template" keyword, issue a helpful warning. */
+ if (warn_missing_template_keyword
+ && looks_like_template_id == 1
+ && !template_keyword_p
+ /* . and -> access tokens. */
+ && ((dependent_scope_p (parser->context->object_type)
+ && parser->scope == NULL)
+ /* :: access expressions. */
+ || (dependent_scope_p (parser->scope)
+ /* A template cannot name a constructor. But don't complain if
+ missing the template keyword in that case; continue on and
+ an error for the (ill-formed) named constructor will get
+ thrown later. */
+ && !(next_token->type == CPP_NAME
+ && MAYBE_CLASS_TYPE_P (parser->scope)
+ && constructor_name_p (next_token->u.value,
+ parser->scope))
+ && !declarator_p))
+ /* ~ before a class template-id disallows the "template" keyword.
+ Probably because in the case of A::~A<>, it is certain that
+ A is a class template. Although a destructor cannot be
+ a template, you can call the destructor of a class template -
+ e.g. see "__node->~_Rb_tree_node<_Val>();" in stl_tree.h. */
+ && next_token->type != CPP_COMPL)
+ {
+ warning_at (next_token->location, OPT_Wmissing_template_keyword,
+ "expected \"template\" keyword before dependent template "
+ "name");
+ }
+
/* If there is a nested-name-specifier, then we are looking at
the first qualified-id production. */
if (nested_name_specifier_p)
@@ -6150,7 +6291,8 @@ cp_parser_id_expression (cp_parser *parser,
unqualified_id = cp_parser_unqualified_id (parser, *template_p,
check_dependency_p,
declarator_p,
- /*optional_p=*/false);
+ /*optional_p=*/false,
+ looks_like_template_id);
/* Restore the SAVED_SCOPE for our caller. */
parser->scope = saved_scope;
parser->object_scope = saved_object_scope;
@@ -6162,33 +6304,23 @@ cp_parser_id_expression (cp_parser *parser,
of the other qualified-id productions. */
else if (global_scope_p)
{
- cp_token *token;
- tree id;
-
- /* Peek at the next token. */
- token = cp_lexer_peek_token (parser->lexer);
-
- /* If it's an identifier, and the next token is not a "<", then
- we can avoid the template-id case. This is an optimization
- for this common case. */
- if (token->type == CPP_NAME
- && !cp_parser_nth_token_starts_template_argument_list_p
- (parser, 2))
- return cp_parser_identifier (parser);
-
- cp_parser_parse_tentatively (parser);
- /* Try a template-id. */
- id = cp_parser_template_id_expr (parser,
- /*template_keyword_p=*/false,
- /*check_dependency_p=*/true,
- declarator_p);
- /* If that worked, we're done. */
- if (cp_parser_parse_definitely (parser))
- return id;
+ if (looks_like_template_id == 1)
+ {
+ cp_parser_parse_tentatively (parser);
+ /* Try a template-id. */
+ tree id = cp_parser_template_id_expr (parser,
+ /*template_keyword_p=*/false,
+ /*check_dependency_p=*/true,
+ declarator_p,
+ looks_like_template_id);
+ /* If that worked, we're done. */
+ if (cp_parser_parse_definitely (parser))
+ return id;
+ }
/* Peek at the next token. (Changes in the token buffer may
have invalidated the pointer obtained above.) */
- token = cp_lexer_peek_token (parser->lexer);
+ cp_token *token = cp_lexer_peek_token (parser->lexer);
switch (token->type)
{
@@ -6209,7 +6341,8 @@ cp_parser_id_expression (cp_parser *parser,
return cp_parser_unqualified_id (parser, template_keyword_p,
/*check_dependency_p=*/true,
declarator_p,
- optional_p);
+ optional_p,
+ looks_like_template_id);
}
/* Parse an unqualified-id.
@@ -6224,6 +6357,10 @@ cp_parser_id_expression (cp_parser *parser,
If TEMPLATE_KEYWORD_P is TRUE, we have just seen the `template'
keyword, in a construct like `A::template ...'.
+ If LOOKS_LIKE_TEMPLATE_ID is 1, we checked and saw that this looks
+ like a template-id. If it is -1, we checked and saw that it did
+ not look like a template-id. If 0, we have not checked.
+
Returns a representation of unqualified-id. For the `identifier'
production, an IDENTIFIER_NODE is returned. For the `~ class-name'
production a BIT_NOT_EXPR is returned; the operand of the
@@ -6239,7 +6376,8 @@ cp_parser_unqualified_id (cp_parser* parser,
bool template_keyword_p,
bool check_dependency_p,
bool declarator_p,
- bool optional_p)
+ bool optional_p,
+ int looks_like_template_id)
{
cp_token *token;
@@ -6250,18 +6388,21 @@ cp_parser_unqualified_id (cp_parser* parser,
{
case CPP_NAME:
{
- tree id;
-
- /* We don't know yet whether or not this will be a
- template-id. */
- cp_parser_parse_tentatively (parser);
- /* Try a template-id. */
- id = cp_parser_template_id_expr (parser, template_keyword_p,
- check_dependency_p,
- declarator_p);
- /* If it worked, we're done. */
- if (cp_parser_parse_definitely (parser))
- return id;
+ if (looks_like_template_id != -1)
+ {
+ /* We don't know yet whether or not this will be a
+ template-id. */
+ cp_parser_parse_tentatively (parser);
+ /* Try a template-id. */
+ tree id = cp_parser_template_id_expr (parser,
+ template_keyword_p,
+ check_dependency_p,
+ declarator_p,
+ looks_like_template_id);
+ /* If it worked, we're done. */
+ if (cp_parser_parse_definitely (parser))
+ return id;
+ }
/* Otherwise, it's an ordinary identifier. */
return cp_parser_identifier (parser);
}
@@ -6269,7 +6410,8 @@ cp_parser_unqualified_id (cp_parser* parser,
case CPP_TEMPLATE_ID:
return cp_parser_template_id_expr (parser, template_keyword_p,
check_dependency_p,
- declarator_p);
+ declarator_p,
+ looks_like_template_id);
case CPP_COMPL:
{
@@ -11409,7 +11551,7 @@ cp_parser_lambda_declarator_opt (cp_parser* parser, tree lambda_expr)
cp_lexer_consume_token (parser->lexer);
template_param_list = cp_parser_template_parameter_list (parser);
- cp_parser_skip_to_end_of_template_parameter_list (parser);
+ cp_parser_require_end_of_template_parameter_list (parser);
/* We may have a constrained generic lambda; parse the requires-clause
immediately after the template-parameter-list and combine with any
@@ -12290,11 +12432,11 @@ cp_parser_statement (cp_parser* parser, tree in_statement_expr,
{
if (saved_tokens.lexer == lexer)
{
- if (saved_tokens.commit)
+ if (saved_tokens.mode == STS_COMMIT)
cp_lexer_commit_tokens (lexer);
gcc_assert (lexer->saved_tokens.length () == saved_tokens.len);
saved_tokens.lexer = parser->lexer;
- saved_tokens.commit = false;
+ saved_tokens.mode = STS_DONOTHING;
saved_tokens.len = parser->lexer->saved_tokens.length ();
}
cp_lexer_destroy (lexer);
@@ -18021,6 +18163,11 @@ cp_parser_type_parameter (cp_parser* parser, bool *is_parameter_pack)
of functions, returns a TEMPLATE_ID_EXPR. If the template-name
names a class, returns a TYPE_DECL for the specialization.
+ If LOOKS_LIKE_TEMPLATE is 1, then we have already discovered
+ that this looks like a template-id. If -1, we have checked and seen
+ that this does not look like a template-id. If it is 0, then we
+ did not check.
+
If CHECK_DEPENDENCY_P is FALSE, names are looked up in
uninstantiated templates. */
@@ -18029,7 +18176,8 @@ cp_parser_template_id (cp_parser *parser,
bool template_keyword_p,
bool check_dependency_p,
enum tag_types tag_type,
- bool is_declaration)
+ bool is_declaration,
+ int looks_like_template_id)
{
tree templ;
tree arguments;
@@ -18050,10 +18198,11 @@ cp_parser_template_id (cp_parser *parser,
/* Avoid performing name lookup if there is no possibility of
finding a template-id. */
- if ((token->type != CPP_NAME && token->keyword != RID_OPERATOR)
+ if (looks_like_template_id == -1
+ || (token->type != CPP_NAME && token->keyword != RID_OPERATOR)
|| (token->type == CPP_NAME
&& !cp_parser_nth_token_starts_template_argument_list_p
- (parser, 2)))
+ (parser, 2)))
{
cp_parser_error (parser, "expected template-id");
return error_mark_node;
@@ -18290,10 +18439,12 @@ static tree
cp_parser_template_id_expr (cp_parser *parser,
bool template_keyword_p,
bool check_dependency_p,
- bool is_declaration)
+ bool is_declaration,
+ int looks_like_template_id)
{
tree x = cp_parser_template_id (parser, template_keyword_p, check_dependency_p,
- none_type, is_declaration);
+ none_type, is_declaration,
+ looks_like_template_id);
if (TREE_CODE (x) == TEMPLATE_ID_EXPR
&& concept_check_p (x))
/* We didn't check the arguments in cp_parser_template_id; do that now. */
@@ -31414,7 +31565,7 @@ cp_parser_explicit_template_declaration (cp_parser* parser, bool member_p)
}
/* Look for the `>'. */
- cp_parser_skip_to_end_of_template_parameter_list (parser);
+ cp_parser_require_end_of_template_parameter_list (parser);
/* Manage template requirements */
if (flag_concepts)
@@ -31933,7 +32084,7 @@ cp_parser_enclosed_template_argument_list (cp_parser* parser)
}
}
else
- cp_parser_skip_to_end_of_template_parameter_list (parser);
+ cp_parser_require_end_of_template_parameter_list (parser);
/* The `>' token might be a greater-than operator again now. */
parser->greater_than_is_operator_p
= saved_greater_than_is_operator_p;
@@ -32825,11 +32976,44 @@ cp_parser_require (cp_parser* parser,
}
}
-/* An error message is produced if the next token is not '>'.
- All further tokens are skipped until the desired token is
- found or '{', '}', ';' or an unbalanced ')' or ']'. */
+/* Skip an entire parameter list from start to finish. The next token must
+ be the initial "<" of the parameter list. Returns true on success and
+ false otherwise. */
+
+static bool cp_parser_skip_entire_template_parameter_list (cp_parser* parser)
+{
+ /* Consume the "<" because cp_parser_skip_to_end_of_template_parameter_list
+ requires it. */
+ cp_lexer_consume_token (parser->lexer);
+ return cp_parser_skip_to_end_of_template_parameter_list (parser);
+}
+
+/* Ensure we are at the end of a template parameter list. If we are, return.
+ If we are not, something has gone wrong, in which case issue an error and
+ skip to end of the parameter list. */
static void
+cp_parser_require_end_of_template_parameter_list (cp_parser* parser)
+{
+ /* Are we ready, yet? If not, issue error message. */
+ if (cp_parser_require (parser, CPP_GREATER, RT_GREATER))
+ return;
+
+ cp_parser_skip_to_end_of_template_parameter_list (parser);
+}
+
+/* You should only call this function from inside a template parameter list
+ (i.e. the current token should at least be the initial "<" of the
+ parameter list). If you are skipping the entire list, it may be better to
+ use cp_parser_skip_entire_template_parameter_list.
+
+ Tokens are skipped until the final ">" is found, or if we see
+ '{', '}', ';', or if we find an unbalanced ')' or ']'.
+
+ Returns true if we successfully reached the end, and false if
+ something unexpected happened (e.g. end of file). */
+
+static bool
cp_parser_skip_to_end_of_template_parameter_list (cp_parser* parser)
{
/* Current level of '< ... >'. */
@@ -32837,10 +33021,6 @@ cp_parser_skip_to_end_of_template_parameter_list (cp_parser* parser)
/* Ignore '<' and '>' nested inside '( ... )' or '[ ... ]'. */
unsigned nesting_depth = 0;
- /* Are we ready, yet? If not, issue error message. */
- if (cp_parser_require (parser, CPP_GREATER, RT_GREATER))
- return;
-
/* Skip tokens until the desired token is found. */
while (true)
{
@@ -32864,7 +33044,7 @@ cp_parser_skip_to_end_of_template_parameter_list (cp_parser* parser)
spurious. Just consume the `>>' and stop; we've
already produced at least one error. */
cp_lexer_consume_token (parser->lexer);
- return;
+ return false;
}
/* Fall through for C++0x, so we handle the second `>' in
the `>>'. */
@@ -32875,7 +33055,7 @@ cp_parser_skip_to_end_of_template_parameter_list (cp_parser* parser)
{
/* We've reached the token we want, consume it and stop. */
cp_lexer_consume_token (parser->lexer);
- return;
+ return true;
}
break;
@@ -32887,7 +33067,7 @@ cp_parser_skip_to_end_of_template_parameter_list (cp_parser* parser)
case CPP_CLOSE_PAREN:
case CPP_CLOSE_SQUARE:
if (nesting_depth-- == 0)
- return;
+ return false;
break;
case CPP_EOF:
@@ -32896,7 +33076,7 @@ cp_parser_skip_to_end_of_template_parameter_list (cp_parser* parser)
case CPP_OPEN_BRACE:
case CPP_CLOSE_BRACE:
/* The '>' was probably forgotten, don't look further. */
- return;
+ return false;
default:
break;
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 4b1b58318f0..f82288f6f06 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -8928,6 +8928,39 @@ type @samp{T}.
This warning can be disabled with @option{-Wno-missing-requires}.
+@item -Wno-missing-template-keyword
+@opindex Wmissing-template-keyword
+@opindex Wno-missing-template-keyword
+
+The member access tokens ., -> and :: must be followed by the @code{template}
+keyword if the parent object is dependent and the member being named is a
+template.
+
+@smallexample
+template <class X>
+void DoStuff (X x)
+@{
+ x.template DoSomeOtherStuff<X>(); // Good.
+ x.DoMoreStuff<X>(); // Warning, x is dependent.
+@}
+@end smallexample
+
+In rare cases it is possible to get false positives. To silence this, wrap
+the expression in parentheses. For example, the following is treated as a
+template, even where m and N are integers:
+
+@smallexample
+void NotATemplate (my_class t)
+@{
+ int N = 5;
+
+ bool test = t.m < N > (0); // Treated as a template.
+ test = (t.m < N) > (0); // Same meaning, but not treated as a template.
+@}
+@end smallexample
+
+This warning can be disabled with @option{-Wno-missing-template-keyword}.
+
@item -Wno-multichar
@opindex Wno-multichar
@opindex Wmultichar
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic-mem_fn2.C b/gcc/testsuite/g++.dg/cpp0x/variadic-mem_fn2.C
index 4a02ab22990..c343f32086d 100644
--- a/gcc/testsuite/g++.dg/cpp0x/variadic-mem_fn2.C
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic-mem_fn2.C
@@ -5,5 +5,6 @@ template <class A0, class... As> struct tuple
tuple<As...> tail;
template <int Offset, class... More> int apply(const More&... more) {
return tail.apply<1>(more...); // { dg-error "" } needs .template
+ // { dg-warning "keyword before dependent template name" "" { target *-*-* } .-1 }
}
};
diff --git a/gcc/testsuite/g++.dg/template/dependent-name17.C b/gcc/testsuite/g++.dg/template/dependent-name17.C
new file mode 100644
index 00000000000..3d16a49d940
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/dependent-name17.C
@@ -0,0 +1,51 @@
+// C++ PR 70317
+// { dg-do compile }
+
+template<class T> class mytemplateclass
+{
+ public:
+ template<class U> void class_func() {}
+ template<class U> static void class_func_static() {}
+};
+
+class myclass
+{
+ public:
+ int testint;
+ template<class U> void class_func() {}
+ template<class U> static void class_func_static() {}
+};
+
+template<class Y> void tests_func(mytemplateclass<Y> c, myclass c2)
+{
+ /* Dependent template accessors (ill-formed code). */
+ c.class_func<Y>(); // { dg-warning "keyword before dependent template name" }
+ // { dg-error "expected primary-expression" "" { target { *-*-* } } .-1 }
+ (&c)->class_func<Y>(); // { dg-warning "keyword before dependent template name" }
+ // { dg-error "expected primary-expression" "" { target { *-*-* } } .-1 }
+ mytemplateclass<Y>::class_func_static<Y>(); // { dg-warning "keyword before dependent template name" }
+ // { dg-error "expected primary-expression" "" { target { *-*-* } } .-1 }
+
+ /* Dependent template accessors (well-formed code). */
+ c.template class_func<Y>();
+ (&c)->template class_func<Y>();
+ mytemplateclass<Y>::template class_func_static<Y>();
+
+ /* Non-dependent template accessors (well-formed code). */
+ c2.class_func<myclass>();
+ (&c2)->class_func<myclass>();
+ myclass::class_func_static<myclass>();
+}
+
+int main()
+{
+ mytemplateclass<myclass> c;
+ myclass c2;
+ tests_func<myclass>(c, c2);
+
+ c2.testint = 53;
+ /* Make sure this isn't treated as a template. */
+ bool testbool = c2.testint < 999 > 7;
+ /* This probably will be treated as a template initially but it should still work. */
+ testbool = c2.testint < 123 > (50);
+}
\ No newline at end of file
--
2.33.0
