-
Notifications
You must be signed in to change notification settings - Fork 11k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Clang][Sema] Revert changes to operator= lookup in templated classes from #91498, #90999, and #90152 #91620
Conversation
…mplete-class contexts (llvm#91498)" This reverts commit 62b5b61.
@llvm/pr-subscribers-clang Author: Krystian Stasiowski (sdkrystian) ChangesThis reverts commit 62b5b61 (#91498). Full diff: https://github.com/llvm/llvm-project/pull/91620.diff 3 Files Affected:
diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index e20de338ebb1..e63da5875d2c 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 480bc74c2001..7e57fa069672 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 43053c18c507..46dd52f8c4c1 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 {
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This reverts the problematic parts of the previous patch too, right? If so, LGTM.
✅ With the latest revision this PR passed the C/C++ code formatter. |
)" This reverts commit 3191e0b.
should be fine to revert as much as you want in a single PR, just make sure to mention what you're reverting in the description |
Agreed, a revert doesnt have to be a single commit, or even a whole one. Just enough to get us back to a 'working' state is fine. Though if you do 'partial' ones, I prefer to not REMOVE tests, but to put a FIXME on the tests you've reverted and fix their test lines (assuming they dont' crash) |
This reverts changes in #91498, #90999, and #90152 which make
operator=
dependent wherever the current class is templated.