jkorous-apple created this revision.
What I would love to have is FixIt for this kind of situation:
template <class T> void foo() {
T::bar<int*>(); /* missing template keyword */
}
which currently gets not that helpful diagnostics:
> clang misleading.cpp
misleading.cpp:2:15: error: expected '(' for function-style cast or type
construction
T::bar<int*>();
~~~^
misleading.cpp:2:16: error: expected expression
T::bar<int*>();
^
misleading.cpp:2:18: error: expected expression
T::bar<int*>();
^
3 errors generated.
while
template <class T> void foo() {
T::bar<int>();
}
gets a good one:
good.cpp:2:8: error: use 'template' keyword to treat 'bar' as a dependent
template name
T::bar<int>();
^
template
1 error generated.
If I understand it correctly than current implementation of
bool Parser::IsTemplateArgumentList(unsigned Skip)
relies on the fact that if there's a '<' token followed by declaration
specifier it has to be a template. Which is true but produces false negatives
for non-type parameters (array<1>) or pointer type parameters (vector<int*>)
(among others).
One particular thing to keep on mind is that there's the [over.over] rule about
resolving names to function pointers - best explained by test
test/SemaTemplate/dependent-template-recover.cpp (look for snippet below):
// Note: no diagnostic here, this is actually valid as a comparison between
// the decayed pointer to Y::g<> and mb!
T::g<mb>(0);
What I did so far is that in case the simple approach doesn't validate there's
a template I try to use speculative parsing. The only problem remaining is
diagnostics - I am able to use TentativeParsingAction which is later reverted
but I still get all the diagnostics which is incorrect in case my speculation
about how to parse the code was wrong.
https://reviews.llvm.org/D43331
Files:
FixIt/fixit-template-for-dependent-name.cpp
Parse/ParseTemplate.cpp
SemaTemplate/dependent-template-recover.cpp
Index: SemaTemplate/dependent-template-recover.cpp
===================================================================
--- SemaTemplate/dependent-template-recover.cpp
+++ SemaTemplate/dependent-template-recover.cpp
@@ -6,6 +6,7 @@
t->f0<int>(); // expected-error{{use 'template' keyword to treat 'f0' as a
dependent template name}}
t->operator+<U const, 1>(); // expected-error{{use 'template' keyword to
treat 'operator +' as a dependent template name}}
+ t->operator+<1, U const>(); // expected-error{{use 'template' keyword to
treat 'operator +' as a dependent template name}}
t->f1<int const, 2>(); // expected-error{{use 'template' keyword to treat
'f1' as a dependent template name}}
T::getAs<U>(); // expected-error{{use 'template' keyword to treat 'getAs'
as a dependent template name}}
Index: FixIt/fixit-template-for-dependent-name.cpp
===================================================================
--- /dev/null
+++ FixIt/fixit-template-for-dependent-name.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+template <class T> void foo() {
+ T::bar<int>(); // expected-error {{use 'template' keyword to treat 'bar'
as a dependent template name}}
+ T::baz<int*>(); // expected-error {{use 'template' keyword to treat 'baz'
as a dependent template name}}
+}
+
+// RUN: not %clang_cc1 -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 |
FileCheck %s
+// CHECK: fix-it:{{.*}}:{4:8-4:8}:"template "
+// CHECK: fix-it:{{.*}}:{5:8-5:8}:"template "
Index: Parse/ParseTemplate.cpp
===================================================================
--- Parse/ParseTemplate.cpp
+++ Parse/ParseTemplate.cpp
@@ -1247,30 +1247,45 @@
/// template argument list (starting with the '<') and never as a '<'
/// expression.
bool Parser::IsTemplateArgumentList(unsigned Skip) {
- struct AlwaysRevertAction : TentativeParsingAction {
- AlwaysRevertAction(Parser &P) : TentativeParsingAction(P) { }
- ~AlwaysRevertAction() { Revert(); }
- } Tentative(*this);
-
- while (Skip) {
- ConsumeAnyToken();
- --Skip;
+ {
+ RevertingTentativeParsingAction AutoRevertScopeGuard(*this);
+
+ for (; Skip > 0; --Skip) {
+ ConsumeToken();
+ }
+
+ if (!TryConsumeToken(tok::less))
+ return false;
+
+ // If the first wannabe template argument consists only of decl specs
+ // it's a template argument list indeed.
+
+ // See whether we have declaration specifiers, which indicate a type.
+ while (isCXXDeclarationSpecifier() == TPResult::True)
+ ConsumeAnyToken();
+
+ // If we have a '>' or a ',' then this is a template argument list.
+ if (Tok.isOneOf(tok::greater, tok::comma))
+ return true;
}
-
- // '<'
+
+ RevertingTentativeParsingAction AutoRevertScopeGuard(*this);
+
+ for (; Skip > 0; --Skip) {
+ ConsumeToken();
+ }
+
if (!TryConsumeToken(tok::less))
return false;
- // An empty template argument list.
- if (Tok.is(tok::greater))
- return true;
-
- // See whether we have declaration specifiers, which indicate a type.
- while (isCXXDeclarationSpecifier() == TPResult::True)
- ConsumeAnyToken();
-
- // If we have a '>' or a ',' then this is a template argument list.
- return Tok.isOneOf(tok::greater, tok::comma);
+ TemplateArgList ignore;
+ if (ParseTemplateArgumentList(ignore))
+ return false;
+
+ if (!TryConsumeToken(tok::greater))
+ return false;
+
+ return true;
}
/// ParseTemplateArgumentList - Parse a C++ template-argument-list
Index: SemaTemplate/dependent-template-recover.cpp
===================================================================
--- SemaTemplate/dependent-template-recover.cpp
+++ SemaTemplate/dependent-template-recover.cpp
@@ -6,6 +6,7 @@
t->f0<int>(); // expected-error{{use 'template' keyword to treat 'f0' as a dependent template name}}
t->operator+<U const, 1>(); // expected-error{{use 'template' keyword to treat 'operator +' as a dependent template name}}
+ t->operator+<1, U const>(); // expected-error{{use 'template' keyword to treat 'operator +' as a dependent template name}}
t->f1<int const, 2>(); // expected-error{{use 'template' keyword to treat 'f1' as a dependent template name}}
T::getAs<U>(); // expected-error{{use 'template' keyword to treat 'getAs' as a dependent template name}}
Index: FixIt/fixit-template-for-dependent-name.cpp
===================================================================
--- /dev/null
+++ FixIt/fixit-template-for-dependent-name.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+template <class T> void foo() {
+ T::bar<int>(); // expected-error {{use 'template' keyword to treat 'bar' as a dependent template name}}
+ T::baz<int*>(); // expected-error {{use 'template' keyword to treat 'baz' as a dependent template name}}
+}
+
+// RUN: not %clang_cc1 -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// CHECK: fix-it:{{.*}}:{4:8-4:8}:"template "
+// CHECK: fix-it:{{.*}}:{5:8-5:8}:"template "
Index: Parse/ParseTemplate.cpp
===================================================================
--- Parse/ParseTemplate.cpp
+++ Parse/ParseTemplate.cpp
@@ -1247,30 +1247,45 @@
/// template argument list (starting with the '<') and never as a '<'
/// expression.
bool Parser::IsTemplateArgumentList(unsigned Skip) {
- struct AlwaysRevertAction : TentativeParsingAction {
- AlwaysRevertAction(Parser &P) : TentativeParsingAction(P) { }
- ~AlwaysRevertAction() { Revert(); }
- } Tentative(*this);
-
- while (Skip) {
- ConsumeAnyToken();
- --Skip;
+ {
+ RevertingTentativeParsingAction AutoRevertScopeGuard(*this);
+
+ for (; Skip > 0; --Skip) {
+ ConsumeToken();
+ }
+
+ if (!TryConsumeToken(tok::less))
+ return false;
+
+ // If the first wannabe template argument consists only of decl specs
+ // it's a template argument list indeed.
+
+ // See whether we have declaration specifiers, which indicate a type.
+ while (isCXXDeclarationSpecifier() == TPResult::True)
+ ConsumeAnyToken();
+
+ // If we have a '>' or a ',' then this is a template argument list.
+ if (Tok.isOneOf(tok::greater, tok::comma))
+ return true;
}
-
- // '<'
+
+ RevertingTentativeParsingAction AutoRevertScopeGuard(*this);
+
+ for (; Skip > 0; --Skip) {
+ ConsumeToken();
+ }
+
if (!TryConsumeToken(tok::less))
return false;
- // An empty template argument list.
- if (Tok.is(tok::greater))
- return true;
-
- // See whether we have declaration specifiers, which indicate a type.
- while (isCXXDeclarationSpecifier() == TPResult::True)
- ConsumeAnyToken();
-
- // If we have a '>' or a ',' then this is a template argument list.
- return Tok.isOneOf(tok::greater, tok::comma);
+ TemplateArgList ignore;
+ if (ParseTemplateArgumentList(ignore))
+ return false;
+
+ if (!TryConsumeToken(tok::greater))
+ return false;
+
+ return true;
}
/// ParseTemplateArgumentList - Parse a C++ template-argument-list
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
