Skip to content

Commit

Permalink
[Clang] Implement P0848 (Conditionally Trivial Special Member Functions)
Browse files Browse the repository at this point in the history
This patch implements P0848 in Clang.

During the instantiation of a C++ class, in `Sema::ActOnFields`, we evaluate constraints for all the SMFs and compare the constraints to compute the eligibility. We defer the computation of the type's [copy-]trivial bits from addedMember to the eligibility computation, like we did for destructors in D126194. `canPassInRegisters` is modified as well to better respect the ineligibility of functions.

Note: Because of the non-implementation of DR1734 and DR1496, I treat deleted member functions as 'eligible' for the purpose of [copy-]triviallity. This is unfortunate, but I couldn't think of a way to make this make sense otherwise.

Reviewed By: #clang-language-wg, cor3ntin, aaron.ballman

Differential Revision: https://reviews.llvm.org/D128619
  • Loading branch information
royjacobson authored and finomen committed Nov 3, 2022
1 parent 4987869 commit 522fc53
Show file tree
Hide file tree
Showing 10 changed files with 773 additions and 18 deletions.
18 changes: 18 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,24 @@ C++20 Feature Support
now possible to overload destructors using concepts. Note that the rest
of the paper about other special member functions is not yet implemented.

- Support capturing structured bindings in lambdas
(`P1091R3 <https://wg21.link/p1091r3>`_ and `P1381R1 <https://wg21.link/P1381R1>`).
This fixes issues `GH52720 <https://github.com/llvm/llvm-project/issues/52720>`_,
`GH54300 <https://github.com/llvm/llvm-project/issues/54300>`_,
`GH54301 <https://github.com/llvm/llvm-project/issues/54301>`_,
and `GH49430 <https://github.com/llvm/llvm-project/issues/49430>`_.
- Consider explicitly defaulted constexpr/consteval special member function
template instantiation to be constexpr/consteval even though a call to such
a function cannot appear in a constant expression.
(C++14 [dcl.constexpr]p6 (CWG DR647/CWG DR1358))
- Correctly defer dependent immediate function invocations until template instantiation.
This fixes `GH55601 <https://github.com/llvm/llvm-project/issues/55601>`_.
- Implemented "Conditionally Trivial Special Member Functions" (`P0848 <https://wg21.link/p0848r3>`_).
Note: The handling of deleted functions is not yet compliant, as Clang
does not implement `DR1496 <https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1496>`_
and `DR1734 <https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1734>`_.


C++2b Feature Support
^^^^^^^^^^^^^^^^^^^^^
- Implemented `P1938R3: if consteval <https://wg21.link/P1938R3>`_.
Expand Down
18 changes: 11 additions & 7 deletions clang/lib/AST/DeclCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -894,9 +894,11 @@ void CXXRecordDecl::addedMember(Decl *D) {
// This is an extension in C++03.
data().PlainOldData = false;
}
// We delay updating destructor relevant properties until
// addedSelectedDestructor.
// FIXME: Defer this for the other special member functions as well.
// When instantiating a class, we delay updating the destructor and
// triviality properties of the class until selecting a destructor and
// computing the eligibility of its special member functions. This is
// because there might be function constraints that we need to evaluate
// and compare later in the instantiation.
if (!Method->isIneligibleOrNotSelected()) {
addedEligibleSpecialMemberFunction(Method, SMKind);
}
Expand Down Expand Up @@ -1437,10 +1439,12 @@ void CXXRecordDecl::finishedDefaultedOrDeletedMember(CXXMethodDecl *D) {

// Update which trivial / non-trivial special members we have.
// addedMember will have skipped this step for this member.
if (D->isTrivial())
data().HasTrivialSpecialMembers |= SMKind;
else
data().DeclaredNonTrivialSpecialMembers |= SMKind;
if (!D->isIneligibleOrNotSelected()) {
if (D->isTrivial())
data().HasTrivialSpecialMembers |= SMKind;
else
data().DeclaredNonTrivialSpecialMembers |= SMKind;
}
}

