[clang] c7fbe21 - Fix rejects-valid when referencing an implicit operator== from within a
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Wed Jun 17 15:12:15 PDT 2020
Author: Richard Smith
Date: 2020-06-17T15:12:04-07:00
New Revision: c7fbe2191b8b9cd64570ed3906d1bed1dd5fff8e
URL: https://github.com/llvm/llvm-project/commit/c7fbe2191b8b9cd64570ed3906d1bed1dd5fff8e
DIFF: https://github.com/llvm/llvm-project/commit/c7fbe2191b8b9cd64570ed3906d1bed1dd5fff8e.diff
LOG: Fix rejects-valid when referencing an implicit operator== from within a
templated class.
When a defaulted operator<=> results in the injection of a defaulted
operator==, that operator== can be named by unqualified name within the
same class, even if the class is templated. To make this work, perform
the transform from defaulted operator<=> to defaulted operator== in the
template definition context instead of the template instantiation
context.
This results in our substituting into a declaration from a context where
we don't have a full list of template arguments (or indeed any), for
which we are now more careful to not spuriously instantiate declarations
that are not dependent on the arguments we're substituting.
Added:
clang/test/SemaTemplate/defaulted.cpp
Modified:
clang/include/clang/AST/DeclBase.h
clang/include/clang/Sema/Template.h
clang/lib/AST/DeclBase.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/test/PCH/cxx2a-defaulted-comparison.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h
index ba84f27617f5..5e00be35d8ce 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -66,6 +66,7 @@ class SourceManager;
class Stmt;
class StoredDeclsMap;
class TemplateDecl;
+class TemplateParameterList;
class TranslationUnitDecl;
class UsingDirectiveDecl;
@@ -862,6 +863,10 @@ class alignas(8) Decl {
// within the scope of a template parameter).
bool isTemplated() const;
+ /// Determine the number of levels of template parameter surrounding this
+ /// declaration.
+ unsigned getTemplateDepth() const;
+
/// isDefinedOutsideFunctionOrMethod - This predicate returns true if this
/// scoped decl is defined outside the current function or method. This is
/// roughly global variables and functions, but also handles enums (which
@@ -1038,8 +1043,16 @@ class alignas(8) Decl {
/// If this is a declaration that describes some template, this
/// method returns that template declaration.
+ ///
+ /// Note that this returns nullptr for partial specializations, because they
+ /// are not modeled as TemplateDecls. Use getDescribedTemplateParams to handle
+ /// those cases.
TemplateDecl *getDescribedTemplate() const;
+ /// If this is a declaration that describes some template or partial
+ /// specialization, this returns the corresponding template parameter list.
+ const TemplateParameterList *getDescribedTemplateParams() const;
+
/// Returns the function itself, or the templated function if this is a
/// function template.
FunctionDecl *getAsFunction() LLVM_READONLY;
diff --git a/clang/include/clang/Sema/Template.h b/clang/include/clang/Sema/Template.h
index 8166b6cf9b6f..d05c3a55f89c 100644
--- a/clang/include/clang/Sema/Template.h
+++ b/clang/include/clang/Sema/Template.h
@@ -95,6 +95,10 @@ class VarDecl;
return TemplateArgumentLists.size();
}
+ unsigned getNumRetainedOuterLevels() const {
+ return NumRetainedOuterLevels;
+ }
+
/// Determine how many of the \p OldDepth outermost template parameter
/// lists would be removed by substituting these arguments.
unsigned getNewDepth(unsigned OldDepth) const {
@@ -159,6 +163,9 @@ class VarDecl;
void addOuterRetainedLevel() {
++NumRetainedOuterLevels;
}
+ void addOuterRetainedLevels(unsigned Num) {
+ NumRetainedOuterLevels += Num;
+ }
/// Retrieve the innermost template argument list.
const ArgList &getInnermost() const {
diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index 2aab53f4fa90..a9cd7918da9b 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -240,6 +240,16 @@ TemplateDecl *Decl::getDescribedTemplate() const {
return nullptr;
}
+const TemplateParameterList *Decl::getDescribedTemplateParams() const {
+ if (auto *TD = getDescribedTemplate())
+ return TD->getTemplateParameters();
+ if (auto *CTPSD = dyn_cast<ClassTemplatePartialSpecializationDecl>(this))
+ return CTPSD->getTemplateParameters();
+ if (auto *VTPSD = dyn_cast<VarTemplatePartialSpecializationDecl>(this))
+ return VTPSD->getTemplateParameters();
+ return nullptr;
+}
+
bool Decl::isTemplated() const {
// A declaration is dependent if it is a template or a template pattern, or
// is within (lexcially for a friend, semantically otherwise) a dependent
@@ -248,7 +258,29 @@ bool Decl::isTemplated() const {
if (auto *AsDC = dyn_cast<DeclContext>(this))
return AsDC->isDependentContext();
auto *DC = getFriendObjectKind() ? getLexicalDeclContext() : getDeclContext();
- return DC->isDependentContext() || isTemplateDecl() || getDescribedTemplate();
+ return DC->isDependentContext() || isTemplateDecl() ||
+ getDescribedTemplateParams();
+}
+
+unsigned Decl::getTemplateDepth() const {
+ if (auto *DC = dyn_cast<DeclContext>(this))
+ if (DC->isFileContext())
+ return 0;
+
+ if (auto *TPL = getDescribedTemplateParams())
+ return TPL->getDepth() + 1;
+
+ // If this is a dependent lambda, there might be an enclosing variable
+ // template. In this case, the next step is not the parent DeclContext (or
+ // even a DeclContext at all).
+ auto *RD = dyn_cast<CXXRecordDecl>(this);
+ if (RD && RD->isDependentLambda())
+ if (Decl *Context = RD->getLambdaContextDecl())
+ return Context->getTemplateDepth();
+
+ const DeclContext *DC =
+ getFriendObjectKind() ? getLexicalDeclContext() : getDeclContext();
+ return cast<Decl>(DC)->getTemplateDepth();
}
const DeclContext *Decl::getParentFunctionOrMethod() const {
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 80469e3bedbe..f1da788b1209 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -17170,10 +17170,10 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc, Decl *EnclosingDecl,
I.setAccess((*I)->getAccess());
}
- if (!CXXRecord->isDependentType()) {
- // Add any implicitly-declared members to this class.
- AddImplicitlyDeclaredMembersToClass(CXXRecord);
+ // Add any implicitly-declared members to this class.
+ AddImplicitlyDeclaredMembersToClass(CXXRecord);
+ if (!CXXRecord->isDependentType()) {
if (!CXXRecord->isInvalidDecl()) {
// If we have virtual base classes, we may end up finding multiple
// final overriders for a given virtual function. Check for this
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index a2b669c99125..e38ea6d2bc08 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -9839,86 +9839,95 @@ static void findImplicitlyDeclaredEqualityComparisons(
/// [special]p1). This routine can only be executed just before the
/// definition of the class is complete.
void Sema::AddImplicitlyDeclaredMembersToClass(CXXRecordDecl *ClassDecl) {
- if (ClassDecl->needsImplicitDefaultConstructor()) {
- ++getASTContext().NumImplicitDefaultConstructors;
+ // Don't add implicit special members to templated classes.
+ // FIXME: This means unqualified lookups for 'operator=' within a class
+ // template don't work properly.
+ if (!ClassDecl->isDependentType()) {
+ if (ClassDecl->needsImplicitDefaultConstructor()) {
+ ++getASTContext().NumImplicitDefaultConstructors;
- if (ClassDecl->hasInheritedConstructor())
- DeclareImplicitDefaultConstructor(ClassDecl);
- }
+ if (ClassDecl->hasInheritedConstructor())
+ DeclareImplicitDefaultConstructor(ClassDecl);
+ }
- if (ClassDecl->needsImplicitCopyConstructor()) {
- ++getASTContext().NumImplicitCopyConstructors;
+ if (ClassDecl->needsImplicitCopyConstructor()) {
+ ++getASTContext().NumImplicitCopyConstructors;
- // If the properties or semantics of the copy constructor couldn't be
- // determined while the class was being declared, force a declaration
- // of it now.
- if (ClassDecl->needsOverloadResolutionForCopyConstructor() ||
- ClassDecl->hasInheritedConstructor())
- DeclareImplicitCopyConstructor(ClassDecl);
- // For the MS ABI we need to know whether the copy ctor is deleted. A
- // prerequisite for deleting the implicit copy ctor is that the class has a
- // move ctor or move assignment that is either user-declared or whose
- // semantics are inherited from a subobject. FIXME: We should provide a more
- // direct way for CodeGen to ask whether the constructor was deleted.
- else if (Context.getTargetInfo().getCXXABI().isMicrosoft() &&
- (ClassDecl->hasUserDeclaredMoveConstructor() ||
- ClassDecl->needsOverloadResolutionForMoveConstructor() ||
- ClassDecl->hasUserDeclaredMoveAssignment() ||
- ClassDecl->needsOverloadResolutionForMoveAssignment()))
- DeclareImplicitCopyConstructor(ClassDecl);
- }
+ // If the properties or semantics of the copy constructor couldn't be
+ // determined while the class was being declared, force a declaration
+ // of it now.
+ if (ClassDecl->needsOverloadResolutionForCopyConstructor() ||
+ ClassDecl->hasInheritedConstructor())
+ DeclareImplicitCopyConstructor(ClassDecl);
+ // For the MS ABI we need to know whether the copy ctor is deleted. A
+ // prerequisite for deleting the implicit copy ctor is that the class has
+ // a move ctor or move assignment that is either user-declared or whose
+ // semantics are inherited from a subobject. FIXME: We should provide a
+ // more direct way for CodeGen to ask whether the constructor was deleted.
+ else if (Context.getTargetInfo().getCXXABI().isMicrosoft() &&
+ (ClassDecl->hasUserDeclaredMoveConstructor() ||
+ ClassDecl->needsOverloadResolutionForMoveConstructor() ||
+ ClassDecl->hasUserDeclaredMoveAssignment() ||
+ ClassDecl->needsOverloadResolutionForMoveAssignment()))
+ DeclareImplicitCopyConstructor(ClassDecl);
+ }
- if (getLangOpts().CPlusPlus11 && ClassDecl->needsImplicitMoveConstructor()) {
- ++getASTContext().NumImplicitMoveConstructors;
+ if (getLangOpts().CPlusPlus11 &&
+ ClassDecl->needsImplicitMoveConstructor()) {
+ ++getASTContext().NumImplicitMoveConstructors;
- if (ClassDecl->needsOverloadResolutionForMoveConstructor() ||
- ClassDecl->hasInheritedConstructor())
- DeclareImplicitMoveConstructor(ClassDecl);
- }
+ if (ClassDecl->needsOverloadResolutionForMoveConstructor() ||
+ ClassDecl->hasInheritedConstructor())
+ DeclareImplicitMoveConstructor(ClassDecl);
+ }
- if (ClassDecl->needsImplicitCopyAssignment()) {
- ++getASTContext().NumImplicitCopyAssignmentOperators;
+ if (ClassDecl->needsImplicitCopyAssignment()) {
+ ++getASTContext().NumImplicitCopyAssignmentOperators;
- // If we have a dynamic class, then the copy assignment operator may be
- // virtual, so we have to declare it immediately. This ensures that, e.g.,
- // it shows up in the right place in the vtable and that we diagnose
- // problems with the implicit exception specification.
- if (ClassDecl->isDynamicClass() ||
- ClassDecl->needsOverloadResolutionForCopyAssignment() ||
- ClassDecl->hasInheritedAssignment())
- DeclareImplicitCopyAssignment(ClassDecl);
- }
+ // If we have a dynamic class, then the copy assignment operator may be
+ // virtual, so we have to declare it immediately. This ensures that, e.g.,
+ // it shows up in the right place in the vtable and that we diagnose
+ // problems with the implicit exception specification.
+ if (ClassDecl->isDynamicClass() ||
+ ClassDecl->needsOverloadResolutionForCopyAssignment() ||
+ ClassDecl->hasInheritedAssignment())
+ DeclareImplicitCopyAssignment(ClassDecl);
+ }
- if (getLangOpts().CPlusPlus11 && ClassDecl->needsImplicitMoveAssignment()) {
- ++getASTContext().NumImplicitMoveAssignmentOperators;
+ if (getLangOpts().CPlusPlus11 && ClassDecl->needsImplicitMoveAssignment()) {
+ ++getASTContext().NumImplicitMoveAssignmentOperators;
- // Likewise for the move assignment operator.
- if (ClassDecl->isDynamicClass() ||
- ClassDecl->needsOverloadResolutionForMoveAssignment() ||
- ClassDecl->hasInheritedAssignment())
- DeclareImplicitMoveAssignment(ClassDecl);
- }
+ // Likewise for the move assignment operator.
+ if (ClassDecl->isDynamicClass() ||
+ ClassDecl->needsOverloadResolutionForMoveAssignment() ||
+ ClassDecl->hasInheritedAssignment())
+ DeclareImplicitMoveAssignment(ClassDecl);
+ }
- if (ClassDecl->needsImplicitDestructor()) {
- ++getASTContext().NumImplicitDestructors;
+ if (ClassDecl->needsImplicitDestructor()) {
+ ++getASTContext().NumImplicitDestructors;
- // If we have a dynamic class, then the destructor may be virtual, so we
- // have to declare the destructor immediately. This ensures that, e.g., it
- // shows up in the right place in the vtable and that we diagnose problems
- // with the implicit exception specification.
- if (ClassDecl->isDynamicClass() ||
- ClassDecl->needsOverloadResolutionForDestructor())
- DeclareImplicitDestructor(ClassDecl);
+ // If we have a dynamic class, then the destructor may be virtual, so we
+ // have to declare the destructor immediately. This ensures that, e.g., it
+ // shows up in the right place in the vtable and that we diagnose problems
+ // with the implicit exception specification.
+ if (ClassDecl->isDynamicClass() ||
+ ClassDecl->needsOverloadResolutionForDestructor())
+ DeclareImplicitDestructor(ClassDecl);
+ }
}
// C++2a [class.compare.default]p3:
// If the member-specification does not explicitly declare any member or
// friend named operator==, an == operator function is declared implicitly
- // for each defaulted three-way comparison operator function defined in the
- // member-specification
+ // for each defaulted three-way comparison operator function defined in
+ // the member-specification
// FIXME: Consider doing this lazily.
- if (getLangOpts().CPlusPlus20) {
- llvm::SmallVector<FunctionDecl*, 4> DefaultedSpaceships;
+ // We do this during the initial parse for a class template, not during
+ // instantiation, so that we can handle unqualified lookups for 'operator=='
+ // when parsing the template.
+ if (getLangOpts().CPlusPlus20 && !inTemplateInstantiation()) {
+ llvm::SmallVector<FunctionDecl *, 4> DefaultedSpaceships;
findImplicitlyDeclaredEqualityComparisons(Context, ClassDecl,
DefaultedSpaceships);
for (auto *FD : DefaultedSpaceships)
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 7d0777cce6ae..ee91c51e2046 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -3719,6 +3719,7 @@ FunctionDecl *Sema::SubstSpaceshipAsEqualEqual(CXXRecordDecl *RD,
// access and function-definition and in the same class scope as the
// three-way comparison operator function
MultiLevelTemplateArgumentList NoTemplateArgs;
+ NoTemplateArgs.addOuterRetainedLevels(RD->getTemplateDepth());
TemplateDeclInstantiator Instantiator(*this, RD, NoTemplateArgs);
Decl *R;
if (auto *MD = dyn_cast<CXXMethodDecl>(Spaceship)) {
@@ -5605,6 +5606,20 @@ DeclContext *Sema::FindInstantiatedContext(SourceLocation Loc, DeclContext* DC,
} else return DC;
}
+/// Determine whether the given context is dependent on template parameters at
+/// level \p Level or below.
+///
+/// Sometimes we only substitute an inner set of template arguments and leave
+/// the outer templates alone. In such cases, contexts dependent only on the
+/// outer levels are not effectively dependent.
+static bool isDependentContextAtLevel(DeclContext *DC, unsigned Level) {
+ if (!DC->isDependentContext())
+ return false;
+ if (!Level)
+ return true;
+ return cast<Decl>(DC)->getTemplateDepth() > Level;
+}
+
/// Find the instantiation of the given declaration within the
/// current instantiation.
///
@@ -5635,6 +5650,10 @@ NamedDecl *Sema::FindInstantiatedDecl(SourceLocation Loc, NamedDecl *D,
const MultiLevelTemplateArgumentList &TemplateArgs,
bool FindingInstantiatedContext) {
DeclContext *ParentDC = D->getDeclContext();
+ // Determine whether our parent context depends on any of the tempalte
+ // arguments we're currently substituting.
+ bool ParentDependsOnArgs = isDependentContextAtLevel(
+ ParentDC, TemplateArgs.getNumRetainedOuterLevels());
// FIXME: Parmeters of pointer to functions (y below) that are themselves
// parameters (p below) can have their ParentDC set to the translation-unit
// - thus we can not consistently check if the ParentDC of such a parameter
@@ -5651,15 +5670,14 @@ NamedDecl *Sema::FindInstantiatedDecl(SourceLocation Loc, NamedDecl *D,
// - as long as we have a ParmVarDecl whose parent is non-dependent and
// whose type is not instantiation dependent, do nothing to the decl
// - otherwise find its instantiated decl.
- if (isa<ParmVarDecl>(D) && !ParentDC->isDependentContext() &&
+ if (isa<ParmVarDecl>(D) && !ParentDependsOnArgs &&
!cast<ParmVarDecl>(D)->getType()->isInstantiationDependentType())
return D;
if (isa<ParmVarDecl>(D) || isa<NonTypeTemplateParmDecl>(D) ||
isa<TemplateTypeParmDecl>(D) || isa<TemplateTemplateParmDecl>(D) ||
- ((ParentDC->isFunctionOrMethod() ||
- isa<OMPDeclareReductionDecl>(ParentDC) ||
- isa<OMPDeclareMapperDecl>(ParentDC)) &&
- ParentDC->isDependentContext()) ||
+ (ParentDependsOnArgs && (ParentDC->isFunctionOrMethod() ||
+ isa<OMPDeclareReductionDecl>(ParentDC) ||
+ isa<OMPDeclareMapperDecl>(ParentDC))) ||
(isa<CXXRecordDecl>(D) && cast<CXXRecordDecl>(D)->isLambda())) {
// D is a local of some kind. Look into the map of local
// declarations to their instantiations.
@@ -5813,7 +5831,7 @@ NamedDecl *Sema::FindInstantiatedDecl(SourceLocation Loc, NamedDecl *D,
// anonymous unions in class templates).
}
- if (!ParentDC->isDependentContext())
+ if (!ParentDependsOnArgs)
return D;
ParentDC = FindInstantiatedContext(Loc, ParentDC, TemplateArgs);
diff --git a/clang/test/PCH/cxx2a-defaulted-comparison.cpp b/clang/test/PCH/cxx2a-defaulted-comparison.cpp
index 4fb0b83b2648..d771424ad4fb 100644
--- a/clang/test/PCH/cxx2a-defaulted-comparison.cpp
+++ b/clang/test/PCH/cxx2a-defaulted-comparison.cpp
@@ -1,4 +1,4 @@
-// RxN: %clang_cc1 -std=c++2a -verify -Wno-defaulted-function-deleted -include %s %s
+// RUN: %clang_cc1 -std=c++2a -verify -Wno-defaulted-function-deleted -include %s %s
//
// RUN: %clang_cc1 -std=c++2a -emit-pch %s -o %t.pch
// RUN: %clang_cc1 -std=c++2a -include-pch %t.pch %s -verify
diff --git a/clang/test/SemaTemplate/defaulted.cpp b/clang/test/SemaTemplate/defaulted.cpp
new file mode 100644
index 000000000000..15111f71202d
--- /dev/null
+++ b/clang/test/SemaTemplate/defaulted.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+// expected-no-diagnostics
+
+namespace SpaceshipImpliesEq {
+ template<typename T> struct A {
+ int operator<=>(const A&) const = default;
+ constexpr bool f() { return operator==(*this); }
+ };
+ static_assert(A<int>().f());
+}
More information about the cfe-commits
mailing list