llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-codegen Author: Clayton Knittel (ClaytonKnittel) <details> <summary>Changes</summary> This is a roll-forward of https://github.com/llvm/llvm-project/pull/218165, which inadvertantly caused missing debug info in https://github.com/llvm/llvm-project/issues/221560 due to an existing issue with standard-layout unions and layout-compatible aliasing (see [[class.mem]](https://timsong-cpp.github.io/cppwp/n3337/class.mem#<!-- -->19)). Constexpr constructors without a visible definition in the TU are not callable in a constexpr context, which means it is not possible to construct a type in constexpr without emitting debug info for the constructor itself. Although undefined constexpr constructors are still callable, they are normal out of line functions, requiring a definition to link against. --- Full diff: https://github.com/llvm/llvm-project/pull/221971.diff 6 Files Affected: - (modified) clang/include/clang/AST/CXXRecordDeclDefinitionBits.def (+3) - (modified) clang/include/clang/AST/DeclCXX.h (+9) - (modified) clang/lib/AST/DeclCXX.cpp (+15-1) - (modified) clang/lib/CodeGen/CGDebugInfo.cpp (+26-5) - (modified) clang/test/DebugInfo/CXX/limited-ctor.cpp (+22-5) - (modified) clang/unittests/AST/DeclTest.cpp (+256) ``````````diff diff --git a/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def b/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def index 97e61aaec7d51..a45a57ecfff20 100644 --- a/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def +++ b/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def @@ -64,6 +64,9 @@ FIELD(Abstract, 1, NO_MERGE) /// language rules (including DRs). FIELD(IsStandardLayout, 1, NO_MERGE) +/// True when this class appears as a member of some standard-layout union. +FIELD(IsStandardLayoutUnionMember, 1, MERGE_OR) + /// True when this class was standard-layout under the C++11 /// definition. /// diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h index afe46fae1bceb..c46ab72cb1bc5 100644 --- a/clang/include/clang/AST/DeclCXX.h +++ b/clang/include/clang/AST/DeclCXX.h @@ -1233,6 +1233,15 @@ class CXXRecordDecl : public RecordDecl { /// C++ [class]p7. bool isStandardLayout() const { return data().IsStandardLayout; } + /// Determine whether this class appears as a member of a standard-layout + /// union. + bool isStandardLayoutUnionMember() const { + return data().IsStandardLayoutUnionMember; + } + void setIsStandardLayoutUnionMember(bool V = true) { + data().IsStandardLayoutUnionMember = V; + } + /// Determine whether this class was standard-layout per /// C++11 [class]p7, specifically using the C++11 rules without any DRs. bool isCXX11StandardLayout() const { return data().IsCXX11StandardLayout; } diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp index f0da56542ae7e..476eb0bc2d5f3 100644 --- a/clang/lib/AST/DeclCXX.cpp +++ b/clang/lib/AST/DeclCXX.cpp @@ -76,7 +76,8 @@ void LazyASTUnresolvedSet::getFromExternalSource(ASTContext &C) const { CXXRecordDecl::DefinitionData::DefinitionData(CXXRecordDecl *D) : UserDeclaredConstructor(false), UserDeclaredSpecialMembers(0), Aggregate(true), PlainOldData(true), Empty(true), Polymorphic(false), - Abstract(false), IsStandardLayout(true), IsCXX11StandardLayout(true), + Abstract(false), IsStandardLayout(true), + IsStandardLayoutUnionMember(false), IsCXX11StandardLayout(true), HasBasesWithFields(false), HasBasesWithNonStaticDataMembers(false), HasPrivateFields(false), HasProtectedFields(false), HasPublicFields(false), HasMutableFields(false), HasVariantMembers(false), @@ -2321,6 +2322,19 @@ void CXXRecordDecl::completeDefinition(CXXFinalOverriderMap *FinalOverriders) { data().IsStandardLayout = false; data().IsCXX11StandardLayout = false; } + + if (isUnion() && isStandardLayout()) { + for (const FieldDecl *FD : fields()) { + if (const auto *RT = + FD->getType()->getBaseElementTypeUnsafe()->getAs<RecordType>()) { + if (auto *CRD = + dyn_cast<CXXRecordDecl>(RT->getDecl()->getDefinitionOrSelf())) { + if (CRD->hasDefinition() && CRD->isStandardLayout()) + CRD->setIsStandardLayoutUnionMember(true); + } + } + } + } } bool CXXRecordDecl::mayBeAbstract() const { diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index 02864621d60a3..38bb67543ebc6 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -3239,11 +3239,28 @@ static bool canUseCtorHoming(const CXXRecordDecl *RD) { if (isClassOrMethodDLLImport(RD)) return false; - if (RD->isLambda() || RD->isAggregate() || - RD->hasTrivialDefaultConstructor() || - RD->hasConstexprNonCopyMoveConstructor()) + if (RD->isLambda() || RD->isAggregate() || RD->hasTrivialDefaultConstructor()) return false; + // Skip this optimization if the class has an implicit constexpr default + // constructor, since those constructors can be invoked without emitting type + // information for the constructor. + if (RD->needsImplicitDefaultConstructor() && + RD->defaultedDefaultConstructorIsConstexpr()) + return false; + + // Skip this optimization if this type is standard-layout and is a member of + // some standard-layout union in this translation unit. Per the C++ spec, "it + // is permitted to inspect the common initial part of any of" the "common + // initial sequence" of distinct types in a standard-layout union. This + // exception to strict aliasing enables producing a reference to a type + // without ever having constructed that type. + // + // See: https://timsong-cpp.github.io/cppwp/n3337/class.mem#19 + if (RD->isStandardLayoutUnionMember()) + return false; + + bool HasNonDeletedCtor = false; for (const CXXConstructorDecl *Ctor : RD->ctors()) { if (Ctor->isCopyOrMoveConstructor()) continue; @@ -3254,11 +3271,15 @@ static bool canUseCtorHoming(const CXXRecordDecl *RD) { // copy/move constructor, which does not enable homing. if (CtorDef->isDelegatingConstructor()) continue; + // Skip this optimization if we see a defined constexpr constructor, which + // can be invoked without emitting type info. + if (Ctor->isConstexpr() && !Ctor->isDeleted()) + return false; } if (!Ctor->isDeleted()) - return true; + HasNonDeletedCtor = true; } - return false; + return HasNonDeletedCtor; } static bool shouldOmitDefinition(llvm::codegenoptions::DebugInfoKind DebugKind, diff --git a/clang/test/DebugInfo/CXX/limited-ctor.cpp b/clang/test/DebugInfo/CXX/limited-ctor.cpp index e820c0703df4f..6394c01a467c6 100644 --- a/clang/test/DebugInfo/CXX/limited-ctor.cpp +++ b/clang/test/DebugInfo/CXX/limited-ctor.cpp @@ -27,10 +27,9 @@ struct E { constexpr E(){}; } TestE; -// Restored by this revert: a constexpr constructor that is only declared keeps -// the class exempt from constructor homing. See Aliased below for a case where -// narrowing the exemption to defined constructors homes the type nowhere. -// CHECK-DAG: !DICompositeType(tag: DW_TAG_structure_type, name: "DeclaredConstexpr"{{.*}}DIFlagTypePassByValue +// Declared but not defined constexpr constructor should not emit full debug +// info. +// CHECK-DAG: !DICompositeType(tag: DW_TAG_structure_type, name: "DeclaredConstexpr"{{.*}}flags: DIFlagFwdDecl struct DeclaredConstexpr { constexpr DeclaredConstexpr(); } TestDeclaredConstexpr; @@ -53,7 +52,7 @@ struct DeclaredConstexpr { template <class A, class B> struct Aliased { A first; B second; - constexpr Aliased(const A &a, const B &b) : first(a), second(b) {} + Aliased(const A &a, const B &b) : first(a), second(b) {} }; union AliasedSlot { Aliased<const int, int> value; @@ -66,6 +65,24 @@ int ReadAliasedSlot() { return TestAliasedSlot.value.first; } +// CHECK-DAG: !DICompositeType(tag: DW_TAG_structure_type, name: "ConstexprAliased<int, int>"{{.*}}DIFlagTypePassByValue +// CHECK-DAG: !DICompositeType(tag: DW_TAG_structure_type, name: "ConstexprAliased<const int, int>"{{.*}}DIFlagTypePassByValue +template <class A, class B> struct ConstexprAliased { + A first; + B second; + constexpr ConstexprAliased(const A &a, const B &b) : first(a), second(b) {} +}; +union ConstexprAliasedSlot { + ConstexprAliased<const int, int> value; + ConstexprAliased<int, int> mutable_value; + ConstexprAliasedSlot() {} + ~ConstexprAliasedSlot() {} +} TestConstexprAliasedSlot; +int ReadConstexprAliasedSlot() { + TestConstexprAliasedSlot.mutable_value = ConstexprAliased<int, int>(1, 2); + return TestConstexprAliasedSlot.value.first; +} + // Defined out-of-line constexpr constructor should emit full debug info. // CHECK-DAG: !DICompositeType(tag: DW_TAG_structure_type, name: "OutOfLineConstexpr"{{.*}}DIFlagTypePassByValue struct OutOfLineConstexpr { diff --git a/clang/unittests/AST/DeclTest.cpp b/clang/unittests/AST/DeclTest.cpp index 195b8ab4c4e66..6fa1da7f01f00 100644 --- a/clang/unittests/AST/DeclTest.cpp +++ b/clang/unittests/AST/DeclTest.cpp @@ -24,10 +24,12 @@ #include "clang/Basic/LLVM.h" #include "clang/Basic/Linkage.h" #include "clang/Basic/TargetInfo.h" +#include "clang/Frontend/ASTUnit.h" #include "clang/Lex/Lexer.h" #include "clang/Tooling/Tooling.h" #include "llvm/Support/raw_ostream.h" #include "llvm/Testing/Annotations/Annotations.h" +#include "gmock/gmock.h" #include "gtest/gtest.h" #include <cassert> #include <memory> @@ -36,6 +38,14 @@ using namespace clang::ast_matchers; using namespace clang::tooling; using namespace clang; +using ::testing::AllOf; +using ::testing::Not; +using ::testing::Pointee; + +MATCHER(IsStandardLayout, "") { return arg.isStandardLayout(); } +MATCHER(IsStandardLayoutUnionMember, "") { + return arg.isStandardLayoutUnionMember(); +} TEST(Decl, CleansUpAPValues) { MatchFinder Finder; @@ -1029,3 +1039,249 @@ TEST(Decl, ObjCPropertyDeclNameForDiagnostic) { /*Qualified=*/true); EXPECT_EQ(ExtPQualifiedOS.str(), "-[MyClass extensionProp]"); } + +class StandardLayoutUnionMemberTest : public ::testing::Test { +protected: + std::unique_ptr<ASTUnit> AST; + ASTContext *Ctx = nullptr; + + void buildAST(StringRef Code, ArrayRef<std::string> Args = {"-std=c++11"}) { + AST = tooling::buildASTFromCodeWithArgs(Code, Args); + ASSERT_NE(AST, nullptr); + Ctx = &AST->getASTContext(); + } + + const CXXRecordDecl *findRecord(StringRef Name) const { + assert(Ctx && "ASTContext not initialized"); + return selectFirst<CXXRecordDecl>( + "d", + match(cxxRecordDecl(hasName(Name), isDefinition()).bind("d"), *Ctx)); + } + + const CXXRecordDecl *findTemplate(StringRef Name, + StringRef TemplateArg) const { + assert(Ctx && "ASTContext not initialized"); + return selectFirst<CXXRecordDecl>( + "d", match(classTemplateSpecializationDecl( + hasName(Name), isDefinition(), + hasTemplateArgument( + 0, refersToType(asString(TemplateArg.str())))) + .bind("d"), + *Ctx)); + } + + // Matches a standard-layout union member. + auto IsSLUnionMember() { + return Pointee(AllOf(IsStandardLayout(), IsStandardLayoutUnionMember())); + } + + // Matches a standard-layout type that is not a member of a standard-layout + // union. + auto IsStandaloneSL() { + return Pointee( + AllOf(IsStandardLayout(), Not(IsStandardLayoutUnionMember()))); + } + + // Matches a non-standard-layout type. + auto IsNonSL() { + return Pointee( + AllOf(Not(IsStandardLayout()), Not(IsStandardLayoutUnionMember()))); + } +}; + +TEST_F(StandardLayoutUnionMemberTest, StandaloneStruct) { + buildAST(R"cc( + struct StandaloneSL { + int x; + }; + )cc"); + + EXPECT_THAT(findRecord("StandaloneSL"), IsStandaloneSL()); +} + +TEST_F(StandardLayoutUnionMemberTest, NonStandardLayoutStruct) { + buildAST(R"cc( + struct NonSL { + virtual void foo(); + }; + )cc"); + + EXPECT_THAT(findRecord("NonSL"), IsNonSL()); +} + +TEST_F(StandardLayoutUnionMemberTest, StandardLayoutUnion) { + buildAST(R"cc( + struct SLInUnion { + int x; + }; + + union SLUnion { + SLInUnion u; + }; + )cc"); + + EXPECT_THAT(findRecord("SLUnion"), IsStandaloneSL()); + EXPECT_THAT(findRecord("SLInUnion"), IsSLUnionMember()); +} + +TEST_F(StandardLayoutUnionMemberTest, TemplatedMember) { + buildAST(R"cc( + template <typename T> + struct TemplatedSL { + T x; + }; + + union TemplatedUnion { + TemplatedSL<int> a; + TemplatedSL<float> b; + }; + )cc"); + + EXPECT_THAT(findRecord("TemplatedUnion"), IsStandaloneSL()); + EXPECT_THAT(findTemplate("TemplatedSL", "int"), IsSLUnionMember()); + EXPECT_THAT(findTemplate("TemplatedSL", "float"), IsSLUnionMember()); +} + +TEST_F(StandardLayoutUnionMemberTest, NonStandardLayoutUnion) { + buildAST(R"cc( + struct NonSL { + virtual void foo(); + }; + + struct SLInNonSLUnion { + int x; + }; + + union NonSLUnion { + SLInNonSLUnion s; + NonSL n; + }; + )cc"); + + EXPECT_THAT(findRecord("NonSLUnion"), Pointee(Not(IsStandardLayout()))); + EXPECT_THAT(findRecord("NonSL"), IsNonSL()); + EXPECT_THAT(findRecord("SLInNonSLUnion"), IsStandaloneSL()); +} + +TEST_F(StandardLayoutUnionMemberTest, NestedStruct) { + buildAST(R"cc( + union NestedUnion { + struct NestedSL { + int a; + } n; + }; + )cc"); + + EXPECT_THAT(findRecord("NestedUnion"), IsStandaloneSL()); + EXPECT_THAT(findRecord("NestedSL"), IsSLUnionMember()); +} + +TEST_F(StandardLayoutUnionMemberTest, Array) { + buildAST(R"cc( + struct SLInArray { + int x; + }; + struct SLInMultiArray { + int y; + }; + union ArrayUnion { + SLInArray arr[3]; + SLInMultiArray multi_arr[2][4]; + int raw; + }; + )cc"); + + EXPECT_THAT(findRecord("ArrayUnion"), IsStandaloneSL()); + EXPECT_THAT(findRecord("SLInArray"), IsSLUnionMember()); + EXPECT_THAT(findRecord("SLInMultiArray"), IsSLUnionMember()); +} + +TEST_F(StandardLayoutUnionMemberTest, CVQualified) { + buildAST(R"cc( + struct SLConst { + int x; + }; + struct SLVolatile { + int y; + }; + union CVUnion { + const SLConst c; + volatile SLVolatile v; + int raw; + }; + )cc"); + + EXPECT_THAT(findRecord("CVUnion"), IsStandaloneSL()); + EXPECT_THAT(findRecord("SLConst"), IsSLUnionMember()); + EXPECT_THAT(findRecord("SLVolatile"), IsSLUnionMember()); +} + +TEST_F(StandardLayoutUnionMemberTest, GenericUnionTemplate) { + buildAST(R"cc( + template <typename T> + union GenericUnion { + T val; + int raw; + }; + struct SLInGenericUnion { + int x; + }; + GenericUnion<SLInGenericUnion> u; + )cc"); + + EXPECT_THAT(findRecord("SLInGenericUnion"), IsSLUnionMember()); +} + +TEST_F(StandardLayoutUnionMemberTest, AnonymousUnion) { + buildAST(R"cc( + struct SLInAnonUnion { + int x; + }; + struct EnclosingStruct { + union { + SLInAnonUnion a; + int b; + }; + }; + )cc"); + + EXPECT_THAT(findRecord("SLInAnonUnion"), IsSLUnionMember()); +} + +TEST_F(StandardLayoutUnionMemberTest, Inheritance) { + buildAST(R"cc( + struct EmptyBase {}; + struct SLDerived : EmptyBase { + int x; + }; + union DerivedUnion { + SLDerived d; + int raw; + }; + )cc"); + + EXPECT_THAT(findRecord("DerivedUnion"), IsStandaloneSL()); + EXPECT_THAT(findRecord("SLDerived"), IsSLUnionMember()); + EXPECT_THAT(findRecord("EmptyBase"), IsStandaloneSL()); +} + +TEST_F(StandardLayoutUnionMemberTest, AnonymousTypedefStruct) { + buildAST(R"cc( + typedef struct { + int x; + } AnonTypedefSL; + + union AnonTypedefUnion { + AnonTypedefSL s; + int raw; + }; + )cc"); + + const auto *TD = selectFirst<TypedefDecl>( + "d", match(typedefDecl(hasName("AnonTypedefSL")).bind("d"), *Ctx)); + ASSERT_NE(TD, nullptr); + const auto *AnonRecord = TD->getUnderlyingType()->getAsCXXRecordDecl(); + + EXPECT_THAT(findRecord("AnonTypedefUnion"), IsStandaloneSL()); + EXPECT_THAT(AnonRecord, IsSLUnionMember()); +} `````````` </details> https://github.com/llvm/llvm-project/pull/221971 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
