[Clang][Sema] Fix bug where operator-> typo corrects in the current instantiation

[sdkrystian]

Fixes this bug introduced in #90152.

This bug occurs when typo-correction attempts to fix a reference to a non-existent member of the current instantiation (even though operator-> may return a different type than the object type). This patch fixes it by simply considering the object expression to be of type ASTContext::DependentTy when the arrow operator is used with a dependent non-pointer non-function operand (after any implicit conversions).

[llvmbot]

@llvm/pr-subscribers-clang

Author: Krystian Stasiowski (sdkrystian)

Changes

Fixes this bug introduced in #90152.

This bug occurs when typo-correction attempts to fix a reference to a non-existent member of the current instantiation (even though operator-> may return a different type than the object type). This patch fixes it by simply considering the object expression to be ASTContext::DependentTy when the arrow operator is used with a dependent non-pointer non-function operand (after any implicit conversions).

---

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

2 Files Affected:

• (modified) clang/lib/Sema/SemaExprMember.cpp (+7-7)
• (modified) clang/test/CXX/temp/temp.res/temp.dep/temp.dep.type/p4.cpp (+7)

diff --git a/clang/lib/Sema/SemaExprMember.cpp b/clang/lib/Sema/SemaExprMember.cpp
index 9fa69da4f9685..a3411b3036d5e 100644
--- a/clang/lib/Sema/SemaExprMember.cpp
+++ b/clang/lib/Sema/SemaExprMember.cpp
@@ -995,11 +995,9 @@ Sema::BuildMemberReferenceExpr(Expr *BaseExpr, QualType BaseExprType,
   // arrow operator was used with a dependent non-pointer object expression,
   // build a CXXDependentScopeMemberExpr.
   if (R.wasNotFoundInCurrentInstantiation() ||
-      (IsArrow && !BaseExprType->isPointerType() &&
-       BaseExprType->isDependentType()) ||
       (R.getLookupName().getCXXOverloadedOperator() == OO_Equal &&
-       (SS.isSet() ? SS.getScopeRep()->isDependent()
-                   : BaseExprType->isDependentType())))
+      (SS.isSet() ? SS.getScopeRep()->isDependent()
+                  : BaseExprType->isDependentType())))
     return ActOnDependentMemberExpr(BaseExpr, BaseExprType, IsArrow, OpLoc, SS,
                                     TemplateKWLoc, FirstQualifierInScope,
                                     R.getLookupNameInfo(), TemplateArgs);
@@ -1322,7 +1320,11 @@ static ExprResult LookupMemberExpr(Sema &S, LookupResult &R,
     else if (const ObjCObjectPointerType *Ptr =
                  BaseType->getAs<ObjCObjectPointerType>())
       BaseType = Ptr->getPointeeType();
-    else if (!BaseType->isDependentType()) {
+    else if (BaseType->isFunctionType())
+      goto fail;
+    else if (BaseType->isDependentType())
+      BaseType = S.Context.DependentTy;
+    else {
       if (BaseType->isRecordType()) {
         // Recover from arrow accesses to records, e.g.:
         //   struct MyRecord foo;
@@ -1337,8 +1339,6 @@ static ExprResult LookupMemberExpr(Sema &S, LookupResult &R,
               << 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();
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
index 1adbc33a701c1..fafd54bde7622 100644
--- 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
@@ -551,4 +551,11 @@ namespace N4 {
 
   template void D<B>::instantiated(D); // expected-note {{in instantiation of}}
 
+  template<typename T>
+  struct Typo {
+    void not_instantiated(Typo a) {
+      a->Not_instantiated;
+      a->typo;
+    }
+  };
 } // namespace N4

[erichkeane]

I think this needs a release note? I know the only case we have right now is because of your other patch, but perhaps this affects elsewhere too?

[sdkrystian]

I think this needs a release note? I know the only case we have right now is because of your other patch, but perhaps this affects elsewhere too?

@erichkeane I don't think we need a release note since the code changes are in LookupMemberExpr, which prior to #90152 had no valid code path for dependent types (and had the assert assert(!BaseType->isDependentType()) to ensure this).

[erichkeane]

I think this needs a release note? I know the only case we have right now is because of your other patch, but perhaps this affects elsewhere too?

@erichkeane I don't think we need a release note since the code changes are in LookupMemberExpr, which prior to #90152 had no valid code path for dependent types (and had the assert assert(!BaseType->isDependentType()) to ensure this).

Ah! Missed that, yes, you're right.