[clang][CodeComplete] Use HeuristicResolver in getAsRecordDecl()

[HighCommander4]

Fixes #130468

[llvmbot]

@llvm/pr-subscribers-clang

Author: Nathan Ridge (HighCommander4)

Changes

Fixes #130468

---

Full diff: https://github.com/llvm/llvm-project/pull/130473.diff

6 Files Affected:

• (modified) clang/include/clang/Sema/HeuristicResolver.h (+12)
• (modified) clang/include/clang/Sema/Sema.h (+3-1)
• (modified) clang/lib/Parse/Parser.cpp (+3-2)
• (modified) clang/lib/Sema/HeuristicResolver.cpp (+25-24)
• (modified) clang/lib/Sema/SemaCodeComplete.cpp (+15-28)
• (modified) clang/test/CodeCompletion/member-access.cpp (+4-2)

diff --git a/clang/include/clang/Sema/HeuristicResolver.h b/clang/include/clang/Sema/HeuristicResolver.h
index f511815b40199..df60d3359c6a6 100644
--- a/clang/include/clang/Sema/HeuristicResolver.h
+++ b/clang/include/clang/Sema/HeuristicResolver.h
@@ -81,6 +81,18 @@ class HeuristicResolver {
   // could look up the name appearing on the RHS.
   const QualType getPointeeType(QualType T) const;
 
+  // Heuristically resolve a possibly-dependent type `T` to a TagDecl
+  // in which a member's name can be looked up.
+  TagDecl *resolveTypeToTagDecl(QualType T) const;
+
+  // Simplify the type `Type`.
+  // `E` is the expression whose type `Type` is, if known. This sometimes
+  // contains information relevant to the type that's not stored in `Type`
+  // itself.
+  // If `UnwrapPointer` is true, exactly only pointer type will be unwrapped
+  // during simplification, and the operation fails if no pointer type is found.
+  QualType simplifyType(QualType Type, const Expr *E, bool UnwrapPointer);
+
 private:
   ASTContext &Ctx;
 };
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index fdef57e84ee3d..89ce852b0a09f 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -290,7 +290,8 @@ class FileNullabilityMap {
 /// parameter. This avoids updating the type on hot paths in the parser.
 class PreferredTypeBuilder {
 public:
-  PreferredTypeBuilder(bool Enabled) : Enabled(Enabled) {}
+  PreferredTypeBuilder(ASTContext *Ctx, bool Enabled)
+      : Ctx(Ctx), Enabled(Enabled) {}
 
   void enterCondition(Sema &S, SourceLocation Tok);
   void enterReturn(Sema &S, SourceLocation Tok);
@@ -336,6 +337,7 @@ class PreferredTypeBuilder {
   }
 
 private:
+  ASTContext *Ctx;
   bool Enabled;
   /// Start position of a token for which we store expected type.
   SourceLocation ExpectedLoc;
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index 0710542f5e938..09e784a8e04de 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -52,8 +52,9 @@ IdentifierInfo *Parser::getSEHExceptKeyword() {
 }
 
 Parser::Parser(Preprocessor &pp, Sema &actions, bool skipFunctionBodies)
-    : PP(pp), PreferredType(pp.isCodeCompletionEnabled()), Actions(actions),
-      Diags(PP.getDiagnostics()), GreaterThanIsOperator(true),
+    : PP(pp),
+      PreferredType(&actions.getASTContext(), pp.isCodeCompletionEnabled()),
+      Actions(actions), Diags(PP.getDiagnostics()), GreaterThanIsOperator(true),
       ColonIsSacred(false), InMessageExpression(false),
       TemplateParameterDepth(0), ParsingInObjCContainer(false) {
   SkipFunctionBodies = pp.isCodeCompletionEnabled() || skipFunctionBodies;
diff --git a/clang/lib/Sema/HeuristicResolver.cpp b/clang/lib/Sema/HeuristicResolver.cpp
index 7aecd2a73b539..d377379c627db 100644
--- a/clang/lib/Sema/HeuristicResolver.cpp
+++ b/clang/lib/Sema/HeuristicResolver.cpp
@@ -47,6 +47,8 @@ class HeuristicResolverImpl {
   std::vector<const NamedDecl *>
   lookupDependentName(CXXRecordDecl *RD, DeclarationName Name,
                       llvm::function_ref<bool(const NamedDecl *ND)> Filter);
+  TagDecl *resolveTypeToTagDecl(QualType T);
+  QualType simplifyType(QualType Type, const Expr *E, bool UnwrapPointer);
 
 private:
   ASTContext &Ctx;
@@ -72,20 +74,6 @@ class HeuristicResolverImpl {
   QualType resolveExprToType(const Expr *E);
   std::vector<const NamedDecl *> resolveExprToDecls(const Expr *E);
 
-  // Helper function for HeuristicResolver::resolveDependentMember()
-  // which takes a possibly-dependent type `T` and heuristically
-  // resolves it to a TagDecl in which we can try name lookup.
-  TagDecl *resolveTypeToTagDecl(const Type *T);
-
-  // Helper function for simplifying a type.
-  // `Type` is the type to simplify.
-  // `E` is the expression whose type `Type` is, if known. This sometimes
-  // contains information relevant to the type that's not stored in `Type`
-  // itself.
-  // If `UnwrapPointer` is true, exactly only pointer type will be unwrapped
-  // during simplification, and the operation fails if no pointer type is found.
-  QualType simplifyType(QualType Type, const Expr *E, bool UnwrapPointer);
-
   bool findOrdinaryMemberInDependentClasses(const CXXBaseSpecifier *Specifier,
                                             CXXBasePath &Path,
                                             DeclarationName Name);
@@ -132,8 +120,10 @@ TemplateName getReferencedTemplateName(const Type *T) {
 // Helper function for HeuristicResolver::resolveDependentMember()
 // which takes a possibly-dependent type `T` and heuristically
 // resolves it to a CXXRecordDecl in which we can try name lookup.
-TagDecl *HeuristicResolverImpl::resolveTypeToTagDecl(const Type *T) {
-  assert(T);
+TagDecl *HeuristicResolverImpl::resolveTypeToTagDecl(QualType QT) {
+  const Type *T = QT.getTypePtrOrNull();
+  if (!T)
+    return nullptr;
 
   // Unwrap type sugar such as type aliases.
   T = T->getCanonicalTypeInternal().getTypePtr();
@@ -147,7 +137,15 @@ TagDecl *HeuristicResolverImpl::resolveTypeToTagDecl(const Type *T) {
   }
 
   if (auto *TT = T->getAs<TagType>()) {
-    return TT->getDecl();
+    TagDecl *TD = TT->getDecl();
+    // Template might not be instantiated yet, fall back to primary template
+    // in such cases.
+    if (const auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(TD)) {
+      if (CTSD->getTemplateSpecializationKind() == TSK_Undeclared) {
+        return CTSD->getSpecializedTemplate()->getTemplatedDecl();
+      }
+    }
+    return TD;
   }
 
   if (const auto *ICNT = T->getAs<InjectedClassNameType>())
@@ -315,8 +313,7 @@ HeuristicResolverImpl::resolveTypeOfCallExpr(const CallExpr *CE) {
   if (const auto *FnTypePtr = CalleeType->getAs<PointerType>())
     CalleeType = FnTypePtr->getPointeeType();
   if (const FunctionType *FnType = CalleeType->getAs<FunctionType>()) {
-    if (const auto *D =
-            resolveTypeToTagDecl(FnType->getReturnType().getTypePtr())) {
+    if (const auto *D = resolveTypeToTagDecl(FnType->getReturnType())) {
       return {D};
     }
   }
@@ -427,7 +424,7 @@ bool findOrdinaryMember(const CXXRecordDecl *RD, CXXBasePath &Path,
 bool HeuristicResolverImpl::findOrdinaryMemberInDependentClasses(
     const CXXBaseSpecifier *Specifier, CXXBasePath &Path,
     DeclarationName Name) {
-  TagDecl *TD = resolveTypeToTagDecl(Specifier->getType().getTypePtr());
+  TagDecl *TD = resolveTypeToTagDecl(Specifier->getType());
   if (const auto *RD = dyn_cast_if_present<CXXRecordDecl>(TD)) {
     return findOrdinaryMember(RD, Path, Name);
   }
@@ -470,10 +467,7 @@ std::vector<const NamedDecl *> HeuristicResolverImpl::lookupDependentName(
 std::vector<const NamedDecl *> HeuristicResolverImpl::resolveDependentMember(
     QualType QT, DeclarationName Name,
     llvm::function_ref<bool(const NamedDecl *ND)> Filter) {
-  const Type *T = QT.getTypePtrOrNull();
-  if (!T)
-    return {};
-  TagDecl *TD = resolveTypeToTagDecl(T);
+  TagDecl *TD = resolveTypeToTagDecl(QT);
   if (!TD)
     return {};
   if (auto *ED = dyn_cast<EnumDecl>(TD)) {
@@ -540,5 +534,12 @@ std::vector<const NamedDecl *> HeuristicResolver::lookupDependentName(
 const QualType HeuristicResolver::getPointeeType(QualType T) const {
   return HeuristicResolverImpl(Ctx).getPointeeType(T);
 }
+TagDecl *HeuristicResolver::resolveTypeToTagDecl(QualType T) const {
+  return HeuristicResolverImpl(Ctx).resolveTypeToTagDecl(T);
+}
+QualType HeuristicResolver::simplifyType(QualType Type, const Expr *E,
+                                         bool UnwrapPointer) {
+  return HeuristicResolverImpl(Ctx).simplifyType(Type, E, UnwrapPointer);
+}
 
 } // namespace clang
diff --git a/clang/lib/Sema/SemaCodeComplete.cpp b/clang/lib/Sema/SemaCodeComplete.cpp
index db467d76b5d32..c20d47d079b9b 100644
--- a/clang/lib/Sema/SemaCodeComplete.cpp
+++ b/clang/lib/Sema/SemaCodeComplete.cpp
@@ -436,7 +436,8 @@ void PreferredTypeBuilder::enterVariableInit(SourceLocation Tok, Decl *D) {
   ExpectedLoc = Tok;
 }
 
-static QualType getDesignatedType(QualType BaseType, const Designation &Desig);
+static QualType getDesignatedType(QualType BaseType, const Designation &Desig,
+                                  HeuristicResolver &Resolver);
 
 void PreferredTypeBuilder::enterDesignatedInitializer(SourceLocation Tok,
                                                       QualType BaseType,
@@ -444,7 +445,8 @@ void PreferredTypeBuilder::enterDesignatedInitializer(SourceLocation Tok,
   if (!Enabled)
     return;
   ComputeType = nullptr;
-  Type = getDesignatedType(BaseType, D);
+  HeuristicResolver Resolver(*Ctx);
+  Type = getDesignatedType(BaseType, D, Resolver);
   ExpectedLoc = Tok;
 }
 
@@ -5346,27 +5348,11 @@ AddRecordMembersCompletionResults(Sema &SemaRef, ResultBuilder &Results,
 // Returns the RecordDecl inside the BaseType, falling back to primary template
 // in case of specializations. Since we might not have a decl for the
 // instantiation/specialization yet, e.g. dependent code.
-static RecordDecl *getAsRecordDecl(QualType BaseType) {
-  BaseType = BaseType.getNonReferenceType();
-  if (auto *RD = BaseType->getAsRecordDecl()) {
-    if (const auto *CTSD =
-            llvm::dyn_cast<ClassTemplateSpecializationDecl>(RD)) {
-      // Template might not be instantiated yet, fall back to primary template
-      // in such cases.
-      if (CTSD->getTemplateSpecializationKind() == TSK_Undeclared)
-        RD = CTSD->getSpecializedTemplate()->getTemplatedDecl();
-    }
-    return RD;
-  }
-
-  if (const auto *TST = BaseType->getAs<TemplateSpecializationType>()) {
-    if (const auto *TD = dyn_cast_or_null<ClassTemplateDecl>(
-            TST->getTemplateName().getAsTemplateDecl())) {
-      return TD->getTemplatedDecl();
-    }
-  }
-
-  return nullptr;
+static RecordDecl *getAsRecordDecl(QualType BaseType,
+                                   HeuristicResolver &Resolver) {
+  BaseType = Resolver.simplifyType(BaseType, nullptr, /*UnwrapPointer=*/false);
+  return dyn_cast_if_present<RecordDecl>(
+      Resolver.resolveTypeToTagDecl(BaseType));
 }
 
 namespace {
@@ -5911,7 +5897,7 @@ void SemaCodeCompletion::CodeCompleteMemberReferenceExpr(
       }
     }
 
-    if (RecordDecl *RD = getAsRecordDecl(BaseType)) {
+    if (RecordDecl *RD = getAsRecordDecl(BaseType, Resolver)) {
       AddRecordMembersCompletionResults(SemaRef, Results, S, BaseType, BaseKind,
                                         RD, std::move(AccessOpFixIt));
     } else if (const auto *TTPT =
@@ -6674,7 +6660,8 @@ QualType SemaCodeCompletion::ProduceTemplateArgumentSignatureHelp(
                               /*Braced=*/false);
 }
 
-static QualType getDesignatedType(QualType BaseType, const Designation &Desig) {
+static QualType getDesignatedType(QualType BaseType, const Designation &Desig,
+                                  HeuristicResolver &Resolver) {
   for (unsigned I = 0; I < Desig.getNumDesignators(); ++I) {
     if (BaseType.isNull())
       break;
@@ -6685,7 +6672,7 @@ static QualType getDesignatedType(QualType BaseType, const Designation &Desig) {
         NextType = BaseType->getAsArrayTypeUnsafe()->getElementType();
     } else {
       assert(D.isFieldDesignator());
-      auto *RD = getAsRecordDecl(BaseType);
+      auto *RD = getAsRecordDecl(BaseType, Resolver);
       if (RD && RD->isCompleteDefinition()) {
         for (const auto *Member : RD->lookup(D.getFieldDecl()))
           if (const FieldDecl *FD = llvm::dyn_cast<FieldDecl>(Member)) {
@@ -6701,10 +6688,10 @@ static QualType getDesignatedType(QualType BaseType, const Designation &Desig) {
 
 void SemaCodeCompletion::CodeCompleteDesignator(
     QualType BaseType, llvm::ArrayRef<Expr *> InitExprs, const Designation &D) {
-  BaseType = getDesignatedType(BaseType, D);
+  BaseType = getDesignatedType(BaseType, D, Resolver);
   if (BaseType.isNull())
     return;
-  const auto *RD = getAsRecordDecl(BaseType);
+  const auto *RD = getAsRecordDecl(BaseType, Resolver);
   if (!RD || RD->fields().empty())
     return;
 
diff --git a/clang/test/CodeCompletion/member-access.cpp b/clang/test/CodeCompletion/member-access.cpp
index b181466cdb628..8526ed7273474 100644
--- a/clang/test/CodeCompletion/member-access.cpp
+++ b/clang/test/CodeCompletion/member-access.cpp
@@ -431,7 +431,9 @@ using Alias = S<T>;
 template <typename T>
 void f(Alias<T> s) {
   s.a.b;
-  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:433:7 %s -o - | FileCheck -check-prefix=CHECK-TEMPLATE-ALIAS %s
-  // CHECK-TEMPLATE-ALIAS: [#int#]b
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:433:5 %s -o - | FileCheck -check-prefix=CHECK-TEMPLATE-ALIAS %s
+  // CHECK-TEMPLATE-ALIAS: [#A#]a
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:433:7 %s -o - | FileCheck -check-prefix=CHECK-TEMPLATE-ALIAS-NESTED %s
+  // CHECK-TEMPLATE-ALIAS-NESTED: [#int#]b
 }
 }

[HighCommander4]

I split the PR up into a few commits for easier reviewing.

The high-level idea here is that the helper function getAsRecordDecl() in SemaCodeComplete.cpp is doing similar things to HeuristicResolver (specifically, simplifyType and resolveTypeToRecordDecl), but HeuristicResolver is handling a few more cases, and to get code completion to benefit from those cases the patch gets the helper function to use HeuristicResolver.

There was one case, TSK_Undeclared, that getAsRecordDecl() handled but HeuristicResolver didn't. I moved that handling into HeuristicResolver. I contemplated adding a standalone test case for it to HeuristicResolverTests but it's not easy because the situation only arises during code completion when the AST is not fully built. The case does have test coverage, in clang/test/CodeCompletion/desig-init.cpp (this test fails without the TSK_Undeclared handling).

[HighCommander4]

(I messed things up a bit here by merging a PR that was stacked on top of this into this one. Marking as Draft until I sort that out.)

[HighCommander4]

Fixed now. Resubmitting for review.

[HighCommander4]

(Rebased)

[HighCommander4]

Review ping :)

[zyn0217]

Thanks for consistently working on this!

[HighCommander4]

(Rebased)