[clang] Distinguish unresolved templates in UnresolvedLookupExpr #89019
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This patch revolves around the misuse of UnresolvedLookupExpr in BuildTemplateIdExpr. Basically, we build up an UnresolvedLookupExpr not only for function overloads but for "unresolved" templates wherever we need an expression for template decls. For example, a dependent VarTemplateDecl can be wrapped with such an expression before template instantiation. (See llvm@6170072) Also, one important thing is that UnresolvedLookupExpr uses a "canonical" QualType to describe the containing unresolved decls: a DependentTy is for dependent expressions and an OverloadTy otherwise. Therefore, this modeling for non-dependent templates leaves a problem in that the expression is marked and perceived as if describing overload functions. The consumer then expects functions for every such expression, although the fact is the reverse. Hence, we run into crashes. As to the patch, I added a new canonical type "UnresolvedTemplateTy" to model these cases. Given that we have been using this model (intentionally or accidentally) and it is pretty baked in throughout the code, I think extending the role of UnresolvedLookupExpr is reasonable. Further, I added some diagnostics for the direct occurrence of these expressions, which are supposed to be ill-formed. As a bonus, this patch also fixes some typos in the diagnostics and creates RecoveryExprs rather than nothing in the hope of a better error-recovery for clangd. Fixes llvm#88832 Fixes llvm#63243 Fixes llvm#48673
zyn0217
commented
Apr 17, 2024
| // expected-note@#add_pointer {{'add_pointer' declared here}} | ||
| // expected-error@#ill-formed-decl {{expected ';' after expression}} | ||
| // expected-error@#ill-formed-decl {{no type named 'type' in the global namespace}} | ||
| // expected-error@#ill-formed-decl {{'std::add_pointer' is expected to be a non-type template, but instantiated to a class template}} |
There was a problem hiding this comment.
RFC: this diagnostic is probably not optimal (and maybe confusing), since CheckPlaceholderExpr is called almost everywhere apart from a full expression. I was considering moving the diagnostic out of that function, but that requires much work I think. (and I'm a bit lazy :)
There was a problem hiding this comment.
This is a case where just doing the -N positioning is fine, since it is on the same thing here.
As far as this diagnostic, can you spend a little time to evaluate how much work it is to fix that? It is particularly awkward.
There was a problem hiding this comment.
As far as this diagnostic, can you spend a little time to evaluate how much work it is to fix that? It is particularly awkward.
I have looked into it a bit more, and I found it is a consequence of not handling TRANSFORM_TYPE_TRAITs as scope specifiers, which was introduced in e9ef456.
Let's explain the difference with an example:
namespace std {
struct add_pointer {};
}
void foo() {
std::__add_pointer<int>::type ptr;
std::Add_pointer<int>::type ptr2;
}we would end up seeing different diagnostics on two declarations ptr and ptr2: the first is what was crashing previously, because __add_pointer is actually a keyword rather than an identifier like Add_pointer. As a result, we would bail out when we see __add_pointer in ParseOptionalCXXScopeSpecifier, and we would continue parsing it as a TemplateIdExpr, which gave us a crash as well as a spurious diagnostic saying we're missing a semicolon after <int>.
For comparison, the second case would keep parsing until we encounter the semicolon after ptr2. This results in a better diagnostic even after the typo correction.
I think a fix would be to add a handling logic to ParseOptionalCXXScopeSpecifier like what we did in e9ef456:
switch (Tok.getKind()) {
#define TRANSFORM_TYPE_TRAIT_DEF(_, Trait) case tok::kw___##Trait:
#include "clang/Basic/TransformTypeTraits.def"
if (NextToken().is(tok::less)) {
Tok.setKind(tok::identifier);
Diag(Tok, diag::ext_keyword_as_ident)
<< Tok.getIdentifierInfo()->getName() << 0;
continue;
}
LLVM_FALLTHROUGH;
default:
break;
}There was a problem hiding this comment.
I think that would be OK? But @cor3ntin would be our expert there.
There was a problem hiding this comment.
@cor3ntin do you have any thoughts here? thanks!
|
The pre-commit CI is still clogging after a few hours, and I'm opening it for feedback anyway. |
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Younan Zhang (zyn0217) ChangesThis patch revolves around the misuse of UnresolvedLookupExpr in Basically, we build up an UnresolvedLookupExpr not only for function Also, one important thing is that UnresolvedLookupExpr uses a "canonical" As to the patch, I added a new canonical type "UnresolvedTemplateTy" to As a bonus, this patch also fixes some typos in the diagnostics and creates Fixes #88832 Full diff: https://github.com/llvm/llvm-project/pull/89019.diff 14 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index efc32212f300cf..8c50988083faa6 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -535,6 +535,7 @@ Bug Fixes to C++ Support
- Fix a crash when deducing ``auto`` from an invalid dereference (#GH88329).
- Fix a crash in requires expression with templated base class member function. Fixes (#GH84020).
- Placement new initializes typedef array with correct size (#GH41441)
+- Fixed a misuse of ``UnresolvedLookupExpr`` for ill-formed templated expressions. Fixes (#GH48673), (#GH63243) and (#GH88832).
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index 28f8d67811f0a2..e9a22f04cfe764 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -1116,7 +1116,8 @@ class ASTContext : public RefCountedBase<ASTContext> {
CanQualType BFloat16Ty;
CanQualType Float16Ty; // C11 extension ISO/IEC TS 18661-3
CanQualType VoidPtrTy, NullPtrTy;
- CanQualType DependentTy, OverloadTy, BoundMemberTy, UnknownAnyTy;
+ CanQualType DependentTy, OverloadTy, BoundMemberTy, UnresolvedTemplateTy,
+ UnknownAnyTy;
CanQualType BuiltinFnTy;
CanQualType PseudoObjectTy, ARCUnbridgedCastTy;
CanQualType ObjCBuiltinIdTy, ObjCBuiltinClassTy, ObjCBuiltinSelTy;
diff --git a/clang/include/clang/AST/BuiltinTypes.def b/clang/include/clang/AST/BuiltinTypes.def
index c04f6f6f127191..fd0cc10be8ebca 100644
--- a/clang/include/clang/AST/BuiltinTypes.def
+++ b/clang/include/clang/AST/BuiltinTypes.def
@@ -285,6 +285,9 @@ PLACEHOLDER_TYPE(Overload, OverloadTy)
// x->foo # if only contains non-static members
PLACEHOLDER_TYPE(BoundMember, BoundMemberTy)
+// The type of an unresolved template. Used in UnresolvedLookupExpr.
+PLACEHOLDER_TYPE(UnresolvedTemplate, UnresolvedTemplateTy)
+
// The type of an expression which refers to a pseudo-object,
// such as those introduced by Objective C's @property or
// VS.NET's __property declarations. A placeholder type. The
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h
index 500098dd3dab1d..2f0a122fdfde14 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -1099,6 +1099,9 @@ enum PredefinedTypeIDs {
// \brief WebAssembly reference types with auto numeration
#define WASM_TYPE(Name, Id, SingletonId) PREDEF_TYPE_##Id##_ID,
#include "clang/Basic/WebAssemblyReferenceTypes.def"
+
+ /// The placeholder type for unresolved templates.
+ PREDEF_TYPE_UNRESOLVED_TEMPLATE,
// Sentinel value. Considered a predefined type but not useable as one.
PREDEF_TYPE_LAST_ID
};
@@ -1108,7 +1111,7 @@ enum PredefinedTypeIDs {
///
/// Type IDs for non-predefined types will start at
/// NUM_PREDEF_TYPE_IDs.
-const unsigned NUM_PREDEF_TYPE_IDS = 502;
+const unsigned NUM_PREDEF_TYPE_IDS = 503;
// Ensure we do not overrun the predefined types we reserved
// in the enum PredefinedTypeIDs above.
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 6ce233704a5885..e9fba116cff559 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -1306,6 +1306,9 @@ void ASTContext::InitBuiltinTypes(const TargetInfo &Target,
// Placeholder type for bound members.
InitBuiltinType(BoundMemberTy, BuiltinType::BoundMember);
+ // Placeholder type for unresolved templates.
+ InitBuiltinType(UnresolvedTemplateTy, BuiltinType::UnresolvedTemplate);
+
// Placeholder type for pseudo-objects.
InitBuiltinType(PseudoObjectTy, BuiltinType::PseudoObject);
diff --git a/clang/lib/AST/NSAPI.cpp b/clang/lib/AST/NSAPI.cpp
index ecc56c13fb7573..4e09b27b0f76cd 100644
--- a/clang/lib/AST/NSAPI.cpp
+++ b/clang/lib/AST/NSAPI.cpp
@@ -454,6 +454,7 @@ NSAPI::getNSNumberFactoryMethodKind(QualType T) const {
#define WASM_TYPE(Name, Id, SingletonId) case BuiltinType::Id:
#include "clang/Basic/WebAssemblyReferenceTypes.def"
case BuiltinType::BoundMember:
+ case BuiltinType::UnresolvedTemplate:
case BuiltinType::Dependent:
case BuiltinType::Overload:
case BuiltinType::UnknownAny:
diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index cb22c91a12aa89..4751d73184262f 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -3381,6 +3381,8 @@ StringRef BuiltinType::getName(const PrintingPolicy &Policy) const {
return "<overloaded function type>";
case BoundMember:
return "<bound member function type>";
+ case UnresolvedTemplate:
+ return "<unresolved template type>";
case PseudoObject:
return "<pseudo-object type>";
case Dependent:
@@ -4673,6 +4675,7 @@ bool Type::canHaveNullability(bool ResultIfUnknown) const {
#include "clang/AST/BuiltinTypes.def"
return false;
+ case BuiltinType::UnresolvedTemplate:
// Dependent types that could instantiate to a pointer type.
case BuiltinType::Dependent:
case BuiltinType::Overload:
diff --git a/clang/lib/AST/TypeLoc.cpp b/clang/lib/AST/TypeLoc.cpp
index 21e152f6aea8a0..e8cd7fd7b35b6f 100644
--- a/clang/lib/AST/TypeLoc.cpp
+++ b/clang/lib/AST/TypeLoc.cpp
@@ -399,6 +399,7 @@ TypeSpecifierType BuiltinTypeLoc::getWrittenTypeSpec() const {
case BuiltinType::NullPtr:
case BuiltinType::Overload:
case BuiltinType::Dependent:
+ case BuiltinType::UnresolvedTemplate:
case BuiltinType::BoundMember:
case BuiltinType::UnknownAny:
case BuiltinType::ARCUnbridgedCast:
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 7c3faba0f78819..8fe3e2c771f1bf 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -6349,6 +6349,7 @@ static bool isPlaceholderToRemoveAsArg(QualType type) {
#include "clang/AST/BuiltinTypes.def"
return false;
+ case BuiltinType::UnresolvedTemplate:
// We cannot lower out overload sets; they might validly be resolved
// by the call machinery.
case BuiltinType::Overload:
@@ -21234,6 +21235,24 @@ ExprResult Sema::CheckPlaceholderExpr(Expr *E) {
if (!placeholderType) return E;
switch (placeholderType->getKind()) {
+ case BuiltinType::UnresolvedTemplate: {
+ auto *ULE = cast<UnresolvedLookupExpr>(E);
+ const DeclarationNameInfo &NameInfo = ULE->getNameInfo();
+ NestedNameSpecifierLoc Loc = ULE->getQualifierLoc();
+ NamedDecl *Temp = *ULE->decls_begin();
+ bool IsTypeAliasTemplateDecl = isa<TypeAliasTemplateDecl>(Temp);
+ if (NestedNameSpecifier *NNS = Loc.getNestedNameSpecifier())
+ Diag(NameInfo.getLoc(), diag::err_template_kw_refers_to_type_template)
+ << NNS << NameInfo.getName().getAsString() << Loc.getSourceRange()
+ << IsTypeAliasTemplateDecl;
+ else
+ Diag(NameInfo.getLoc(), diag::err_template_kw_refers_to_type_template)
+ << "" << NameInfo.getName().getAsString() << Loc.getSourceRange()
+ << IsTypeAliasTemplateDecl;
+ Diag(Temp->getLocation(), diag::note_referenced_type_template)
+ << IsTypeAliasTemplateDecl;
+ return CreateRecoveryExpr(NameInfo.getBeginLoc(), NameInfo.getEndLoc(), {});
+ }
// Overloaded expressions.
case BuiltinType::Overload: {
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 95171359f0ab17..14768ca66ac1c5 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -5553,7 +5553,7 @@ ExprResult Sema::BuildTemplateIdExpr(const CXXScopeSpec &SS,
R.getRepresentativeDecl(), TemplateKWLoc, TemplateArgs);
if (Res.isInvalid() || Res.isUsable())
return Res;
- // Result is dependent. Carry on to build an UnresolvedLookupEpxr.
+ // Result is dependent. Carry on to build an UnresolvedLookupExpr.
KnownDependent = true;
}
@@ -5571,6 +5571,12 @@ ExprResult Sema::BuildTemplateIdExpr(const CXXScopeSpec &SS,
TemplateKWLoc, R.getLookupNameInfo(), RequiresADL, TemplateArgs,
R.begin(), R.end(), KnownDependent);
+ // Model the templates with UnresolvedTemplateTy. The expression should then
+ // either be transformed in an instantiation or diagnosed.
+ if (ULE->getType() == Context.OverloadTy && R.isSingleResult() &&
+ !R.getFoundDecl()->getAsFunction())
+ ULE->setType(Context.UnresolvedTemplateTy);
+
return ULE;
}
@@ -5609,8 +5615,9 @@ Sema::BuildQualifiedTemplateIdExpr(CXXScopeSpec &SS,
Diag(NameInfo.getLoc(), diag::err_template_kw_refers_to_type_template)
<< SS.getScopeRep() << NameInfo.getName().getAsString() << SS.getRange()
<< isTypeAliasTemplateDecl;
- Diag(Temp->getLocation(), diag::note_referenced_type_template) << 0;
- return ExprError();
+ Diag(Temp->getLocation(), diag::note_referenced_type_template)
+ << isTypeAliasTemplateDecl;
+ return CreateRecoveryExpr(NameInfo.getBeginLoc(), NameInfo.getEndLoc(), {});
};
if (ClassTemplateDecl *Temp = R.getAsSingle<ClassTemplateDecl>())
diff --git a/clang/lib/Serialization/ASTCommon.cpp b/clang/lib/Serialization/ASTCommon.cpp
index f8d54c0c398906..cd2d40a2c65fcb 100644
--- a/clang/lib/Serialization/ASTCommon.cpp
+++ b/clang/lib/Serialization/ASTCommon.cpp
@@ -186,6 +186,9 @@ serialization::TypeIdxFromBuiltin(const BuiltinType *BT) {
case BuiltinType::Overload:
ID = PREDEF_TYPE_OVERLOAD_ID;
break;
+ case BuiltinType::UnresolvedTemplate:
+ ID = PREDEF_TYPE_UNRESOLVED_TEMPLATE;
+ break;
case BuiltinType::BoundMember:
ID = PREDEF_TYPE_BOUND_MEMBER;
break;
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index b28df03b4a95e9..445c042330194b 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -7311,6 +7311,9 @@ QualType ASTReader::GetType(TypeID ID) {
case PREDEF_TYPE_OVERLOAD_ID:
T = Context.OverloadTy;
break;
+ case PREDEF_TYPE_UNRESOLVED_TEMPLATE:
+ T = Context.UnresolvedTemplateTy;
+ break;
case PREDEF_TYPE_BOUND_MEMBER:
T = Context.BoundMemberTy;
break;
diff --git a/clang/test/SemaCXX/PR62533.cpp b/clang/test/SemaCXX/PR62533.cpp
index 920ea54d4b00ed..0753156813f8e7 100644
--- a/clang/test/SemaCXX/PR62533.cpp
+++ b/clang/test/SemaCXX/PR62533.cpp
@@ -2,7 +2,7 @@
template<typename T>
struct test {
- template<typename> using fun_diff = char; // expected-note 2{{class template declared here}}
+ template<typename> using fun_diff = char; // expected-note 2{{type alias template declared here}}
};
template<typename T, typename V>
diff --git a/clang/test/SemaTemplate/template-id-expr.cpp b/clang/test/SemaTemplate/template-id-expr.cpp
index 0555d8b94504fb..741742d9e5d7eb 100644
--- a/clang/test/SemaTemplate/template-id-expr.cpp
+++ b/clang/test/SemaTemplate/template-id-expr.cpp
@@ -186,3 +186,74 @@ class E {
#endif
template<typename T> using D = int; // expected-note {{declared here}}
E<D> ed; // expected-note {{instantiation of}}
+
+namespace non_functions {
+
+#if __cplusplus >= 201103L
+namespace PR88832 {
+template <typename T> struct O {
+ static const T v = 0;
+};
+
+struct P {
+ template <typename T> using I = typename O<T>::v; // #TypeAlias
+};
+
+struct Q {
+ template <typename T> int foo() {
+ return T::template I<int>; // expected-error {{'P::I' is expected to be a non-type template, but instantiated to a type alias template}}
+ // expected-note@#TypeAlias {{type alias template declared here}}
+ }
+};
+
+int bar() {
+ return Q().foo<P>(); // expected-note-re {{function template specialization {{.*}} requested here}}
+}
+
+} // namespace PR88832
+#endif
+
+namespace PR63243 {
+
+namespace std {
+template <class T> struct add_pointer { // #add_pointer
+};
+} // namespace std
+
+class A {};
+
+int main() {
+ std::__add_pointer<A>::type ptr; // #ill-formed-decl
+ // expected-error@#ill-formed-decl {{no template named '__add_pointer'}}
+ // expected-note@#add_pointer {{'add_pointer' declared here}}
+ // expected-error@#ill-formed-decl {{expected ';' after expression}}
+ // expected-error@#ill-formed-decl {{no type named 'type' in the global namespace}}
+ // expected-error@#ill-formed-decl {{'std::add_pointer' is expected to be a non-type template, but instantiated to a class template}}
+ // expected-note@#add_pointer {{class template declared here}}
+
+ // expected-warning@#ill-formed-decl {{keyword '__add_pointer' will be made available as an identifier here}}
+}
+
+} // namespace PR63243
+
+namespace PR48673 {
+
+template <typename T> struct C {
+ template <int TT> class Type {}; // #ClassTemplate
+};
+
+template <typename T1> struct A {
+ void foo() {
+ C<T1>::template Type<2>; // #templated-decl-as-expression
+ // expected-error@#templated-decl-as-expression {{'C<float>::Type' is expected to be a non-type template, but instantiated to a class template}}}
+ // expected-note@#ClassTemplate {{class template declared here}}
+ }
+};
+
+void test() {
+ A<float>().foo(); // expected-note-re {{instantiation of member function {{.*}} requested here}}
+}
+
+} // namespace PR48673
+
+}
|
|
@llvm/pr-subscribers-clang-modules Author: Younan Zhang (zyn0217) ChangesThis patch revolves around the misuse of UnresolvedLookupExpr in Basically, we build up an UnresolvedLookupExpr not only for function Also, one important thing is that UnresolvedLookupExpr uses a "canonical" As to the patch, I added a new canonical type "UnresolvedTemplateTy" to As a bonus, this patch also fixes some typos in the diagnostics and creates Fixes #88832 Full diff: https://github.com/llvm/llvm-project/pull/89019.diff 14 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index efc32212f300cf..8c50988083faa6 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -535,6 +535,7 @@ Bug Fixes to C++ Support
- Fix a crash when deducing ``auto`` from an invalid dereference (#GH88329).
- Fix a crash in requires expression with templated base class member function. Fixes (#GH84020).
- Placement new initializes typedef array with correct size (#GH41441)
+- Fixed a misuse of ``UnresolvedLookupExpr`` for ill-formed templated expressions. Fixes (#GH48673), (#GH63243) and (#GH88832).
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index 28f8d67811f0a2..e9a22f04cfe764 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -1116,7 +1116,8 @@ class ASTContext : public RefCountedBase<ASTContext> {
CanQualType BFloat16Ty;
CanQualType Float16Ty; // C11 extension ISO/IEC TS 18661-3
CanQualType VoidPtrTy, NullPtrTy;
- CanQualType DependentTy, OverloadTy, BoundMemberTy, UnknownAnyTy;
+ CanQualType DependentTy, OverloadTy, BoundMemberTy, UnresolvedTemplateTy,
+ UnknownAnyTy;
CanQualType BuiltinFnTy;
CanQualType PseudoObjectTy, ARCUnbridgedCastTy;
CanQualType ObjCBuiltinIdTy, ObjCBuiltinClassTy, ObjCBuiltinSelTy;
diff --git a/clang/include/clang/AST/BuiltinTypes.def b/clang/include/clang/AST/BuiltinTypes.def
index c04f6f6f127191..fd0cc10be8ebca 100644
--- a/clang/include/clang/AST/BuiltinTypes.def
+++ b/clang/include/clang/AST/BuiltinTypes.def
@@ -285,6 +285,9 @@ PLACEHOLDER_TYPE(Overload, OverloadTy)
// x->foo # if only contains non-static members
PLACEHOLDER_TYPE(BoundMember, BoundMemberTy)
+// The type of an unresolved template. Used in UnresolvedLookupExpr.
+PLACEHOLDER_TYPE(UnresolvedTemplate, UnresolvedTemplateTy)
+
// The type of an expression which refers to a pseudo-object,
// such as those introduced by Objective C's @property or
// VS.NET's __property declarations. A placeholder type. The
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h
index 500098dd3dab1d..2f0a122fdfde14 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -1099,6 +1099,9 @@ enum PredefinedTypeIDs {
// \brief WebAssembly reference types with auto numeration
#define WASM_TYPE(Name, Id, SingletonId) PREDEF_TYPE_##Id##_ID,
#include "clang/Basic/WebAssemblyReferenceTypes.def"
+
+ /// The placeholder type for unresolved templates.
+ PREDEF_TYPE_UNRESOLVED_TEMPLATE,
// Sentinel value. Considered a predefined type but not useable as one.
PREDEF_TYPE_LAST_ID
};
@@ -1108,7 +1111,7 @@ enum PredefinedTypeIDs {
///
/// Type IDs for non-predefined types will start at
/// NUM_PREDEF_TYPE_IDs.
-const unsigned NUM_PREDEF_TYPE_IDS = 502;
+const unsigned NUM_PREDEF_TYPE_IDS = 503;
// Ensure we do not overrun the predefined types we reserved
// in the enum PredefinedTypeIDs above.
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 6ce233704a5885..e9fba116cff559 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -1306,6 +1306,9 @@ void ASTContext::InitBuiltinTypes(const TargetInfo &Target,
// Placeholder type for bound members.
InitBuiltinType(BoundMemberTy, BuiltinType::BoundMember);
+ // Placeholder type for unresolved templates.
+ InitBuiltinType(UnresolvedTemplateTy, BuiltinType::UnresolvedTemplate);
+
// Placeholder type for pseudo-objects.
InitBuiltinType(PseudoObjectTy, BuiltinType::PseudoObject);
diff --git a/clang/lib/AST/NSAPI.cpp b/clang/lib/AST/NSAPI.cpp
index ecc56c13fb7573..4e09b27b0f76cd 100644
--- a/clang/lib/AST/NSAPI.cpp
+++ b/clang/lib/AST/NSAPI.cpp
@@ -454,6 +454,7 @@ NSAPI::getNSNumberFactoryMethodKind(QualType T) const {
#define WASM_TYPE(Name, Id, SingletonId) case BuiltinType::Id:
#include "clang/Basic/WebAssemblyReferenceTypes.def"
case BuiltinType::BoundMember:
+ case BuiltinType::UnresolvedTemplate:
case BuiltinType::Dependent:
case BuiltinType::Overload:
case BuiltinType::UnknownAny:
diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index cb22c91a12aa89..4751d73184262f 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -3381,6 +3381,8 @@ StringRef BuiltinType::getName(const PrintingPolicy &Policy) const {
return "<overloaded function type>";
case BoundMember:
return "<bound member function type>";
+ case UnresolvedTemplate:
+ return "<unresolved template type>";
case PseudoObject:
return "<pseudo-object type>";
case Dependent:
@@ -4673,6 +4675,7 @@ bool Type::canHaveNullability(bool ResultIfUnknown) const {
#include "clang/AST/BuiltinTypes.def"
return false;
+ case BuiltinType::UnresolvedTemplate:
// Dependent types that could instantiate to a pointer type.
case BuiltinType::Dependent:
case BuiltinType::Overload:
diff --git a/clang/lib/AST/TypeLoc.cpp b/clang/lib/AST/TypeLoc.cpp
index 21e152f6aea8a0..e8cd7fd7b35b6f 100644
--- a/clang/lib/AST/TypeLoc.cpp
+++ b/clang/lib/AST/TypeLoc.cpp
@@ -399,6 +399,7 @@ TypeSpecifierType BuiltinTypeLoc::getWrittenTypeSpec() const {
case BuiltinType::NullPtr:
case BuiltinType::Overload:
case BuiltinType::Dependent:
+ case BuiltinType::UnresolvedTemplate:
case BuiltinType::BoundMember:
case BuiltinType::UnknownAny:
case BuiltinType::ARCUnbridgedCast:
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 7c3faba0f78819..8fe3e2c771f1bf 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -6349,6 +6349,7 @@ static bool isPlaceholderToRemoveAsArg(QualType type) {
#include "clang/AST/BuiltinTypes.def"
return false;
+ case BuiltinType::UnresolvedTemplate:
// We cannot lower out overload sets; they might validly be resolved
// by the call machinery.
case BuiltinType::Overload:
@@ -21234,6 +21235,24 @@ ExprResult Sema::CheckPlaceholderExpr(Expr *E) {
if (!placeholderType) return E;
switch (placeholderType->getKind()) {
+ case BuiltinType::UnresolvedTemplate: {
+ auto *ULE = cast<UnresolvedLookupExpr>(E);
+ const DeclarationNameInfo &NameInfo = ULE->getNameInfo();
+ NestedNameSpecifierLoc Loc = ULE->getQualifierLoc();
+ NamedDecl *Temp = *ULE->decls_begin();
+ bool IsTypeAliasTemplateDecl = isa<TypeAliasTemplateDecl>(Temp);
+ if (NestedNameSpecifier *NNS = Loc.getNestedNameSpecifier())
+ Diag(NameInfo.getLoc(), diag::err_template_kw_refers_to_type_template)
+ << NNS << NameInfo.getName().getAsString() << Loc.getSourceRange()
+ << IsTypeAliasTemplateDecl;
+ else
+ Diag(NameInfo.getLoc(), diag::err_template_kw_refers_to_type_template)
+ << "" << NameInfo.getName().getAsString() << Loc.getSourceRange()
+ << IsTypeAliasTemplateDecl;
+ Diag(Temp->getLocation(), diag::note_referenced_type_template)
+ << IsTypeAliasTemplateDecl;
+ return CreateRecoveryExpr(NameInfo.getBeginLoc(), NameInfo.getEndLoc(), {});
+ }
// Overloaded expressions.
case BuiltinType::Overload: {
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 95171359f0ab17..14768ca66ac1c5 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -5553,7 +5553,7 @@ ExprResult Sema::BuildTemplateIdExpr(const CXXScopeSpec &SS,
R.getRepresentativeDecl(), TemplateKWLoc, TemplateArgs);
if (Res.isInvalid() || Res.isUsable())
return Res;
- // Result is dependent. Carry on to build an UnresolvedLookupEpxr.
+ // Result is dependent. Carry on to build an UnresolvedLookupExpr.
KnownDependent = true;
}
@@ -5571,6 +5571,12 @@ ExprResult Sema::BuildTemplateIdExpr(const CXXScopeSpec &SS,
TemplateKWLoc, R.getLookupNameInfo(), RequiresADL, TemplateArgs,
R.begin(), R.end(), KnownDependent);
+ // Model the templates with UnresolvedTemplateTy. The expression should then
+ // either be transformed in an instantiation or diagnosed.
+ if (ULE->getType() == Context.OverloadTy && R.isSingleResult() &&
+ !R.getFoundDecl()->getAsFunction())
+ ULE->setType(Context.UnresolvedTemplateTy);
+
return ULE;
}
@@ -5609,8 +5615,9 @@ Sema::BuildQualifiedTemplateIdExpr(CXXScopeSpec &SS,
Diag(NameInfo.getLoc(), diag::err_template_kw_refers_to_type_template)
<< SS.getScopeRep() << NameInfo.getName().getAsString() << SS.getRange()
<< isTypeAliasTemplateDecl;
- Diag(Temp->getLocation(), diag::note_referenced_type_template) << 0;
- return ExprError();
+ Diag(Temp->getLocation(), diag::note_referenced_type_template)
+ << isTypeAliasTemplateDecl;
+ return CreateRecoveryExpr(NameInfo.getBeginLoc(), NameInfo.getEndLoc(), {});
};
if (ClassTemplateDecl *Temp = R.getAsSingle<ClassTemplateDecl>())
diff --git a/clang/lib/Serialization/ASTCommon.cpp b/clang/lib/Serialization/ASTCommon.cpp
index f8d54c0c398906..cd2d40a2c65fcb 100644
--- a/clang/lib/Serialization/ASTCommon.cpp
+++ b/clang/lib/Serialization/ASTCommon.cpp
@@ -186,6 +186,9 @@ serialization::TypeIdxFromBuiltin(const BuiltinType *BT) {
case BuiltinType::Overload:
ID = PREDEF_TYPE_OVERLOAD_ID;
break;
+ case BuiltinType::UnresolvedTemplate:
+ ID = PREDEF_TYPE_UNRESOLVED_TEMPLATE;
+ break;
case BuiltinType::BoundMember:
ID = PREDEF_TYPE_BOUND_MEMBER;
break;
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index b28df03b4a95e9..445c042330194b 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -7311,6 +7311,9 @@ QualType ASTReader::GetType(TypeID ID) {
case PREDEF_TYPE_OVERLOAD_ID:
T = Context.OverloadTy;
break;
+ case PREDEF_TYPE_UNRESOLVED_TEMPLATE:
+ T = Context.UnresolvedTemplateTy;
+ break;
case PREDEF_TYPE_BOUND_MEMBER:
T = Context.BoundMemberTy;
break;
diff --git a/clang/test/SemaCXX/PR62533.cpp b/clang/test/SemaCXX/PR62533.cpp
index 920ea54d4b00ed..0753156813f8e7 100644
--- a/clang/test/SemaCXX/PR62533.cpp
+++ b/clang/test/SemaCXX/PR62533.cpp
@@ -2,7 +2,7 @@
template<typename T>
struct test {
- template<typename> using fun_diff = char; // expected-note 2{{class template declared here}}
+ template<typename> using fun_diff = char; // expected-note 2{{type alias template declared here}}
};
template<typename T, typename V>
diff --git a/clang/test/SemaTemplate/template-id-expr.cpp b/clang/test/SemaTemplate/template-id-expr.cpp
index 0555d8b94504fb..741742d9e5d7eb 100644
--- a/clang/test/SemaTemplate/template-id-expr.cpp
+++ b/clang/test/SemaTemplate/template-id-expr.cpp
@@ -186,3 +186,74 @@ class E {
#endif
template<typename T> using D = int; // expected-note {{declared here}}
E<D> ed; // expected-note {{instantiation of}}
+
+namespace non_functions {
+
+#if __cplusplus >= 201103L
+namespace PR88832 {
+template <typename T> struct O {
+ static const T v = 0;
+};
+
+struct P {
+ template <typename T> using I = typename O<T>::v; // #TypeAlias
+};
+
+struct Q {
+ template <typename T> int foo() {
+ return T::template I<int>; // expected-error {{'P::I' is expected to be a non-type template, but instantiated to a type alias template}}
+ // expected-note@#TypeAlias {{type alias template declared here}}
+ }
+};
+
+int bar() {
+ return Q().foo<P>(); // expected-note-re {{function template specialization {{.*}} requested here}}
+}
+
+} // namespace PR88832
+#endif
+
+namespace PR63243 {
+
+namespace std {
+template <class T> struct add_pointer { // #add_pointer
+};
+} // namespace std
+
+class A {};
+
+int main() {
+ std::__add_pointer<A>::type ptr; // #ill-formed-decl
+ // expected-error@#ill-formed-decl {{no template named '__add_pointer'}}
+ // expected-note@#add_pointer {{'add_pointer' declared here}}
+ // expected-error@#ill-formed-decl {{expected ';' after expression}}
+ // expected-error@#ill-formed-decl {{no type named 'type' in the global namespace}}
+ // expected-error@#ill-formed-decl {{'std::add_pointer' is expected to be a non-type template, but instantiated to a class template}}
+ // expected-note@#add_pointer {{class template declared here}}
+
+ // expected-warning@#ill-formed-decl {{keyword '__add_pointer' will be made available as an identifier here}}
+}
+
+} // namespace PR63243
+
+namespace PR48673 {
+
+template <typename T> struct C {
+ template <int TT> class Type {}; // #ClassTemplate
+};
+
+template <typename T1> struct A {
+ void foo() {
+ C<T1>::template Type<2>; // #templated-decl-as-expression
+ // expected-error@#templated-decl-as-expression {{'C<float>::Type' is expected to be a non-type template, but instantiated to a class template}}}
+ // expected-note@#ClassTemplate {{class template declared here}}
+ }
+};
+
+void test() {
+ A<float>().foo(); // expected-note-re {{instantiation of member function {{.*}} requested here}}
+}
+
+} // namespace PR48673
+
+}
|
Sirraide
reviewed
Apr 17, 2024
There was a problem hiding this comment.
I think the general approach of this makes sense.
Can you add some more tests that involve an expression with this new type as the argument to a function call? You’ve taken care of handling that case from what I’ve seen, but having tests for that would probably be a good idea.
... so that these "keywords" can be parsed as ordinary identifiers in a specifier.
|
@cor3ntin @erichkeane Any chance we can move forward with this review? I appreciate your feedback! |
erichkeane
reviewed
May 3, 2024
Yeah, I added some explanation to the document of |
erichkeane
approved these changes
May 3, 2024
|
Hello, |
|
Hi @zyn0217. Also: Thanks. |
|
Hi @zyn0217, I am not sure. But if I revert your patch, two tests compiles. |
I don't see any justification that these examples should compile: the first example is a typical one handled by #90152 - Clang now looks up members of the current instantiation in the template definition context
if the current instantiation has no dependent base classes.
template<typename T>
struct A {
int f() {
return this->x; // error: no member named 'x' in 'A<T>'
}
};
and the second one would also be rejected by GCC once the inner class |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch revolves around the misuse of UnresolvedLookupExpr in
BuildTemplateIdExpr.
Basically, we build up an UnresolvedLookupExpr not only for function
overloads but for "unresolved" templates wherever we need an expression
for template decls. For example, a dependent VarTemplateDecl can be
wrapped with such an expression before template instantiation. (See
6170072)
Also, one important thing is that UnresolvedLookupExpr uses a "canonical"
QualType to describe the containing unresolved decls: a DependentTy is
for dependent expressions and an OverloadTy otherwise. Therefore, this
modeling for non-dependent templates leaves a problem in that the expression
is marked and perceived as if describing overload functions. The consumer then
expects functions for every such expression, although the fact is the reverse.
Hence, we run into crashes.
As to the patch, I added a new canonical type "UnresolvedTemplateTy" to
model these cases. Given that we have been using this model (intentionally or
accidentally) and it is pretty baked in throughout the code, I think
extending the role of UnresolvedLookupExpr is reasonable. Further, I added
some diagnostics for the direct occurrence of these expressions, which
are supposed to be ill-formed.
As a bonus, this patch also fixes some typos in the diagnostics and creates
RecoveryExprs rather than nothing in the hope of a better error-recovery
for clangd.
Fixes #88832
Fixes #63243
Fixes #48673