Author: Younan Zhang
Date: 2024-05-05T11:38:49+08:00
New Revision: 7a484d3a1f630ba9ce7b22e744818be974971470

URL: 
https://github.com/llvm/llvm-project/commit/7a484d3a1f630ba9ce7b22e744818be974971470
DIFF: 
https://github.com/llvm/llvm-project/commit/7a484d3a1f630ba9ce7b22e744818be974971470.diff

LOG: [clang] Distinguish unresolved templates in UnresolvedLookupExpr (#89019)

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

https://github.com/llvm/llvm-project/commit/617007240cbfb97c8ccf6d61b0c4ca0bb62d43c9)
    
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 https://github.com/llvm/llvm-project/issues/88832
Fixes https://github.com/llvm/llvm-project/issues/63243
Fixes https://github.com/llvm/llvm-project/issues/48673

Added: 
    

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/include/clang/AST/ASTContext.h
    clang/include/clang/AST/BuiltinTypes.def
    clang/include/clang/AST/ExprCXX.h
    clang/include/clang/Serialization/ASTBitCodes.h
    clang/lib/AST/ASTContext.cpp
    clang/lib/AST/NSAPI.cpp
    clang/lib/AST/Type.cpp
    clang/lib/AST/TypeLoc.cpp
    clang/lib/Parse/ParseExprCXX.cpp
    clang/lib/Sema/SemaExpr.cpp
    clang/lib/Sema/SemaTemplate.cpp
    clang/lib/Serialization/ASTCommon.cpp
    clang/lib/Serialization/ASTReader.cpp
    clang/test/SemaCXX/PR62533.cpp
    clang/test/SemaTemplate/template-id-expr.cpp

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 54b58b1ae99fbd..b146a9b56884ad 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -681,6 +681,8 @@ Bug Fixes to C++ Support
   whose type is `decltype(auto)`. Fixes (#GH68885).
 - Clang now correctly treats the noexcept-specifier of a friend function to be 
a complete-class context.
 - Fix an assertion failure when parsing an invalid members of an anonymous 
class. (#GH85447)
+- 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 6dbd06251ddad6..e03b1121947867 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 0a36fdc5d9c0f7..444be4311a7431 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/AST/ExprCXX.h 
b/clang/include/clang/AST/ExprCXX.h
index ab3f810b45192b..fac65628ffede8 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -3163,8 +3163,30 @@ class OverloadExpr : public Expr {
 /// This arises in several ways:
 ///   * we might be waiting for argument-dependent lookup;
 ///   * the name might resolve to an overloaded function;
+///   * the name might resolve to a non-function template; for example, in the
+///   following snippet, the return expression of the member function
+///   'foo()' might remain unresolved until instantiation:
+///
+/// \code
+/// struct P {
+///   template <class T> using I = T;
+/// };
+///
+/// struct Q {
+///   template <class T> int foo() {
+///     return T::template I<int>;
+///   }
+/// };
+/// \endcode
+///
+/// ...which is distinct from modeling function overloads, and therefore we use
+/// a 
diff erent builtin type 'UnresolvedTemplate' to avoid confusion. This is
+/// done in Sema::BuildTemplateIdExpr.
+///
 /// and eventually:
 ///   * the lookup might have included a function template.
+///   * the unresolved template gets transformed in an instantiation or gets
+///   diagnosed for its direct use.
 ///
 /// These never include UnresolvedUsingValueDecls, which are always class
 /// members and therefore appear only in UnresolvedMemberLookupExprs.

diff  --git a/clang/include/clang/Serialization/ASTBitCodes.h 
b/clang/include/clang/Serialization/ASTBitCodes.h
index a8df5a0bda0850..b7aaceacc00627 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -1091,6 +1091,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
 };
@@ -1100,7 +1103,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 699721aabdb5e1..5f96e86f803a80 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -1307,6 +1307,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 6f586173edb021..2d16237f5325ad 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 68e81f45b4c28e..2385c5e02cb269 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -3393,6 +3393,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:
@@ -4685,6 +4687,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 ce45b47d5cfea5..9dd90d9bf4e543 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/Parse/ParseExprCXX.cpp 
b/clang/lib/Parse/ParseExprCXX.cpp
index 0d2ad980696fcc..825031da358ad5 100644
--- a/clang/lib/Parse/ParseExprCXX.cpp
+++ b/clang/lib/Parse/ParseExprCXX.cpp
@@ -407,6 +407,20 @@ bool Parser::ParseOptionalCXXScopeSpecifier(
       continue;
     }
 
+    switch (Tok.getKind()) {
+#define TRANSFORM_TYPE_TRAIT_DEF(_, Trait) case tok::kw___##Trait:
+#include "clang/Basic/TransformTypeTraits.def"
+      if (!NextToken().is(tok::l_paren)) {
+        Tok.setKind(tok::identifier);
+        Diag(Tok, diag::ext_keyword_as_ident)
+            << Tok.getIdentifierInfo()->getName() << 0;
+        continue;
+      }
+      [[fallthrough]];
+    default:
+      break;
+    }
+
     // The rest of the nested-name-specifier possibilities start with
     // tok::identifier.
     if (Tok.isNot(tok::identifier))

diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 2557b1af8f024e..b1322f30fa6b6a 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -6345,6 +6345,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:
@@ -21253,6 +21254,27 @@ 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();
+    // There's only one FoundDecl for UnresolvedTemplate type. See
+    // BuildTemplateIdExpr.
+    NamedDecl *Temp = *ULE->decls_begin();
+    const bool IsTypeAliasTemplateDecl = isa<TypeAliasTemplateDecl>(Temp);
+
+    if (NestedNameSpecifierLoc Loc = ULE->getQualifierLoc(); 
Loc.hasQualifier())
+      Diag(NameInfo.getLoc(), diag::err_template_kw_refers_to_type_template)
+          << Loc.getNestedNameSpecifier() << NameInfo.getName().getAsString()
+          << Loc.getSourceRange() << IsTypeAliasTemplateDecl;
+    else
+      Diag(NameInfo.getLoc(), diag::err_template_kw_refers_to_type_template)
+          << "" << NameInfo.getName().getAsString() << ULE->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 989f3995ca5991..e647ac267ab395 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -5554,7 +5554,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;
   }
 
@@ -5572,6 +5572,13 @@ 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 be diagnosed in
+  // CheckPlaceholderExpr.
+  if (ULE->getType() == Context.OverloadTy && R.isSingleResult() &&
+      !R.getFoundDecl()->getAsFunction())
+    ULE->setType(Context.UnresolvedTemplateTy);
+
   return ULE;
 }
 
@@ -5608,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 e017f5bdb48858..63c5140086d8ec 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 6d121f4ab80e84..36ec6d6b2eb7ea 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -7320,6 +7320,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..ce40aade9cf174 100644
--- a/clang/test/SemaTemplate/template-id-expr.cpp
+++ b/clang/test/SemaTemplate/template-id-expr.cpp
@@ -186,3 +186,93 @@ 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@-1 {{'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;
+  // expected-warning@-1 {{keyword '__add_pointer' will be made available as 
an identifier here}}
+  // expected-error@-2 {{no template named '__add_pointer'}}
+  // expected-note@#add_pointer {{'add_pointer' declared here}}
+  // expected-error-re@-4 {{no type named 'type' in 
'{{.*}}std::add_pointer<{{.*}}A>'}}
+
+  __add_pointer<A>::type ptr2;
+  // expected-error@-1 {{no template named '__add_pointer'}}
+  // expected-error-re@-2 {{no type named 'type' in 
'{{.*}}std::add_pointer<{{.*}}A>'}}
+  // expected-note@#add_pointer {{'std::add_pointer' declared here}}
+}
+
+} // namespace PR63243
+
+namespace PR48673 {
+
+template <typename T> struct C {
+  template <int TT> class Type {}; // #ClassTemplate
+};
+
+template <typename T1> struct A {
+
+  template <typename T2>
+  void foo(T2) {}
+
+  void foo() {
+    C<T1>::template Type<2>;
+    // expected-error@-1 {{'C<float>::Type' is expected to be a non-type 
template, but instantiated to a class template}}}
+    // expected-note@#ClassTemplate {{class template declared here}}
+
+    foo(C<T1>::Type<2>); // expected-error {{expected expression}}
+
+    foo(C<T1>::template Type<2>);
+    // expected-error@-1 {{'C<float>::Type' is expected to be a non-type 
template, but instantiated to a class template}}
+    // expected-note@#ClassTemplate {{class template declared here}}
+
+    foo(C<T1>::template Type<2>());
+    // expected-error@-1 {{'C<float>::Type' is expected to be a non-type 
template, but instantiated to a class template}}
+    // expected-error@-2 {{called object type '<dependent type>' is not a 
function or function pointer}}
+    // expected-note@#ClassTemplate {{class template declared here}}
+
+    foo(typename C<T1>::template Type<2>());
+  }
+};
+
+void test() {
+  A<float>().foo(); // expected-note-re {{instantiation of member function 
{{.*}} requested here}}
+}
+
+} // namespace PR48673
+
+}


        
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to