Skip to content

Commit

Permalink
[Sema] Mark ineligibility of special member functions correctly
Browse files Browse the repository at this point in the history
I checked if the member function declaration was a copy constructor, but it's not sufficient; We need to check
the arguments against the instantiated class.

Fixed #62555

Reviewed By: erichkeane

Differential Revision: https://reviews.llvm.org/D149961
  • Loading branch information
royjacobson committed May 19, 2023
1 parent b18a819 commit 0f59bee
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 6 deletions.
3 changes: 2 additions & 1 deletion clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ C++ Specific Potentially Breaking Changes

ABI Changes in This Version
---------------------------

- A bug in evaluating the ineligibility of some special member functions has been fixed. This can
make some classes trivially copyable that were not trivially copyable before. (`#62555 <https://github.com/llvm/llvm-project/issues/62555>`_)

What's New in Clang |release|?
==============================
Expand Down
21 changes: 16 additions & 5 deletions clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2524,9 +2524,6 @@ 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 @@ -2549,8 +2546,6 @@ 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 Expand Up @@ -2753,6 +2748,22 @@ Decl *TemplateDeclInstantiator::VisitCXXMethodDecl(
if (IsExplicitSpecialization && !isFriend)
SemaRef.CompleteMemberSpecialization(Method, Previous);

// If the method is a special member function, we need to mark it as
// ineligible so that Owner->addDecl() won't mark the class as non trivial.
// At the end of the class instantiation, we calculate eligibility again and
// then we adjust trivility if needed.
// We need this check to happen only after the method parameters are set,
// because being e.g. a copy constructor depends on the instantiated
// arguments.
if (auto *Constructor = dyn_cast<CXXConstructorDecl>(Method)) {
if (Constructor->isDefaultConstructor() ||
Constructor->isCopyOrMoveConstructor())
Method->setIneligibleOrNotSelected(true);
} else if (Method->isCopyAssignmentOperator() ||
Method->isMoveAssignmentOperator()) {
Method->setIneligibleOrNotSelected(true);
}

// If there's a function template, let our caller handle it.
if (FunctionTemplate) {
// do nothing
Expand Down
13 changes: 13 additions & 0 deletions clang/test/SemaCXX/constrained-special-member-functions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -299,3 +299,16 @@ static_assert(__is_trivial(S<int>));
static_assert(!__is_trivial(S<D>));

}

namespace GH62555 {

template <bool B>
struct ExplicitTemplateArgs {
ExplicitTemplateArgs(ExplicitTemplateArgs&&) = default;
ExplicitTemplateArgs(ExplicitTemplateArgs<false>&&) requires B {};
};

static_assert(__is_trivially_copyable(ExplicitTemplateArgs<false>));
static_assert(__is_trivially_copyable(ExplicitTemplateArgs<true>));

}

0 comments on commit 0f59bee

Please sign in to comment.