void CXXRecordDecl::setCaptures(ASTContext &Context,
Expand Down
6 changes: 5 additions & 1 deletion clang/lib/Frontend/InitPreprocessor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -625,7 +625,11 @@ static void InitializeCPlusPlusFeatureTestMacros(const LangOptions &LangOpts,

// C++20 features.
if (LangOpts.CPlusPlus20) {
//Builder.defineMacro("__cpp_aggregate_paren_init", "201902L");
// Builder.defineMacro("__cpp_aggregate_paren_init", "201902L");

// P0848 is implemented, but we're still waiting for other concepts
// issues to be addressed before bumping __cpp_concepts up to 202002L.
// Refer to the discussion of this at https://reviews.llvm.org/D128619.
Builder.defineMacro("__cpp_concepts", "201907L");
Builder.defineMacro("__cpp_conditional_explicit", "201806L");
//Builder.defineMacro("__cpp_consteval", "201811L");
Expand Down
138 changes: 132 additions & 6 deletions clang/lib/Sema/SemaDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17456,7 +17456,6 @@ void Sema::ActOnLastBitfield(SourceLocation DeclLoc,
AllIvarDecls.push_back(Ivar);
}

namespace {
/// [class.dtor]p4:
/// At the end of the definition of a class, overload resolution is
/// performed among the prospective destructors declared in that class with
Expand All @@ -17465,7 +17464,7 @@ namespace {
///
/// We do the overload resolution here, then mark the selected constructor in the AST.
/// Later CXXRecordDecl::getDestructor() will return the selected constructor.
void ComputeSelectedDestructor(Sema &S, CXXRecordDecl *Record) {
static void ComputeSelectedDestructor(Sema &S, CXXRecordDecl *Record) {
if (!Record->hasUserDeclaredDestructor()) {
return;
}
Expand Down Expand Up @@ -17523,7 +17522,135 @@ void ComputeSelectedDestructor(Sema &S, CXXRecordDecl *Record) {
Record->addedSelectedDestructor(dyn_cast<CXXDestructorDecl>(OCS.begin()->Function));
}
}
} // namespace

/// [class.mem.special]p5
/// Two special member functions are of the same kind if:
/// - they are both default constructors,
/// - they are both copy or move constructors with the same first parameter
/// type, or
/// - they are both copy or move assignment operators with the same first
/// parameter type and the same cv-qualifiers and ref-qualifier, if any.
static bool AreSpecialMemberFunctionsSameKind(ASTContext &Context,
CXXMethodDecl *M1,
CXXMethodDecl *M2,
Sema::CXXSpecialMember CSM) {
if (CSM == Sema::CXXDefaultConstructor)
return true;
if (!Context.hasSameType(M1->getParamDecl(0)->getType(),
M2->getParamDecl(0)->getType()))
return false;
if (!Context.hasSameType(M1->getThisType(), M2->getThisType()))
return false;

return true;
}

/// [class.mem.special]p6:
/// An eligible special member function is a special member function for which:
/// - the function is not deleted,
/// - the associated constraints, if any, are satisfied, and
/// - no special member function of the same kind whose associated constraints
/// [CWG2595], if any, are satisfied is more constrained.
static void SetEligibleMethods(Sema &S, CXXRecordDecl *Record,
ArrayRef<CXXMethodDecl *> Methods,
Sema::CXXSpecialMember CSM) {
SmallVector<bool, 4> SatisfactionStatus;

for (CXXMethodDecl *Method : Methods) {
const Expr *Constraints = Method->getTrailingRequiresClause();
if (!Constraints)
SatisfactionStatus.push_back(true);
else {
ConstraintSatisfaction Satisfaction;
if (S.CheckFunctionConstraints(Method, Satisfaction))
SatisfactionStatus.push_back(false);
else
SatisfactionStatus.push_back(Satisfaction.IsSatisfied);
}
}

for (size_t i = 0; i < Methods.size(); i++) {
if (!SatisfactionStatus[i])
continue;
CXXMethodDecl *Method = Methods[i];
const Expr *Constraints = Method->getTrailingRequiresClause();
bool AnotherMethodIsMoreConstrained = false;
for (size_t j = 0; j < Methods.size(); j++) {
if (i == j || !SatisfactionStatus[j])
continue;
CXXMethodDecl *OtherMethod = Methods[j];
if (!AreSpecialMemberFunctionsSameKind(S.Context, Method, OtherMethod,
CSM))
continue;

const Expr *OtherConstraints = OtherMethod->getTrailingRequiresClause();
if (!OtherConstraints)
continue;
if (!Constraints) {
AnotherMethodIsMoreConstrained = true;
break;
}
if (S.IsAtLeastAsConstrained(OtherMethod, {OtherConstraints}, Method,
{Constraints},
AnotherMethodIsMoreConstrained)) {
// There was an error with the constraints comparison. Exit the loop
// and don't consider this function eligible.
AnotherMethodIsMoreConstrained = true;
}
if (AnotherMethodIsMoreConstrained)
break;
}
// FIXME: Do not consider deleted methods as eligible after implementing
// DR1734 and DR1496.
if (!AnotherMethodIsMoreConstrained) {
Method->setIneligibleOrNotSelected(false);
Record->addedEligibleSpecialMemberFunction(Method, 1 << CSM);
}
}
}

static void ComputeSpecialMemberFunctionsEligiblity(Sema &S,
CXXRecordDecl *Record) {
SmallVector<CXXMethodDecl *, 4> DefaultConstructors;
SmallVector<CXXMethodDecl *, 4> CopyConstructors;
SmallVector<CXXMethodDecl *, 4> MoveConstructors;
SmallVector<CXXMethodDecl *, 4> CopyAssignmentOperators;
SmallVector<CXXMethodDecl *, 4> MoveAssignmentOperators;

for (auto *Decl : Record->decls()) {
auto *MD = dyn_cast<CXXMethodDecl>(Decl);
if (!MD) {
auto *FTD = dyn_cast<FunctionTemplateDecl>(Decl);
if (FTD)
MD = dyn_cast<CXXMethodDecl>(FTD->getTemplatedDecl());
}
if (!MD)
continue;
if (auto *CD = dyn_cast<CXXConstructorDecl>(MD)) {
if (CD->isInvalidDecl())
continue;
if (CD->isDefaultConstructor())
DefaultConstructors.push_back(MD);
else if (CD->isCopyConstructor())
CopyConstructors.push_back(MD);
else if (CD->isMoveConstructor())
MoveConstructors.push_back(MD);
} else if (MD->isCopyAssignmentOperator()) {
CopyAssignmentOperators.push_back(MD);
} else if (MD->isMoveAssignmentOperator()) {
MoveAssignmentOperators.push_back(MD);
}
}

SetEligibleMethods(S, Record, DefaultConstructors,
Sema::CXXDefaultConstructor);
SetEligibleMethods(S, Record, CopyConstructors, Sema::CXXCopyConstructor);
SetEligibleMethods(S, Record, MoveConstructors, Sema::CXXMoveConstructor);
SetEligibleMethods(S, Record, CopyAssignmentOperators,
Sema::CXXCopyAssignment);
SetEligibleMethods(S, Record, MoveAssignmentOperators,
Sema::CXXMoveAssignment);
}

void Sema::ActOnFields(Scope *S, SourceLocation RecLoc, Decl *EnclosingDecl,
ArrayRef<Decl *> Fields, SourceLocation LBrac,
Expand Down Expand Up @@ -17551,9 +17678,6 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc, Decl *EnclosingDecl,
RecordDecl *Record = dyn_cast<RecordDecl>(EnclosingDecl);
CXXRecordDecl *CXXRecord = dyn_cast<CXXRecordDecl>(EnclosingDecl);

if (CXXRecord && !CXXRecord->isDependentType())
ComputeSelectedDestructor(*this, CXXRecord);

// Start counting up the number of named members; make sure to include
// members of anonymous structs and unions in the total.
unsigned NumNamedMembers = 0;
Expand Down Expand Up @@ -17839,6 +17963,8 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc, Decl *EnclosingDecl,
Completed = true;
}
}
ComputeSelectedDestructor(*this, CXXRecord);
ComputeSpecialMemberFunctionsEligiblity(*this, CXXRecord);
}
}

Expand Down
5 changes: 3 additions & 2 deletions clang/lib/Sema/SemaDeclCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6559,7 +6559,7 @@ static bool canPassInRegisters(Sema &S, CXXRecordDecl *D,
bool CopyCtorIsTrivial = false, CopyCtorIsTrivialForCall = false;
bool DtorIsTrivialForCall = false;

// If a class has at least one non-deleted, trivial copy constructor, it
// If a class has at least one eligible, trivial copy constructor, it
// is passed according to the C ABI. Otherwise, it is passed indirectly.
//
// Note: This permits classes with non-trivial copy or move ctors to be
Expand All @@ -6574,7 +6574,8 @@ static bool canPassInRegisters(Sema &S, CXXRecordDecl *D,
}
} else {
for (const CXXConstructorDecl *CD : D->ctors()) {
if (CD->isCopyConstructor() && !CD->isDeleted()) {
if (CD->isCopyConstructor() && !CD->isDeleted() &&
!CD->isIneligibleOrNotSelected()) {
if (CD->isTrivial())
CopyCtorIsTrivial = true;
if (CD->isTrivialForCall())
Expand Down
5 changes: 5 additions & 0 deletions clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2420,6 +2420,9 @@ Decl *TemplateDeclInstantiator::VisitCXXMethodDecl(
Constructor->getConstexprKind(), InheritedConstructor(),
TrailingRequiresClause);
Method->setRangeEnd(Constructor->getEndLoc());
if (Constructor->isDefaultConstructor() ||
Constructor->isCopyOrMoveConstructor())
Method->setIneligibleOrNotSelected(true);
} else if (CXXDestructorDecl *Destructor = dyn_cast<CXXDestructorDecl>(D)) {
Method = CXXDestructorDecl::Create(
SemaRef.Context, Record, StartLoc, NameInfo, T, TInfo,
Expand All @@ -2442,6 +2445,8 @@ Decl *TemplateDeclInstantiator::VisitCXXMethodDecl(
SemaRef.Context, Record, StartLoc, NameInfo, T, TInfo, SC,
D->UsesFPIntrin(), D->isInlineSpecified(), D->getConstexprKind(),
D->getEndLoc(), TrailingRequiresClause);
if (D->isMoveAssignmentOperator() || D->isCopyAssignmentOperator())
Method->setIneligibleOrNotSelected(true);
}

if (D->isInlined())
Expand Down
15 changes: 15 additions & 0 deletions clang/test/AST/conditionally-trivial-smfs-2.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// RUN: %clang_cc1 -std=c++20 -triple x86_64-pc-linux %s -ast-dump | FileCheck %s
// RUN: %clang_cc1 -std=c++20 -triple x86_64-pc-win32 %s -ast-dump | FileCheck %s

template<class X>
struct DefaultConstructibleWithTemplate {
template<class T = int>
DefaultConstructibleWithTemplate();
};

void f() {
DefaultConstructibleWithTemplate<int> x;
}

// CHECK: | `-ClassTemplateSpecializationDecl {{.*}} struct DefaultConstructibleWithTemplate definition
// CHECK: | | |-CXXConstructorDecl {{.*}} DefaultConstructibleWithTemplate 'void ()'
Loading

0 comments on commit 522fc53

Please sign in to comment.