[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent base classes

[sdkrystian]

Consider the following:

template<typename T>
struct A
{
    auto f()
    {
        return this->x;
    }
};

Although A has no dependent base classes and the lookup context for x is the current instantiation, we currently do not diagnose the absence of a member x until A<T>::f is instantiated. This patch moves the point of diagnosis for such expressions to occur at the point of definition (i.e. prior to instantiation).

Note that this PR is a bit of a work in progress. Although it passes all tests, some aspects (e.g. calling LookupResult::setNotFoundInCurrentInstantiation in LookupMemberExprInRecord, using Type::getAsRecordDecl to determine whether a type names the current instantiation, and using computeDeclContext to determine whether a nested-name-specifier names the current instantiation) are a bit "sketchy" and may need some work. I also need to add more tests, e.g. for using declarations, and class member access expressions naming members of nested classes which are explicitly specialized for a given implicitly instantiated specialization of the enclosing class template.

[llvmbot]

@llvm/pr-subscribers-hlsl

@llvm/pr-subscribers-clang

Author: Krystian Stasiowski (sdkrystian)

Changes

Consider the following:

template<typename T>
struct A
{
    auto f()
    {
        return this->x;
    }
};

Although A has no dependent base classes and the lookup context for x is the current instantiation, we currently do not diagnose the absence of a member x until A<T>::f is instantiated. This patch moves the point of diagnosis for such expressions to occur at the point of definition.

Note that this PR is a bit of a work in progress. Although it passes all tests, some aspects (e.g. calling LookupResult::setNotFoundInCurrentInstantiation in LookupMemberExprInRecord, using Type::getAsRecordDecl to determine whether a type names the current instantiation, and using computeDeclContext to determine whether a nested-name-specifier names the current instantiation) are a bit "sketchy" and may need some work. I also need to add more tests, e.g. for using declarations, or class member access expressions naming members of nested classes which are explicitly specialized for a given implicitly instantiated specialization of the enclosing class template.

---

Patch is 23.23 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/84050.diff

11 Files Affected:

• (modified) clang/lib/AST/Expr.cpp (+1-1)
• (modified) clang/lib/Sema/SemaExpr.cpp (+3-1)
• (modified) clang/lib/Sema/SemaExprMember.cpp (+80-52)
• (modified) clang/lib/Sema/SemaOverload.cpp (+3)
• (modified) clang/lib/Sema/TreeTransform.h (+20)
• (modified) clang/test/AST/HLSL/this-reference-template.hlsl (+1-1)
• (added) clang/test/CXX/temp/temp.res/temp.dep/temp.dep.type/p4.cpp (+110)
• (modified) clang/test/CodeGenCXX/mangle.cpp (-8)
• (modified) clang/test/Index/annotate-nested-name-specifier.cpp (+2-2)
• (modified) clang/test/SemaTemplate/instantiate-function-1.cpp (+7-7)
• (modified) clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp (+2-2)

diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index b4de2155adcebd..643c938d63c654 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -103,7 +103,7 @@ const Expr *Expr::skipRValueSubobjectAdjustments(
       }
     } else if (const auto *ME = dyn_cast<MemberExpr>(E)) {
       if (!ME->isArrow()) {
-        assert(ME->getBase()->getType()->isRecordType());
+        assert(ME->getBase()->getType()->getAsRecordDecl());
         if (const auto *Field = dyn_cast<FieldDecl>(ME->getMemberDecl())) {
           if (!Field->isBitField() && !Field->getType()->isReferenceType()) {
             E = ME->getBase();
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 47bb263f56aade..e071c3e580b9d8 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -666,7 +666,9 @@ ExprResult Sema::DefaultLvalueConversion(Expr *E) {
   // expressions of certain types in C++.
   if (getLangOpts().CPlusPlus &&
       (E->getType() == Context.OverloadTy ||
-       T->isDependentType() ||
+      // FIXME: This is a hack! We want the lvalue-to-rvalue conversion applied
+      // to pointer types even if the pointee type is dependent.
+      (T->isDependentType() && !T->isPointerType()) ||
        T->isRecordType()))
     return E;
 
diff --git a/clang/lib/Sema/SemaExprMember.cpp b/clang/lib/Sema/SemaExprMember.cpp
index 32998ae60eafe2..eb87e0edda0128 100644
--- a/clang/lib/Sema/SemaExprMember.cpp
+++ b/clang/lib/Sema/SemaExprMember.cpp
@@ -624,8 +624,8 @@ namespace {
 // classes, one of its base classes.
 class RecordMemberExprValidatorCCC final : public CorrectionCandidateCallback {
 public:
-  explicit RecordMemberExprValidatorCCC(const RecordType *RTy)
-      : Record(RTy->getDecl()) {
+  explicit RecordMemberExprValidatorCCC(QualType RTy)
+      : Record(RTy->getAsRecordDecl()) {
     // Don't add bare keywords to the consumer since they will always fail
     // validation by virtue of not being associated with any decls.
     WantTypeSpecifiers = false;
@@ -671,33 +671,54 @@ class RecordMemberExprValidatorCCC final : public CorrectionCandidateCallback {
 
 static bool LookupMemberExprInRecord(Sema &SemaRef, LookupResult &R,
                                      Expr *BaseExpr,
-                                     const RecordType *RTy,
+                                     QualType RTy,
                                      SourceLocation OpLoc, bool IsArrow,
                                      CXXScopeSpec &SS, bool HasTemplateArgs,
                                      SourceLocation TemplateKWLoc,
                                      TypoExpr *&TE) {
+  DeclContext *DC = SemaRef.computeDeclContext(RTy);
+  // If the object expression is dependent and isn't the current instantiation,
+  // lookup will not find anything and we must defer until instantiation.
+  if (!DC) {
+    R.setNotFoundInCurrentInstantiation();
+    return false;
+  }
+
+  // FIXME: Should this use Name.isDependentName()?
+  if (DeclarationName Name = R.getLookupName();
+      Name.getNameKind() == DeclarationName::CXXConversionFunctionName &&
+      Name.getCXXNameType()->isDependentType()) {
+    R.setNotFoundInCurrentInstantiation();
+    return false;
+  }
+
   SourceRange BaseRange = BaseExpr ? BaseExpr->getSourceRange() : SourceRange();
-  RecordDecl *RDecl = RTy->getDecl();
-  if (!SemaRef.isThisOutsideMemberFunctionBody(QualType(RTy, 0)) &&
-      SemaRef.RequireCompleteType(OpLoc, QualType(RTy, 0),
+  if (!RTy->isDependentType() &&
+      !SemaRef.isThisOutsideMemberFunctionBody(RTy) &&
+      SemaRef.RequireCompleteType(OpLoc, RTy,
                                   diag::err_typecheck_incomplete_tag,
                                   BaseRange))
     return true;
 
   if (HasTemplateArgs || TemplateKWLoc.isValid()) {
     // LookupTemplateName doesn't expect these both to exist simultaneously.
-    QualType ObjectType = SS.isSet() ? QualType() : QualType(RTy, 0);
+    QualType ObjectType = SS.isSet() ? QualType() : RTy;
 
     bool MOUS;
     return SemaRef.LookupTemplateName(R, nullptr, SS, ObjectType, false, MOUS,
                                       TemplateKWLoc);
   }
 
-  DeclContext *DC = RDecl;
   if (SS.isSet()) {
     // If the member name was a qualified-id, look into the
     // nested-name-specifier.
     DC = SemaRef.computeDeclContext(SS, false);
+    // We tried to look into a dependent context that is not the current
+    // instantiation. Defer lookup until instantiation.
+    if (!DC) {
+      R.setNotFoundInCurrentInstantiation();
+      return false;
+    }
 
     if (SemaRef.RequireCompleteDeclContext(SS, DC)) {
       SemaRef.Diag(SS.getRange().getEnd(), diag::err_typecheck_incomplete_tag)
@@ -717,7 +738,7 @@ static bool LookupMemberExprInRecord(Sema &SemaRef, LookupResult &R,
   // The record definition is complete, now look up the member.
   SemaRef.LookupQualifiedName(R, DC, SS);
 
-  if (!R.empty())
+  if (!R.empty() || R.wasNotFoundInCurrentInstantiation())
     return false;
 
   DeclarationName Typo = R.getLookupName();
@@ -781,14 +802,6 @@ Sema::BuildMemberReferenceExpr(Expr *Base, QualType BaseType,
                                const TemplateArgumentListInfo *TemplateArgs,
                                const Scope *S,
                                ActOnMemberAccessExtraArgs *ExtraArgs) {
-  if (BaseType->isDependentType() ||
-      (SS.isSet() && isDependentScopeSpecifier(SS)) ||
-      NameInfo.getName().isDependentName())
-    return ActOnDependentMemberExpr(Base, BaseType,
-                                    IsArrow, OpLoc,
-                                    SS, TemplateKWLoc, FirstQualifierInScope,
-                                    NameInfo, TemplateArgs);
-
   LookupResult R(*this, NameInfo, LookupMemberName);
 
   // Implicit member accesses.
@@ -797,7 +810,7 @@ Sema::BuildMemberReferenceExpr(Expr *Base, QualType BaseType,
     QualType RecordTy = BaseType;
     if (IsArrow) RecordTy = RecordTy->castAs<PointerType>()->getPointeeType();
     if (LookupMemberExprInRecord(
-            *this, R, nullptr, RecordTy->castAs<RecordType>(), OpLoc, IsArrow,
+            *this, R, nullptr, RecordTy, OpLoc, IsArrow,
             SS, TemplateArgs != nullptr, TemplateKWLoc, TE))
       return ExprError();
     if (TE)
@@ -990,6 +1003,15 @@ Sema::BuildMemberReferenceExpr(Expr *BaseExpr, QualType BaseExprType,
                                const Scope *S,
                                bool SuppressQualifierCheck,
                                ActOnMemberAccessExtraArgs *ExtraArgs) {
+
+  if (R.wasNotFoundInCurrentInstantiation() ||
+      (SS.isValid() && !computeDeclContext(SS, false))) {
+    return ActOnDependentMemberExpr(BaseExpr, BaseExprType,
+                                    IsArrow, OpLoc,
+                                    SS, TemplateKWLoc, FirstQualifierInScope,
+                                    R.getLookupNameInfo(), TemplateArgs);
+  }
+
   QualType BaseType = BaseExprType;
   if (IsArrow) {
     assert(BaseType->isPointerType());
@@ -1028,11 +1050,11 @@ Sema::BuildMemberReferenceExpr(Expr *BaseExpr, QualType BaseExprType,
     // Rederive where we looked up.
     DeclContext *DC = (SS.isSet()
                        ? computeDeclContext(SS, false)
-                       : BaseType->castAs<RecordType>()->getDecl());
+                       : BaseType->getAsRecordDecl());
 
     if (ExtraArgs) {
       ExprResult RetryExpr;
-      if (!IsArrow && BaseExpr) {
+      if (!IsArrow && BaseExpr && !BaseExpr->isTypeDependent()) {
         SFINAETrap Trap(*this, true);
         ParsedType ObjectType;
         bool MayBePseudoDestructor = false;
@@ -1055,9 +1077,16 @@ Sema::BuildMemberReferenceExpr(Expr *BaseExpr, QualType BaseExprType,
       }
     }
 
-    Diag(R.getNameLoc(), diag::err_no_member)
-      << MemberName << DC
-      << (BaseExpr ? BaseExpr->getSourceRange() : SourceRange());
+    if(DC) {
+      Diag(R.getNameLoc(), diag::err_no_member)
+          << MemberName << DC
+          << (BaseExpr ? BaseExpr->getSourceRange() : SourceRange());
+    } else {
+      // FIXME: Is this needed?
+      Diag(R.getNameLoc(), diag::err_no_member)
+          << MemberName << BaseExprType
+          << (BaseExpr ? BaseExpr->getSourceRange() : SourceRange());
+    }
     return ExprError();
   }
 
@@ -1287,7 +1316,10 @@ static ExprResult LookupMemberExpr(Sema &S, LookupResult &R,
     return ExprError();
 
   QualType BaseType = BaseExpr.get()->getType();
+
+  #if 0
   assert(!BaseType->isDependentType());
+  #endif
 
   DeclarationName MemberName = R.getLookupName();
   SourceLocation MemberLoc = R.getNameLoc();
@@ -1299,29 +1331,32 @@ static ExprResult LookupMemberExpr(Sema &S, LookupResult &R,
   if (IsArrow) {
     if (const PointerType *Ptr = BaseType->getAs<PointerType>())
       BaseType = Ptr->getPointeeType();
-    else if (const ObjCObjectPointerType *Ptr
+    else if (!BaseType->isDependentType()) {
+      if (const ObjCObjectPointerType *Ptr
                = BaseType->getAs<ObjCObjectPointerType>())
       BaseType = Ptr->getPointeeType();
-    else if (BaseType->isRecordType()) {
-      // Recover from arrow accesses to records, e.g.:
-      //   struct MyRecord foo;
-      //   foo->bar
-      // This is actually well-formed in C++ if MyRecord has an
-      // overloaded operator->, but that should have been dealt with
-      // by now--or a diagnostic message already issued if a problem
-      // was encountered while looking for the overloaded operator->.
-      if (!S.getLangOpts().CPlusPlus) {
-        S.Diag(OpLoc, diag::err_typecheck_member_reference_suggestion)
-          << BaseType << int(IsArrow) << BaseExpr.get()->getSourceRange()
-          << FixItHint::CreateReplacement(OpLoc, ".");
+      else if (BaseType->isRecordType()) {
+        // Recover from arrow accesses to records, e.g.:
+         //   struct MyRecord foo;
+        //   foo->bar
+        // This is actually well-formed in C++ if MyRecord has an
+        // overloaded operator->, but that should have been dealt with
+        // by now--or a diagnostic message already issued if a problem
+        // was encountered while looking for the overloaded operator->.
+        if (!S.getLangOpts().CPlusPlus) {
+          S.Diag(OpLoc, diag::err_typecheck_member_reference_suggestion)
+              << BaseType << int(IsArrow) << BaseExpr.get()->getSourceRange()
+              << FixItHint::CreateReplacement(OpLoc, ".");
+        }
+        IsArrow = false;
+      } else if (BaseType->isFunctionType()) {
+        goto fail;
+      } else {
+        S.Diag(MemberLoc, diag::err_typecheck_member_reference_arrow)
+            << BaseType << BaseExpr.get()->getSourceRange();
+        return ExprError();
       }
-      IsArrow = false;
-    } else if (BaseType->isFunctionType()) {
-      goto fail;
-    } else {
-      S.Diag(MemberLoc, diag::err_typecheck_member_reference_arrow)
-        << BaseType << BaseExpr.get()->getSourceRange();
-      return ExprError();
+
     }
   }
 
@@ -1341,9 +1376,9 @@ static ExprResult LookupMemberExpr(Sema &S, LookupResult &R,
   }
 
   // Handle field access to simple records.
-  if (const RecordType *RTy = BaseType->getAs<RecordType>()) {
+  if (BaseType->getAsRecordDecl() || BaseType->isDependentType()) {
     TypoExpr *TE = nullptr;
-    if (LookupMemberExprInRecord(S, R, BaseExpr.get(), RTy, OpLoc, IsArrow, SS,
+    if (LookupMemberExprInRecord(S, R, BaseExpr.get(), BaseType, OpLoc, IsArrow, SS,
                                  HasTemplateArgs, TemplateKWLoc, TE))
       return ExprError();
 
@@ -1781,13 +1816,6 @@ ExprResult Sema::ActOnMemberAccessExpr(Scope *S, Expr *Base,
   if (Result.isInvalid()) return ExprError();
   Base = Result.get();
 
-  if (Base->getType()->isDependentType() || Name.isDependentName() ||
-      isDependentScopeSpecifier(SS)) {
-    return ActOnDependentMemberExpr(Base, Base->getType(), IsArrow, OpLoc, SS,
-                                    TemplateKWLoc, FirstQualifierInScope,
-                                    NameInfo, TemplateArgs);
-  }
-
   ActOnMemberAccessExtraArgs ExtraArgs = {S, Id, ObjCImpDecl};
   ExprResult Res = BuildMemberReferenceExpr(
       Base, Base->getType(), OpLoc, IsArrow, SS, TemplateKWLoc,
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 7d38043890ca20..7218553bcd77f8 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -5744,7 +5744,10 @@ static ImplicitConversionSequence TryObjectArgumentInitialization(
     return ICS;
   }
 
+  // FIXME: Should this check getAsRecordDecl instead?
+  #if 0
   assert(FromType->isRecordType());
+  #endif
 
   QualType ClassType = S.Context.getTypeDeclType(ActingContext);
   // C++98 [class.dtor]p2:
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 7389a48fe56fcc..27d3588aaa9082 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -12930,6 +12930,26 @@ bool TreeTransform<Derived>::TransformOverloadExprDecls(OverloadExpr *Old,
   // Resolve a kind, but don't do any further analysis.  If it's
   // ambiguous, the callee needs to deal with it.
   R.resolveKind();
+
+  if (Old->hasTemplateKeyword() && !R.empty()) {
+    NamedDecl *FoundDecl = R.getRepresentativeDecl()->getUnderlyingDecl();
+    getSema().FilterAcceptableTemplateNames(R,
+                                            /*AllowFunctionTemplates=*/true,
+                                            /*AllowDependent=*/true);
+    if (R.empty()) {
+      // If a 'template' keyword was used, a lookup that finds only non-template
+      // names is an error.
+      getSema().Diag(R.getNameLoc(), diag::err_template_kw_refers_to_non_template)
+          << R.getLookupName()
+          << Old->getQualifierLoc().getSourceRange()
+          << Old->hasTemplateKeyword()
+          << Old->getTemplateKeywordLoc();
+      getSema().Diag(FoundDecl->getLocation(), diag::note_template_kw_refers_to_non_template)
+          << R.getLookupName();
+        return true;
+    }
+  }
+
   return false;
 }
 
diff --git a/clang/test/AST/HLSL/this-reference-template.hlsl b/clang/test/AST/HLSL/this-reference-template.hlsl
index 60e057986ebf80..d427e73044b788 100644
--- a/clang/test/AST/HLSL/this-reference-template.hlsl
+++ b/clang/test/AST/HLSL/this-reference-template.hlsl
@@ -24,7 +24,7 @@ void main() {
 // CHECK:     -CXXMethodDecl 0x{{[0-9A-Fa-f]+}} <line:8:3, line:10:3> line:8:5 getFirst 'K ()' implicit-inline
 // CHECK-NEXT:-CompoundStmt 0x{{[0-9A-Fa-f]+}} <col:16, line:10:3>
 // CHECK-NEXT:-ReturnStmt 0x{{[0-9A-Fa-f]+}} <line:9:4, col:16>
-// CHECK-NEXT:-CXXDependentScopeMemberExpr 0x{{[0-9A-Fa-f]+}} <col:11, col:16> '<dependent type>' lvalue .First
+// CHECK-NEXT:-MemberExpr 0x{{[0-9A-Fa-f]+}} <col:11, col:16> 'K' lvalue .First 0x{{[0-9A-Fa-f]+}}
 // CHECK-NEXT:-CXXThisExpr 0x{{[0-9A-Fa-f]+}} <col:11> 'Pair<K, V>' lvalue this
 // CHECK-NEXT:-CXXMethodDecl 0x{{[0-9A-Fa-f]+}} <line:12:3, line:14:3> line:12:5 getSecond 'V ()' implicit-inline
 // CHECK-NEXT:-CompoundStmt 0x{{[0-9A-Fa-f]+}} <col:17, line:14:3>
diff --git a/clang/test/CXX/temp/temp.res/temp.dep/temp.dep.type/p4.cpp b/clang/test/CXX/temp/temp.res/temp.dep/temp.dep.type/p4.cpp
new file mode 100644
index 00000000000000..7b36e3ad3b0131
--- /dev/null
+++ b/clang/test/CXX/temp/temp.res/temp.dep/temp.dep.type/p4.cpp
@@ -0,0 +1,110 @@
+// RUN: %clang_cc1 -Wno-unused-value -verify %s
+
+namespace N0 {
+  template<typename T>
+  struct A {
+    int x;
+    void f();
+    using X = int;
+
+    void not_instantiated(A *a, A &b) {
+      x;
+      f();
+      new X;
+
+      this->x;
+      this->f();
+      this->A::x;
+      this->A::f();
+
+      a->x;
+      a->f();
+      a->A::x;
+      a->A::f();
+
+      (*this).x;
+      (*this).f();
+      (*this).A::x;
+      (*this).A::f();
+
+      b.x;
+      b.f();
+      b.A::x;
+      b.A::f();
+
+      A::x;
+      A::f();
+      new A::X;
+
+      y; // expected-error{{use of undeclared identifier 'y'}}
+      g(); // expected-error{{use of undeclared identifier 'g'}}
+      new Y; // expected-error{{unknown type name 'Y'}}
+
+      this->y; // expected-error{{no member named 'y' in 'A<T>'}}
+      this->g(); // expected-error{{no member named 'g' in 'A<T>'}}
+      this->A::y; // expected-error{{no member named 'y' in 'A<T>'}}
+      this->A::g(); // expected-error{{no member named 'g' in 'A<T>'}}
+
+      a->y; // expected-error{{no member named 'y' in 'A<T>'}}
+      a->g(); // expected-error{{no member named 'g' in 'A<T>'}}
+      a->A::y; // expected-error{{no member named 'y' in 'A<T>'}}
+      a->A::g(); // expected-error{{no member named 'g' in 'A<T>'}}
+
+      // FIXME: An overloaded unary 'operator*' is built for these
+      // even though the operand is a pointer (to a dependent type).
+      // Type::isOverloadableType should return false for such cases.
+      (*this).y;
+      (*this).g();
+      (*this).A::y;
+      (*this).A::g();
+
+      b.y; // expected-error{{no member named 'y' in 'A<T>'}}
+      b.g(); // expected-error{{no member named 'g' in 'A<T>'}}
+      b.A::y; // expected-error{{no member named 'y' in 'A<T>'}}
+      b.A::g(); // expected-error{{no member named 'g' in 'A<T>'}}
+
+      A::y; // expected-error{{no member named 'y' in 'A<T>'}}
+      A::g(); // expected-error{{no member named 'g' in 'A<T>'}}
+      new A::Y; // expected-error{{no type named 'Y' in 'A<T>'}}
+    }
+  };
+} // namespace N0
+
+namespace N1 {
+  struct A {
+    template<int I>
+    void f();
+  };
+
+  template<typename T>
+  struct B {
+    template<int I>
+    void f();
+
+    A x;
+    A g();
+
+    void not_instantiated(B *a, B &b) {
+      f<0>();
+      this->f<0>();
+      a->f<0>();
+      // FIXME: This should not require 'template'!
+      (*this).f<0>(); // expected-error{{missing 'template' keyword prior to dependent template name 'f'}}
+      b.f<0>();
+
+      x.f<0>();
+      this->x.f<0>();
+      a->x.f<0>();
+      // FIXME: This should not require 'template'!
+      (*this).x.f<0>(); // expected-error{{missing 'template' keyword prior to dependent template name 'f'}}
+      b.x.f<0>();
+
+      // FIXME: None of these should require 'template'!
+      g().f<0>(); // expected-error{{missing 'template' keyword prior to dependent template name 'f'}}
+      this->g().f<0>(); // expected-error{{missing 'template' keyword prior to dependent template name 'f'}}
+      a->g().f<0>(); // expected-error{{missing 'template' keyword prior to dependent template name 'f'}}
+      (*this).g().f<0>(); // expected-error{{missing 'template' keyword prior to dependent template name 'f'}}
+      b.g().f<0>(); // expected-error{{missing 'template' keyword prior to dependent template name 'f'}}
+    }
+  };
+} // namespace N1
diff --git a/clang/test/CodeGenCXX/mangle.cpp b/clang/test/CodeGenCXX/mangle.cpp
index 31467d943840e0..d0800af55c87e8 100644
--- a/clang/test/CodeGenCXX/mangle.cpp
+++ b/clang/test/CodeGenCXX/mangle.cpp
@@ -1032,10 +1032,6 @@ namespace test51 {
   template <typename T>
   decltype(S1<T>().~S1<T>(), S1<T>().~S1<T>()) fun4() {};
   template <typename T>
-  decltype(S1<int>().~S1<T>()) fun5(){};
-  template <template <typename T> class U>
-  decltype(S1<int>().~U<int>()) fun6(){};
-  template <typename T>
   decltype(E().E::~T()) fun7() {}
   template <template <typename> class U>
   decltype(X<int>::Y().U<int>::Y::~Y()) fun8() {}
@@ -1047,10 +1043,6 @@ namespace test51 {
   // CHECK-LABEL: @_ZN6test514fun3I2S1IiEiEEDTcldtcvS1_IT0_E_EdnT_EEv
   template void fun4<int>();
   // CHECK-LABEL: @_ZN6test514fun4IiEEDTcmcldtcv2S1IT_E_Edn2S1IS2_EEcldtcvS3__Edn2S1IS2_EEEv
-  template void fun5<int>();
-  // CHECK-LABEL: @_ZN6test514fun5IiEEDTcldtcv2S1IiE_Edn2S1IT_EEEv
-  template void fun6<S1>();
-  // CHECK-LABEL: @_ZN6test514fun6I2S1EEDTcldtcvS1_IiE_EdnT_IiEEEv
   template void fun7<E>();
   // CHECK-LABEL: @_ZN6test514fun7INS_1EEEEDTcldtcvS1__Esr1EEdnT_EEv
   template void fun8<X>();
diff --git a/clang/test/Index/annotate-nested-name-specifier.cpp b/clang/test/Index/annotate-nested-name-specifier.cpp
index a7338db6b05b77..3181497258407f 100644
--- a/clang/test/Index/annotate-nested-name-specifier.cpp
+++ b/clang/test/Index/annotate-nested-name-specifier.cpp
@@ -132,7 +132,7 @@ struct X8 {
 
 struct X9 : X8 {
   typedef X8 inherited;
-  void f() { 
+  void f() {
     inherited::f();
   }
 };
@@ -29...
[truncated]

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[erichkeane]

I like the direction of this, and look forward to seeing how this turns out!

[sdkrystian]

@erichkeane This PR is in a more "ready" state now, if you'd like to take another look. The added overload of LookupTemplateName which has no MemberOfUnknownSpecialization parameter it more of an experimental change... it can be ignored.

[Endilll]

Sema.h changes look good to me.

[sdkrystian]

Ping @erichkeane

[erichkeane]

Quite a bit of cleanup level stuff, plus wanting 1 minor refactor here (re the lookup function), but otherwise is pretty good.

[erichkeane]

Suggested change

	/*AllowBuiltinCreation*/ false);
	/*AllowBuiltinCreation=*/ false);

[erichkeane]

Also, why is this specifying 'false' here, which is the default? I realize it was already there, but we should probably not be.

Also-also-- I think this many default arguments has hit the point for ME where we just need an overload for the version where it would be 'all the default things' and specify for everything else. This is out of hand.

[sdkrystian]

@erichkeane I replaced every call to LookupParsedName which passed nullptr/an empty CXXScopeSpec and a null QualType with a call to LookupName.

The two parameters with default arguments are forwarded to LookupQualifiedName/LookupName (the corresponding parameters for those functions also have default arguments).

[erichkeane]

Thats an interesting question... I think we SHOULD evaluate that, but feel free to do so in a follow up.

[sdkrystian]

@erichkeane PR updated.

When we encounter a class member access expression with a nested-name-specifier that is invalid, should we simply ignore it and instead use the type of the object expression as the lookup context? This is the current behavior, but it leads to lots of duplicate diagnostics. For example:

struct A
{
    using I = int;
};

void f(A* x)
{
    x->I::z; // error: 'A::I' (aka 'int') is not a class, namespace, or enumeration
               // error: no member named 'z' in 'A'
}

IMO we should just return ExprError from BuildMemberReferenceExpr and thus not emit the second error.

[erichkeane]

@erichkeane PR updated.

When we encounter a class member access expression with a nested-name-specifier that is invalid, should we simply ignore it and instead use the type of the object expression as the lookup context? This is the current behavior, but it leads to lots of duplicate diagnostics. For example:

struct A
{
    using I = int;
};

void f(A* x)
{
    x->I::z; // error: 'A::I' (aka 'int') is not a class, namespace, or enumeration
               // error: no member named 'z' in 'A'
}

IMO we should just return ExprError from BuildMemberReferenceExpr and thus not emit the second error.

Sorry, I'm not really getting the question here. Obviously that 2nd error is invalid and we shouldn't be emitting it, and doing the ::z lookup inside of A seems wrong to me. But I'm not sure I get the issue.

[sdkrystian]

Sorry, I'm not really getting the question here. Obviously that 2nd error is invalid and we shouldn't be emitting it, and doing the ::z lookup inside of A seems wrong to me. But I'm not sure I get the issue.

We currently emit the second error. I'm asking if we shouldn't emit it (and your response suggests that we shouldn't)

I added a commit which eliminates the second error.

[nikic]

FYI this change has some visible compile-time impact, with some 0.5-1% regressions on various LLVM files (https://llvm-compile-time-tracker.com/compare_clang.php?from=2f2e31c3c980407b2660b4f5d10e7cdb3fa79138&to=a8fd0d029dca7d17eee72d0445223c2fe1ee7758&stat=instructions%3Au&sortBy=interestingness). Nothing terrible (it doesn't seem to be an across-the-board regression), just wanted to let you know in case this is unexpected.

[alexey-bataev]

I think this breaks the build of SPEC2017/xalancbmk

./xalanc/XMLSupport/XalanOtherEncodingWriter.hpp:319:30: error: no member named 'm_isPresentable' in 'XalanOtherEncodingWriter<Predicate, ConstantsType>'
  319 |                     if(this->m_isPresentable(value))
      |                        ~~~~  ^
./xalanc/XMLSupport/XalanOtherEncodingWriter.hpp:325:31: error: no member named 'writeNumberedEntityReference' in 'XalanOtherEncodingWriter<Predicate, ConstantsType>'
  325 |                         this->writeNumberedEntityReference(value);
      |                         ~~~~  ^

[sdkrystian]

@alexey-bataev see #90152 (comment)

[alexey-bataev]

@alexey-bataev see #90152 (comment)

Thanks for the link