llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Corentin Jabot (cor3ntin) <details> <summary>Changes</summary> Use the definition of trivially relocatable types to short-circuit some checks for trivial_abi. Note that this is mostly a no-op as there is a lot of overlap between trivial_abi and trivial relocatability (ie, I can't envision a scenario in which there would be a trivially relocatable type that would not be eligible for trivial_abi based on its special member function... which is good!) Note that for bases and members, we need to check CanPassInRegister rather than just relocation. So we do these checks first, which leads to better diagnostics. --- Full diff: https://github.com/llvm/llvm-project/pull/143111.diff 5 Files Affected: - (modified) clang/include/clang/Sema/Sema.h (+1) - (modified) clang/lib/Sema/SemaDeclCXX.cpp (+26-29) - (modified) clang/lib/Sema/SemaTypeTraits.cpp (+6-6) - (modified) clang/test/SemaCXX/attr-trivial-abi.cpp (+11-5) - (modified) clang/test/SemaObjCXX/attr-trivial-abi.mm (+2-1) ``````````diff diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 65c84c0c40f60..88db3f5de7486 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -8680,6 +8680,7 @@ class Sema final : public SemaBase { // FIXME: This is in Sema because it requires // overload resolution, can we move to ASTContext? bool IsCXXTriviallyRelocatableType(QualType T); + bool IsCXXTriviallyRelocatableType(const CXXRecordDecl &RD); //// Determines if a type is replaceable /// according to the C++26 rules. diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 55e078f3180a2..ff7c306cd69a0 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -10572,35 +10572,6 @@ void Sema::checkIllFormedTrivialABIStruct(CXXRecordDecl &RD) { RD.dropAttr<TrivialABIAttr>(); }; - // Ill-formed if the copy and move constructors are deleted. - auto HasNonDeletedCopyOrMoveConstructor = [&]() { - // If the type is dependent, then assume it might have - // implicit copy or move ctor because we won't know yet at this point. - if (RD.isDependentType()) - return true; - if (RD.needsImplicitCopyConstructor() && - !RD.defaultedCopyConstructorIsDeleted()) - return true; - if (RD.needsImplicitMoveConstructor() && - !RD.defaultedMoveConstructorIsDeleted()) - return true; - for (const CXXConstructorDecl *CD : RD.ctors()) - if (CD->isCopyOrMoveConstructor() && !CD->isDeleted()) - return true; - return false; - }; - - if (!HasNonDeletedCopyOrMoveConstructor()) { - PrintDiagAndRemoveAttr(0); - return; - } - - // Ill-formed if the struct has virtual functions. - if (RD.isPolymorphic()) { - PrintDiagAndRemoveAttr(1); - return; - } - for (const auto &B : RD.bases()) { // Ill-formed if the base class is non-trivial for the purpose of calls or a // virtual base. @@ -10638,6 +10609,32 @@ void Sema::checkIllFormedTrivialABIStruct(CXXRecordDecl &RD) { return; } } + + if (IsCXXTriviallyRelocatableType(RD)) + return; + + // Ill-formed if the copy and move constructors are deleted. + auto HasNonDeletedCopyOrMoveConstructor = [&]() { + // If the type is dependent, then assume it might have + // implicit copy or move ctor because we won't know yet at this point. + if (RD.isDependentType()) + return true; + if (RD.needsImplicitCopyConstructor() && + !RD.defaultedCopyConstructorIsDeleted()) + return true; + if (RD.needsImplicitMoveConstructor() && + !RD.defaultedMoveConstructorIsDeleted()) + return true; + for (const CXXConstructorDecl *CD : RD.ctors()) + if (CD->isCopyOrMoveConstructor() && !CD->isDeleted()) + return true; + return false; + }; + + if (!HasNonDeletedCopyOrMoveConstructor()) { + PrintDiagAndRemoveAttr(0); + return; + } } void Sema::checkIncorrectVTablePointerAuthenticationAttribute( diff --git a/clang/lib/Sema/SemaTypeTraits.cpp b/clang/lib/Sema/SemaTypeTraits.cpp index b849bbe94af62..865a460a97458 100644 --- a/clang/lib/Sema/SemaTypeTraits.cpp +++ b/clang/lib/Sema/SemaTypeTraits.cpp @@ -295,13 +295,13 @@ Sema::CheckCXX2CRelocatableAndReplaceable(const CXXRecordDecl *D) { return Info; } -static bool IsCXXTriviallyRelocatableType(Sema &S, const CXXRecordDecl *RD) { +bool Sema::IsCXXTriviallyRelocatableType(const CXXRecordDecl &RD) { if (std::optional<ASTContext::CXXRecordDeclRelocationInfo> Info = - S.getASTContext().getRelocationInfoForCXXRecord(RD)) + getASTContext().getRelocationInfoForCXXRecord(&RD)) return Info->IsRelocatable; ASTContext::CXXRecordDeclRelocationInfo Info = - S.CheckCXX2CRelocatableAndReplaceable(RD); - S.getASTContext().setRelocationInfoForCXXRecord(RD, Info); + CheckCXX2CRelocatableAndReplaceable(&RD); + getASTContext().setRelocationInfoForCXXRecord(&RD, Info); return Info.IsRelocatable; } @@ -325,7 +325,7 @@ bool Sema::IsCXXTriviallyRelocatableType(QualType Type) { return true; if (const auto *RD = BaseElementType->getAsCXXRecordDecl()) - return ::IsCXXTriviallyRelocatableType(*this, RD); + return IsCXXTriviallyRelocatableType(*RD); return false; } @@ -667,7 +667,7 @@ static bool IsTriviallyRelocatableType(Sema &SemaRef, QualType T) { return false; if (const auto *RD = BaseElementType->getAsCXXRecordDecl(); - RD && !RD->isPolymorphic() && IsCXXTriviallyRelocatableType(SemaRef, RD)) + RD && !RD->isPolymorphic() && SemaRef.IsCXXTriviallyRelocatableType(*RD)) return true; if (const auto *RD = BaseElementType->getAsRecordDecl()) diff --git a/clang/test/SemaCXX/attr-trivial-abi.cpp b/clang/test/SemaCXX/attr-trivial-abi.cpp index 0347dc37e839b..1068b222fed20 100644 --- a/clang/test/SemaCXX/attr-trivial-abi.cpp +++ b/clang/test/SemaCXX/attr-trivial-abi.cpp @@ -1,6 +1,6 @@ -// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11 -// RUN: %clang_cc1 -fsyntax-only -verify %s -triple x86_64-windows-msvc -std=c++11 -// RUN: %clang_cc1 -fsyntax-only -verify %s -triple x86_64-scei-ps4 -std=c++11 +// RUN: %clang_cc1 -fsyntax-only -verify=expected,itanium %s -std=c++11 +// RUN: %clang_cc1 -fsyntax-only -verify=expected,windows %s -triple x86_64-windows-msvc -std=c++11 +// RUN: %clang_cc1 -fsyntax-only -verify=expected,scei %s -triple x86_64-scei-ps4 -std=c++11 void __attribute__((trivial_abi)) foo(); // expected-warning {{'trivial_abi' attribute only applies to classes}} @@ -165,7 +165,10 @@ static_assert(!__builtin_is_cpp_trivially_relocatable(CopyMoveDeleted), ""); #endif -struct __attribute__((trivial_abi)) S18 { // expected-warning {{'trivial_abi' cannot be applied to 'S18'}} expected-note {{copy constructors and move constructors are all deleted}} +struct __attribute__((trivial_abi)) S18 { // expected-warning {{'trivial_abi' cannot be applied to 'S18'}} \ + // itanium-note {{'trivial_abi' is disallowed on 'S18' because it has a field of a non-trivial class type}} \ + // windows-note {{'trivial_abi' is disallowed on 'S18' because it has a field of a non-trivial class type}} \ + // scei-note {{'trivial_abi' is disallowed on 'S18' because its copy constructors and move constructors are all deleted}} CopyMoveDeleted a; }; #ifdef __ORBIS__ @@ -195,7 +198,10 @@ struct __attribute__((trivial_abi)) MoveDeleted { }; static_assert(__is_trivially_relocatable(MoveDeleted), ""); // expected-warning{{deprecated}} static_assert(!__builtin_is_cpp_trivially_relocatable(MoveDeleted), ""); -struct __attribute__((trivial_abi)) S19 { // expected-warning {{'trivial_abi' cannot be applied to 'S19'}} expected-note {{copy constructors and move constructors are all deleted}} +struct __attribute__((trivial_abi)) S19 { // expected-warning {{'trivial_abi' cannot be applied to 'S19'}} \ + // itanium-note {{'trivial_abi' is disallowed on 'S19' because its copy constructors and move constructors are all deleted}} \ + // windows-note {{'trivial_abi' is disallowed on 'S19' because it has a field of a non-trivial class type}} \ + // scei-note {{'trivial_abi' is disallowed on 'S19' because its copy constructors and move constructors are all deleted}} CopyDeleted a; MoveDeleted b; }; diff --git a/clang/test/SemaObjCXX/attr-trivial-abi.mm b/clang/test/SemaObjCXX/attr-trivial-abi.mm index 87b79c14d07a6..a08be07c6379d 100644 --- a/clang/test/SemaObjCXX/attr-trivial-abi.mm +++ b/clang/test/SemaObjCXX/attr-trivial-abi.mm @@ -108,7 +108,8 @@ struct __attribute__((trivial_abi)) CopyMoveDeleted { // expected-warning {{'tri CopyMoveDeleted(CopyMoveDeleted &&) = delete; }; - struct __attribute__((trivial_abi)) S18 { // expected-warning {{'trivial_abi' cannot be applied to 'S18'}} expected-note {{copy constructors and move constructors are all deleted}} + struct __attribute__((trivial_abi)) S18 { // expected-warning {{'trivial_abi' cannot be applied to 'S18'}} \ + // expected-note {{'trivial_abi' is disallowed on 'S18' because it has a field of a non-trivial class type}} CopyMoveDeleted a; }; `````````` </details> https://github.com/llvm/llvm-project/pull/143111 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
