[clang] Revert "[Clang][Sema] Fix lookup of dependent operator= outside of complete-class contexts (#91498)" (PR #91620)

Krystian Stasiowski via cfe-commits cfe-commits at lists.llvm.org
Thu May 9 10:35:47 PDT 2024


https://github.com/sdkrystian updated https://github.com/llvm/llvm-project/pull/91620

>From 7b2f3da17dfc93a4f0aa69ad4da90707b6f2e8b6 Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Thu, 9 May 2024 12:30:28 -0400
Subject: [PATCH 1/3] Revert "[Clang][Sema] Fix lookup of dependent operator=
 outside of complete-class contexts (#91498)"

This reverts commit 62b5b61f436add042d8729dc9837d055613180d9.
---
 clang/lib/Sema/SemaLookup.cpp                 | 35 ++++++++-----------
 clang/lib/Sema/SemaTemplate.cpp               |  7 ++--
 .../temp.res/temp.dep/temp.dep.type/p4.cpp    | 13 -------
 3 files changed, 20 insertions(+), 35 deletions(-)

diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index e20de338ebb16..e63da5875d2c9 100644
--- a/clang/lib/Sema/SemaLookup.cpp
+++ b/clang/lib/Sema/SemaLookup.cpp
@@ -1267,20 +1267,6 @@ struct FindLocalExternScope {
   LookupResult &R;
   bool OldFindLocalExtern;
 };
-
-/// Returns true if 'operator=' should be treated as a dependent name.
-bool isDependentAssignmentOperator(DeclarationName Name,
-                                   DeclContext *LookupContext) {
-  const auto *LookupRecord = dyn_cast_if_present<CXXRecordDecl>(LookupContext);
-  // If the lookup context is the current instantiation but we are outside a
-  // complete-class context, we will never find the implicitly declared
-  // copy/move assignment operators because they are declared at the closing '}'
-  // of the class specifier. In such cases, we treat 'operator=' like any other
-  // unqualified name because the results of name lookup in the template
-  // definition/instantiation context will always be the same.
-  return Name.getCXXOverloadedOperator() == OO_Equal && LookupRecord &&
-         !LookupRecord->isBeingDefined() && LookupRecord->isDependentContext();
-}
 } // end anonymous namespace
 
 bool Sema::CppLookupName(LookupResult &R, Scope *S) {
@@ -1289,6 +1275,13 @@ bool Sema::CppLookupName(LookupResult &R, Scope *S) {
   DeclarationName Name = R.getLookupName();
   Sema::LookupNameKind NameKind = R.getLookupKind();
 
+  // If this is the name of an implicitly-declared special member function,
+  // go through the scope stack to implicitly declare
+  if (isImplicitlyDeclaredMemberFunctionName(Name)) {
+    for (Scope *PreS = S; PreS; PreS = PreS->getParent())
+      if (DeclContext *DC = PreS->getEntity())
+        DeclareImplicitMemberFunctionsWithName(*this, Name, R.getNameLoc(), DC);
+  }
   // C++23 [temp.dep.general]p2:
   //   The component name of an unqualified-id is dependent if
   //   - it is a conversion-function-id whose conversion-type-id
@@ -1306,8 +1299,9 @@ bool Sema::CppLookupName(LookupResult &R, Scope *S) {
   if (isImplicitlyDeclaredMemberFunctionName(Name)) {
     for (Scope *PreS = S; PreS; PreS = PreS->getParent())
       if (DeclContext *DC = PreS->getEntity()) {
-        if (!R.isTemplateNameLookup() &&
-            isDependentAssignmentOperator(Name, DC)) {
+        if (DC->isDependentContext() && isa<CXXRecordDecl>(DC) &&
+            Name.getCXXOverloadedOperator() == OO_Equal &&
+            !R.isTemplateNameLookup()) {
           R.setNotFoundInCurrentInstantiation();
           return false;
         }
@@ -2478,6 +2472,8 @@ bool Sema::LookupQualifiedName(LookupResult &R, DeclContext *LookupCtx,
     }
   } QL(LookupCtx);
 
+  bool TemplateNameLookup = R.isTemplateNameLookup();
+  CXXRecordDecl *LookupRec = dyn_cast<CXXRecordDecl>(LookupCtx);
   if (!InUnqualifiedLookup && !R.isForRedeclaration()) {
     // C++23 [temp.dep.type]p5:
     //   A qualified name is dependent if
@@ -2490,14 +2486,13 @@ bool Sema::LookupQualifiedName(LookupResult &R, DeclContext *LookupCtx,
     if (DeclarationName Name = R.getLookupName();
         (Name.getNameKind() == DeclarationName::CXXConversionFunctionName &&
          Name.getCXXNameType()->isDependentType()) ||
-        (!R.isTemplateNameLookup() &&
-         isDependentAssignmentOperator(Name, LookupCtx))) {
+        (Name.getCXXOverloadedOperator() == OO_Equal && LookupRec &&
+         LookupRec->isDependentContext() && !TemplateNameLookup)) {
       R.setNotFoundInCurrentInstantiation();
       return false;
     }
   }
 
-  CXXRecordDecl *LookupRec = dyn_cast<CXXRecordDecl>(LookupCtx);
   if (LookupDirect(*this, R, LookupCtx)) {
     R.resolveKind();
     if (LookupRec)
@@ -2609,7 +2604,7 @@ bool Sema::LookupQualifiedName(LookupResult &R, DeclContext *LookupCtx,
         //   template, and if the name is used as a template-name, the
         //   reference refers to the class template itself and not a
         //   specialization thereof, and is not ambiguous.
-        if (R.isTemplateNameLookup())
+        if (TemplateNameLookup)
           if (auto *TD = getAsTemplateNameDecl(ND))
             ND = TD;
 
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 480bc74c2001a..7e57fa0696725 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -726,7 +726,7 @@ Sema::ActOnDependentIdExpression(const CXXScopeSpec &SS,
                                  const DeclarationNameInfo &NameInfo,
                                  bool isAddressOfOperand,
                            const TemplateArgumentListInfo *TemplateArgs) {
-  QualType ThisType = getCurrentThisType();
+  DeclContext *DC = getFunctionLevelDeclContext();
 
   // C++11 [expr.prim.general]p12:
   //   An id-expression that denotes a non-static data member or non-static
@@ -748,7 +748,10 @@ Sema::ActOnDependentIdExpression(const CXXScopeSpec &SS,
     IsEnum = isa_and_nonnull<EnumType>(NNS->getAsType());
 
   if (!MightBeCxx11UnevalField && !isAddressOfOperand && !IsEnum &&
-      !ThisType.isNull()) {
+      isa<CXXMethodDecl>(DC) &&
+      cast<CXXMethodDecl>(DC)->isImplicitObjectMemberFunction()) {
+    QualType ThisType = cast<CXXMethodDecl>(DC)->getThisType().getNonReferenceType();
+
     // Since the 'this' expression is synthesized, we don't need to
     // perform the double-lookup check.
     NamedDecl *FirstQualifierInScope = nullptr;
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 43053c18c5076..46dd52f8c4c13 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
@@ -471,19 +471,6 @@ namespace N3 {
       this->C::operator=(*this);
     }
   };
-
-  template<typename T>
-  struct D {
-    auto not_instantiated() -> decltype(operator=(0)); // expected-error {{use of undeclared 'operator='}}
-  };
-
-  template<typename T>
-  struct E {
-    auto instantiated(E& e) -> decltype(operator=(e)); // expected-error {{use of undeclared 'operator='}}
-  };
-
-  template struct E<int>; // expected-note {{in instantiation of template class 'N3::E<int>' requested here}}
-
 } // namespace N3
 
 namespace N4 {

>From eb65b733c7d62eee885f2798979fe97db9d253ab Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Thu, 9 May 2024 12:41:44 -0400
Subject: [PATCH 2/3] Revert "[Clang][Sema] Fix template name lookup for
 operator= (#90999)"

This reverts commit 3191e0b52725aa17651e38d26284386f3ea64eb6.
---
 clang/lib/Sema/SemaLookup.cpp                  | 11 +++++++----
 .../temp.res/temp.dep/temp.dep.type/p4.cpp     | 18 ------------------
 2 files changed, 7 insertions(+), 22 deletions(-)

diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index e63da5875d2c9..2f6ad49fc08b6 100644
--- a/clang/lib/Sema/SemaLookup.cpp
+++ b/clang/lib/Sema/SemaLookup.cpp
@@ -1300,8 +1300,7 @@ bool Sema::CppLookupName(LookupResult &R, Scope *S) {
     for (Scope *PreS = S; PreS; PreS = PreS->getParent())
       if (DeclContext *DC = PreS->getEntity()) {
         if (DC->isDependentContext() && isa<CXXRecordDecl>(DC) &&
-            Name.getCXXOverloadedOperator() == OO_Equal &&
-            !R.isTemplateNameLookup()) {
+            Name.getCXXOverloadedOperator() == OO_Equal) {
           R.setNotFoundInCurrentInstantiation();
           return false;
         }
@@ -2472,8 +2471,10 @@ bool Sema::LookupQualifiedName(LookupResult &R, DeclContext *LookupCtx,
     }
   } QL(LookupCtx);
 
-  bool TemplateNameLookup = R.isTemplateNameLookup();
   CXXRecordDecl *LookupRec = dyn_cast<CXXRecordDecl>(LookupCtx);
+  // FIXME: Per [temp.dep.general]p2, an unqualified name is also dependent
+  // if it's a dependent conversion-function-id or operator= where the current
+  // class is a templated entity. This should be handled in LookupName.
   if (!InUnqualifiedLookup && !R.isForRedeclaration()) {
     // C++23 [temp.dep.type]p5:
     //   A qualified name is dependent if
@@ -2487,7 +2488,7 @@ bool Sema::LookupQualifiedName(LookupResult &R, DeclContext *LookupCtx,
         (Name.getNameKind() == DeclarationName::CXXConversionFunctionName &&
          Name.getCXXNameType()->isDependentType()) ||
         (Name.getCXXOverloadedOperator() == OO_Equal && LookupRec &&
-         LookupRec->isDependentContext() && !TemplateNameLookup)) {
+         LookupRec->isDependentContext())) {
       R.setNotFoundInCurrentInstantiation();
       return false;
     }
@@ -2583,6 +2584,8 @@ bool Sema::LookupQualifiedName(LookupResult &R, DeclContext *LookupCtx,
     return true;
   };
 
+  bool TemplateNameLookup = R.isTemplateNameLookup();
+
   // Determine whether two sets of members contain the same members, as
   // required by C++ [class.member.lookup]p6.
   auto HasSameDeclarations = [&](DeclContext::lookup_iterator A,
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 46dd52f8c4c13..0f24d716a7b74 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
@@ -453,24 +453,6 @@ namespace N3 {
       this->A::operator=(*this);
     }
   };
-
-  template<typename T>
-  struct C {
-    template<typename U>
-    void operator=(int);
-
-    void not_instantiated() {
-      operator=<int>(0);
-      C::operator=<int>(0);
-      this->operator=<int>(0);
-      this->C::operator=<int>(0);
-
-      operator=(*this);
-      C::operator=(*this);
-      this->operator=(*this);
-      this->C::operator=(*this);
-    }
-  };
 } // namespace N3
 
 namespace N4 {

>From eabdb1ca528f0d15143c4f7326d87bdc0428d577 Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Thu, 9 May 2024 13:11:49 -0400
Subject: [PATCH 3/3] [Clang][Sema] Partially revert changes to operator= from
 #90152

---
 clang/lib/Sema/SemaExprMember.cpp             |  5 +++-
 clang/lib/Sema/SemaLookup.cpp                 | 21 ++-----------
 clang/lib/Sema/SemaTemplate.cpp               |  3 +-
 .../temp.res/temp.dep/temp.dep.type/p4.cpp    | 30 +++++++++++++++++++
 4 files changed, 39 insertions(+), 20 deletions(-)

diff --git a/clang/lib/Sema/SemaExprMember.cpp b/clang/lib/Sema/SemaExprMember.cpp
index 5facb14a18b7c..9fa69da4f9685 100644
--- a/clang/lib/Sema/SemaExprMember.cpp
+++ b/clang/lib/Sema/SemaExprMember.cpp
@@ -996,7 +996,10 @@ Sema::BuildMemberReferenceExpr(Expr *BaseExpr, QualType BaseExprType,
   // build a CXXDependentScopeMemberExpr.
   if (R.wasNotFoundInCurrentInstantiation() ||
       (IsArrow && !BaseExprType->isPointerType() &&
-       BaseExprType->isDependentType()))
+       BaseExprType->isDependentType()) ||
+      (R.getLookupName().getCXXOverloadedOperator() == OO_Equal &&
+       (SS.isSet() ? SS.getScopeRep()->isDependent()
+                   : BaseExprType->isDependentType())))
     return ActOnDependentMemberExpr(BaseExpr, BaseExprType, IsArrow, OpLoc, SS,
                                     TemplateKWLoc, FirstQualifierInScope,
                                     R.getLookupNameInfo(), TemplateArgs);
diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index 2f6ad49fc08b6..7251aabc6af21 100644
--- a/clang/lib/Sema/SemaLookup.cpp
+++ b/clang/lib/Sema/SemaLookup.cpp
@@ -1282,6 +1282,7 @@ bool Sema::CppLookupName(LookupResult &R, Scope *S) {
       if (DeclContext *DC = PreS->getEntity())
         DeclareImplicitMemberFunctionsWithName(*this, Name, R.getNameLoc(), DC);
   }
+
   // C++23 [temp.dep.general]p2:
   //   The component name of an unqualified-id is dependent if
   //   - it is a conversion-function-id whose conversion-type-id
@@ -1294,20 +1295,6 @@ bool Sema::CppLookupName(LookupResult &R, Scope *S) {
     return false;
   }
 
-  // If this is the name of an implicitly-declared special member function,
-  // go through the scope stack to implicitly declare
-  if (isImplicitlyDeclaredMemberFunctionName(Name)) {
-    for (Scope *PreS = S; PreS; PreS = PreS->getParent())
-      if (DeclContext *DC = PreS->getEntity()) {
-        if (DC->isDependentContext() && isa<CXXRecordDecl>(DC) &&
-            Name.getCXXOverloadedOperator() == OO_Equal) {
-          R.setNotFoundInCurrentInstantiation();
-          return false;
-        }
-        DeclareImplicitMemberFunctionsWithName(*this, Name, R.getNameLoc(), DC);
-      }
-  }
-
   // Implicitly declare member functions with the name we're looking for, if in
   // fact we are in a scope where it matters.
 
@@ -2485,10 +2472,8 @@ bool Sema::LookupQualifiedName(LookupResult &R, DeclContext *LookupCtx,
     //     is operator=, or
     //   - [...]
     if (DeclarationName Name = R.getLookupName();
-        (Name.getNameKind() == DeclarationName::CXXConversionFunctionName &&
-         Name.getCXXNameType()->isDependentType()) ||
-        (Name.getCXXOverloadedOperator() == OO_Equal && LookupRec &&
-         LookupRec->isDependentContext())) {
+        Name.getNameKind() == DeclarationName::CXXConversionFunctionName &&
+        Name.getCXXNameType()->isDependentType()) {
       R.setNotFoundInCurrentInstantiation();
       return false;
     }
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 7e57fa0696725..2fce2238f9c10 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -750,7 +750,8 @@ Sema::ActOnDependentIdExpression(const CXXScopeSpec &SS,
   if (!MightBeCxx11UnevalField && !isAddressOfOperand && !IsEnum &&
       isa<CXXMethodDecl>(DC) &&
       cast<CXXMethodDecl>(DC)->isImplicitObjectMemberFunction()) {
-    QualType ThisType = cast<CXXMethodDecl>(DC)->getThisType().getNonReferenceType();
+    QualType ThisType =
+        cast<CXXMethodDecl>(DC)->getThisType().getNonReferenceType();
 
     // Since the 'this' expression is synthesized, we don't need to
     // perform the double-lookup check.
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 0f24d716a7b74..095bef80f30a4 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
@@ -453,6 +453,36 @@ namespace N3 {
       this->A::operator=(*this);
     }
   };
+
+  template<typename T>
+  struct C {
+    template<typename U>
+    void operator=(int);
+
+    void not_instantiated() {
+      operator=<int>(0);
+      C::operator=<int>(0);
+      this->operator=<int>(0);
+      this->C::operator=<int>(0);
+
+      operator=(*this);
+      C::operator=(*this);
+      this->operator=(*this);
+      this->C::operator=(*this);
+    }
+  };
+
+  template<typename T>
+  struct D {
+    auto not_instantiated() -> decltype(operator=(0)); // expected-error {{use of undeclared 'operator='}}
+  };
+
+  template<typename T>
+  struct E {
+    auto instantiated(E& e) -> decltype(operator=(e)); // expected-error {{use of undeclared 'operator='}}
+  };
+
+  template struct E<int>; // expected-note {{in instantiation of template class 'N3::E<int>' requested here}}
 } // namespace N3
 
 namespace N4 {



More information about the cfe-commits mailing list