Skip to content

Conversation

@sdkrystian
Copy link
Member

Reverts #109422

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jan 22, 2025
@llvmbot
Copy link
Member

llvmbot commented Jan 22, 2025

@llvm/pr-subscribers-clang

Author: Krystian Stasiowski (sdkrystian)

Changes

Reverts llvm/llvm-project#109422


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

6 Files Affected:

  • (modified) clang/include/clang/Sema/Sema.h (+3-2)
  • (modified) clang/lib/Sema/SemaExprCXX.cpp (+17-4)
  • (modified) clang/lib/Sema/SemaExprMember.cpp (+1-1)
  • (modified) clang/lib/Sema/SemaOverload.cpp (+5-15)
  • (modified) clang/lib/Sema/TreeTransform.h (+2-3)
  • (modified) clang/test/CXX/temp/temp.res/temp.dep/temp.dep.type/p4.cpp (+11-34)
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index a2a47d535b8e06..9fa33d6ca76ba5 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -10608,8 +10608,9 @@ class Sema final : public SemaBase {
   /// BuildOverloadedArrowExpr - Build a call to an overloaded @c operator->
   ///  (if one exists), where @c Base is an expression of class type and
   /// @c Member is the name of the member we're trying to find.
-  ExprResult BuildOverloadedArrowExpr(Expr *Base, SourceLocation OpLoc,
-                                      bool *NoArrowOperatorFound);
+  ExprResult BuildOverloadedArrowExpr(Scope *S, Expr *Base,
+                                      SourceLocation OpLoc,
+                                      bool *NoArrowOperatorFound = nullptr);
 
   ExprResult BuildCXXMemberCallExpr(Expr *Exp, NamedDecl *FoundDecl,
                                     CXXConversionDecl *Method,
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 0ebf5f54613926..1e39d69e8b230f 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -7999,6 +7999,18 @@ ExprResult Sema::ActOnStartCXXMemberReference(Scope *S, Expr *Base,
 
   QualType BaseType = Base->getType();
   MayBePseudoDestructor = false;
+  if (BaseType->isDependentType()) {
+    // If we have a pointer to a dependent type and are using the -> operator,
+    // the object type is the type that the pointer points to. We might still
+    // have enough information about that type to do something useful.
+    if (OpKind == tok::arrow)
+      if (const PointerType *Ptr = BaseType->getAs<PointerType>())
+        BaseType = Ptr->getPointeeType();
+
+    ObjectType = ParsedType::make(BaseType);
+    MayBePseudoDestructor = true;
+    return Base;
+  }
 
   // C++ [over.match.oper]p8:
   //   [...] When operator->returns, the operator-> is applied  to the value
@@ -8013,7 +8025,7 @@ ExprResult Sema::ActOnStartCXXMemberReference(Scope *S, Expr *Base,
     SmallVector<FunctionDecl*, 8> OperatorArrows;
     CTypes.insert(Context.getCanonicalType(BaseType));
 
-    while (BaseType->getAsRecordDecl()) {
+    while (BaseType->isRecordType()) {
       if (OperatorArrows.size() >= getLangOpts().ArrowDepth) {
         Diag(OpLoc, diag::err_operator_arrow_depth_exceeded)
           << StartingType << getLangOpts().ArrowDepth << Base->getSourceRange();
@@ -8024,7 +8036,7 @@ ExprResult Sema::ActOnStartCXXMemberReference(Scope *S, Expr *Base,
       }
 
       Result = BuildOverloadedArrowExpr(
-          Base, OpLoc,
+          S, Base, OpLoc,
           // When in a template specialization and on the first loop iteration,
           // potentially give the default diagnostic (with the fixit in a
           // separate note) instead of having the error reported back to here
@@ -8088,7 +8100,7 @@ ExprResult Sema::ActOnStartCXXMemberReference(Scope *S, Expr *Base,
   // it's legal for the type to be incomplete if this is a pseudo-destructor
   // call.  We'll do more incomplete-type checks later in the lookup process,
   // so just skip this check for ObjC types.
-  if (BaseType->isDependentType() || !BaseType->isRecordType()) {
+  if (!BaseType->isRecordType()) {
     ObjectType = ParsedType::make(BaseType);
     MayBePseudoDestructor = true;
     return Base;
@@ -8099,7 +8111,8 @@ ExprResult Sema::ActOnStartCXXMemberReference(Scope *S, Expr *Base,
   //   Unlike the object expression in other contexts, *this is not required to
   //   be of complete type for purposes of class member access (5.2.5) outside
   //   the member function body.
-  if (!isThisOutsideMemberFunctionBody(BaseType) &&
+  if (!BaseType->isDependentType() &&
+      !isThisOutsideMemberFunctionBody(BaseType) &&
       RequireCompleteType(OpLoc, BaseType,
                           diag::err_incomplete_member_access)) {
     return CreateRecoveryExpr(Base->getBeginLoc(), Base->getEndLoc(), {Base});
diff --git a/clang/lib/Sema/SemaExprMember.cpp b/clang/lib/Sema/SemaExprMember.cpp
index 8326a4db0a7719..d130e8b86bc56d 100644
--- a/clang/lib/Sema/SemaExprMember.cpp
+++ b/clang/lib/Sema/SemaExprMember.cpp
@@ -1357,7 +1357,7 @@ static ExprResult LookupMemberExpr(Sema &S, LookupResult &R,
       BaseType = Ptr->getPointeeType();
     else if (BaseType->isFunctionType())
       goto fail;
-    else if (BaseExpr.get()->isTypeDependent())
+    else if (BaseType->isDependentType())
       BaseType = S.Context.DependentTy;
     else if (BaseType->isRecordType()) {
       // Recover from arrow accesses to records, e.g.:
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index bf4c0288274ac7..7e8811b5274efb 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -15962,9 +15962,10 @@ Sema::BuildCallToObjectOfClassType(Scope *S, Expr *Obj,
   return CheckForImmediateInvocation(MaybeBindToTemporary(TheCall), Method);
 }
 
-ExprResult Sema::BuildOverloadedArrowExpr(Expr *Base, SourceLocation OpLoc,
-                                          bool *NoArrowOperatorFound) {
-  assert(Base->getType()->getAsRecordDecl() &&
+ExprResult
+Sema::BuildOverloadedArrowExpr(Scope *S, Expr *Base, SourceLocation OpLoc,
+                               bool *NoArrowOperatorFound) {
+  assert(Base->getType()->isRecordType() &&
          "left-hand side must have class type");
 
   if (checkPlaceholderForOverload(*this, Base))
@@ -15987,20 +15988,9 @@ ExprResult Sema::BuildOverloadedArrowExpr(Expr *Base, SourceLocation OpLoc,
     return ExprError();
 
   LookupResult R(*this, OpName, OpLoc, LookupOrdinaryName);
-  LookupParsedName(R, /*S=*/nullptr, /*SS=*/nullptr, Base->getType());
+  LookupQualifiedName(R, Base->getType()->castAs<RecordType>()->getDecl());
   R.suppressAccessDiagnostics();
 
-  if (Base->getType()->isDependentType() &&
-      (!R.empty() || R.wasNotFoundInCurrentInstantiation())) {
-    DeclarationNameInfo OpNameInfo(OpName, OpLoc);
-    ExprResult Fn = CreateUnresolvedLookupExpr(
-        /*NamingClass=*/nullptr, /*NNSLoc=*/NestedNameSpecifierLoc(),
-        OpNameInfo, R.asUnresolvedSet(), /*PerformADL=*/false);
-    return CXXOperatorCallExpr::Create(Context, OO_Arrow, Fn.get(), Base,
-                                       Context.DependentTy, VK_PRValue, OpLoc,
-                                       CurFPFeatureOverrides());
-  }
-
   for (LookupResult::iterator Oper = R.begin(), OperEnd = R.end();
        Oper != OperEnd; ++Oper) {
     AddMethodCandidate(Oper.getPair(), Base->getType(), Base->Classify(Context),
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index c0363692a3eb72..7dc88a1ae23b98 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -17282,11 +17282,10 @@ ExprResult TreeTransform<Derived>::RebuildCXXOperatorCallExpr(
   } else if (Op == OO_Arrow) {
     // It is possible that the type refers to a RecoveryExpr created earlier
     // in the tree transformation.
-    if (First->containsErrors())
+    if (First->getType()->isDependentType())
       return ExprError();
     // -> is never a builtin operation.
-    return getSema().BuildOverloadedArrowExpr(First, OpLoc,
-                                              /*NoArrowOperatorFound=*/nullptr);
+    return SemaRef.BuildOverloadedArrowExpr(nullptr, First, OpLoc);
   } else if (Second == nullptr || isPostIncDec) {
     if (!First->getType()->isOverloadableType() ||
         (Op == OO_Amp && getSema().isQualifiedMemberAccess(First))) {
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 03eda1f13feed7..f32f49ef4539a5 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
@@ -484,19 +484,16 @@ namespace N4 {
   template<typename T>
   struct A {
     void not_instantiated(A a, A<T> b, T c) {
-      a->x; // expected-error {{member reference type 'A<T>' is not a pointer; did you mean to use '.'?}}
-      b->x; // expected-error {{member reference type 'A<T>' is not a pointer; did you mean to use '.'?}}
+      a->x;
+      b->x;
       c->x;
     }
 
     void instantiated(A a, A<T> b, T c) {
-      // FIXME: We should only emit a single diagnostic suggesting to use '.'!
-      a->x; // expected-error {{member reference type 'A<T>' is not a pointer; did you mean to use '.'?}}
-            // expected-error@-1 {{member reference type 'A<int>' is not a pointer; did you mean to use '.'?}}
-            // expected-error@-2 {{no member named 'x' in 'N4::A<int>'}}
-      b->x; // expected-error {{member reference type 'A<T>' is not a pointer; did you mean to use '.'?}}
-            // expected-error@-1 {{member reference type 'A<int>' is not a pointer; did you mean to use '.'?}}
-            // expected-error@-2 {{no member named 'x' in 'N4::A<int>'}}
+      a->x; // expected-error {{member reference type 'A<int>' is not a pointer; did you mean to use '.'?}}
+            // expected-error@-1 {{no member named 'x' in 'N4::A<int>'}}
+      b->x; // expected-error {{member reference type 'A<int>' is not a pointer; did you mean to use '.'?}}
+            // expected-error@-1 {{no member named 'x' in 'N4::A<int>'}}
       c->x; // expected-error {{member reference type 'int' is not a pointer}}
     }
   };
@@ -543,10 +540,11 @@ namespace N4 {
       a->T::f();
       a->T::g();
 
-      a->U::x;
-      a->U::y;
-      a->U::f();
-      a->U::g();
+      // FIXME: 'U' should be a dependent name, and its lookup context should be 'a.operator->()'!
+      a->U::x; // expected-error {{use of undeclared identifier 'U'}}
+      a->U::y; // expected-error {{use of undeclared identifier 'U'}}
+      a->U::f(); // expected-error {{use of undeclared identifier 'U'}}
+      a->U::g(); // expected-error {{use of undeclared identifier 'U'}}
     }
 
     void instantiated(D a) {
@@ -607,24 +605,3 @@ namespace N5 {
 
   template void g(int); // expected-note {{in instantiation of}}
 } // namespace N5
-
-namespace N6 {
-  struct A {
-    int x;
-  };
-
-  struct B {
-    A* operator->();
-  };
-
-  struct C {
-    B y;
-  };
-
-  template<typename T>
-  struct D : C {
-    void f() {
-      y->x;
-    }
-  };
-} // namespace N6

@github-actions
Copy link

github-actions bot commented Jan 22, 2025

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

@sdkrystian sdkrystian merged commit 5ede7b6 into main Jan 22, 2025
5 of 6 checks passed
@sdkrystian sdkrystian deleted the revert-109422-reapply-fix-overloaded-arrow branch January 22, 2025 19:18
@llvm-ci
Copy link
Collaborator

llvm-ci commented Jan 22, 2025

LLVM Buildbot has detected a new failure on builder llvm-clang-aarch64-darwin running on doug-worker-5 while building clang at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/190/builds/13304

Here is the relevant piece of the build log for the reference
Step 6 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'LLVM :: ExecutionEngine/OrcLazy/multiple-compile-threads-basic.ll' FAILED ********************
Exit Code: 2

Command Output (stderr):
--
RUN: at line 1: /Users/buildbot/buildbot-root/aarch64-darwin/build/bin/lli -jit-kind=orc-lazy -compile-threads=2 -thread-entry hello /Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/llvm/test/ExecutionEngine/OrcLazy/multiple-compile-threads-basic.ll | /Users/buildbot/buildbot-root/aarch64-darwin/build/bin/FileCheck /Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/llvm/test/ExecutionEngine/OrcLazy/multiple-compile-threads-basic.ll
+ /Users/buildbot/buildbot-root/aarch64-darwin/build/bin/lli -jit-kind=orc-lazy -compile-threads=2 -thread-entry hello /Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/llvm/test/ExecutionEngine/OrcLazy/multiple-compile-threads-basic.ll
+ /Users/buildbot/buildbot-root/aarch64-darwin/build/bin/FileCheck /Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/llvm/test/ExecutionEngine/OrcLazy/multiple-compile-threads-basic.ll
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
 #0 0x000000010547e468 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/Users/buildbot/buildbot-root/aarch64-darwin/build/bin/lli+0x100e9a468)
 #1 0x000000010547c4ec llvm::sys::RunSignalHandlers() (/Users/buildbot/buildbot-root/aarch64-darwin/build/bin/lli+0x100e984ec)
 #2 0x000000010547eb24 SignalHandler(int) (/Users/buildbot/buildbot-root/aarch64-darwin/build/bin/lli+0x100e9ab24)
 #3 0x0000000191c1a584 (/usr/lib/system/libsystem_platform.dylib+0x18047a584)
 #4 0x0000000191be921c (/usr/lib/system/libsystem_pthread.dylib+0x18044921c)
 #5 0x0000000191b0fad0 (/usr/lib/libc++.1.dylib+0x18036fad0)
 #6 0x000000010502e8dc void llvm::detail::UniqueFunctionBase<void, llvm::Expected<llvm::DenseMap<llvm::orc::SymbolStringPtr, llvm::orc::ExecutorSymbolDef, llvm::DenseMapInfo<llvm::orc::SymbolStringPtr, void>, llvm::detail::DenseMapPair<llvm::orc::SymbolStringPtr, llvm::orc::ExecutorSymbolDef>>>>::CallImpl<llvm::orc::Platform::lookupInitSymbols(llvm::orc::ExecutionSession&, llvm::DenseMap<llvm::orc::JITDylib*, llvm::orc::SymbolLookupSet, llvm::DenseMapInfo<llvm::orc::JITDylib*, void>, llvm::detail::DenseMapPair<llvm::orc::JITDylib*, llvm::orc::SymbolLookupSet>> const&)::$_45>(void*, llvm::Expected<llvm::DenseMap<llvm::orc::SymbolStringPtr, llvm::orc::ExecutorSymbolDef, llvm::DenseMapInfo<llvm::orc::SymbolStringPtr, void>, llvm::detail::DenseMapPair<llvm::orc::SymbolStringPtr, llvm::orc::ExecutorSymbolDef>>>&) (/Users/buildbot/buildbot-root/aarch64-darwin/build/bin/lli+0x100a4a8dc)
 #7 0x000000010502a64c llvm::orc::AsynchronousSymbolQuery::handleComplete(llvm::orc::ExecutionSession&)::RunQueryCompleteTask::run() (/Users/buildbot/buildbot-root/aarch64-darwin/build/bin/lli+0x100a4664c)
 #8 0x00000001050e69bc void* std::__1::__thread_proxy[abi:un170006]<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, llvm::orc::DynamicThreadPoolTaskDispatcher::dispatch(std::__1::unique_ptr<llvm::orc::Task, std::__1::default_delete<llvm::orc::Task>>)::$_0>>(void*) (/Users/buildbot/buildbot-root/aarch64-darwin/build/bin/lli+0x100b029bc)
 #9 0x0000000191be9f94 (/usr/lib/system/libsystem_pthread.dylib+0x180449f94)
#10 0x0000000191be4d34 (/usr/lib/system/libsystem_pthread.dylib+0x180444d34)
FileCheck error: '<stdin>' is empty.
FileCheck command line:  /Users/buildbot/buildbot-root/aarch64-darwin/build/bin/FileCheck /Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/llvm/test/ExecutionEngine/OrcLazy/multiple-compile-threads-basic.ll

--

********************


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants