From 7846d590033e8d661198f4c00f56f46a4993c526 Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Fri, 4 Feb 2022 20:06:13 -0800 Subject: [PATCH] Extend the C++03 definition of POD to include defaulted functions The AST/conditionally-trivial-smfs tests look a bit questionable, but are consistent with GCC's POD-ness, at least as far as packing is concerned: https://godbolt.org/z/36nqPMbKM (questionable because it looks like the type would be non-copyable, so how could it be pod? But the calling convention/pass by value seems to work correctly (local testing verifies that this behavior is preserved even with this patch: https://godbolt.org/z/3Pa89zsv6 )) Differential Revision: https://reviews.llvm.org/D119051 --- clang/include/clang/Basic/LangOptions.h | 1 + clang/include/clang/Basic/TargetInfo.h | 8 +++++ clang/lib/AST/DeclCXX.cpp | 47 +++++++++++++++++---------- clang/lib/Basic/TargetInfo.cpp | 4 +++ clang/lib/Basic/Targets/OSTargets.h | 8 +++++ clang/test/AST/conditionally-trivial-smfs.cpp | 3 ++ clang/test/SemaCXX/class-layout.cpp | 26 ++++++++++++++- 7 files changed, 78 insertions(+), 19 deletions(-) diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h index cfa9832..a790385 100644 --- a/clang/include/clang/Basic/LangOptions.h +++ b/clang/include/clang/Basic/LangOptions.h @@ -229,6 +229,7 @@ public: /// This causes clang to: /// - Reverse the implementation for DR692, DR1395 and DR1432. /// - pack non-POD members of packed structs. + /// - consider classes with defaulted special member functions non-pod. Ver15, /// Conform to the underlying platform's C and C++ ABIs as closely diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h index 9b9439e..2c50b19 100644 --- a/clang/include/clang/Basic/TargetInfo.h +++ b/clang/include/clang/Basic/TargetInfo.h @@ -1566,6 +1566,14 @@ public: virtual CallingConvKind getCallingConvKind(bool ClangABICompat4) const; + /// Controls whether explicitly defaulted (`= default`) special member + /// functions disqualify something from being POD-for-the-purposes-of-layout. + /// Historically, Clang didn't consider these acceptable for POD, but GCC + /// does. So in newer Clang ABIs they are acceptable for POD to be compatible + /// with GCC/Itanium ABI, and remains disqualifying for targets that need + /// Clang backwards compatibility rather than GCC/Itanium ABI compatibility. + virtual bool areDefaultedSMFStillPOD(const LangOptions&) const; + /// Controls if __builtin_longjmp / __builtin_setjmp can be lowered to /// llvm.eh.sjlj.longjmp / llvm.eh.sjlj.setjmp. virtual bool hasSjLjLowering() const { diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp index b922109..16b2139 100644 --- a/clang/lib/AST/DeclCXX.cpp +++ b/clang/lib/AST/DeclCXX.cpp @@ -36,6 +36,7 @@ #include "clang/Basic/PartialDiagnostic.h" #include "clang/Basic/SourceLocation.h" #include "clang/Basic/Specifiers.h" +#include "clang/Basic/TargetInfo.h" #include "llvm/ADT/None.h" #include "llvm/ADT/SmallPtrSet.h" #include "llvm/ADT/SmallVector.h" @@ -768,12 +769,16 @@ void CXXRecordDecl::addedMember(Decl *D) { // Note that we have a user-declared constructor. data().UserDeclaredConstructor = true; - // C++ [class]p4: - // A POD-struct is an aggregate class [...] - // Since the POD bit is meant to be C++03 POD-ness, clear it even if - // the type is technically an aggregate in C++0x since it wouldn't be - // in 03. - data().PlainOldData = false; + const TargetInfo &TI = getASTContext().getTargetInfo(); + if ((!Constructor->isDeleted() && !Constructor->isDefaulted()) || + !TI.areDefaultedSMFStillPOD(getLangOpts())) { + // C++ [class]p4: + // A POD-struct is an aggregate class [...] + // Since the POD bit is meant to be C++03 POD-ness, clear it even if + // the type is technically an aggregate in C++0x since it wouldn't be + // in 03. + data().PlainOldData = false; + } } if (Constructor->isDefaultConstructor()) { @@ -881,18 +886,24 @@ void CXXRecordDecl::addedMember(Decl *D) { if (!Method->isImplicit()) { data().UserDeclaredSpecialMembers |= SMKind; - // C++03 [class]p4: - // A POD-struct is an aggregate class that has [...] no user-defined - // copy assignment operator and no user-defined destructor. - // - // Since the POD bit is meant to be C++03 POD-ness, and in C++03, - // aggregates could not have any constructors, clear it even for an - // explicitly defaulted or deleted constructor. - // type is technically an aggregate in C++0x since it wouldn't be in 03. - // - // Also, a user-declared move assignment operator makes a class non-POD. - // This is an extension in C++03. - data().PlainOldData = false; + const TargetInfo &TI = getASTContext().getTargetInfo(); + if ((!Method->isDeleted() && !Method->isDefaulted() && + SMKind != SMF_MoveAssignment) || + !TI.areDefaultedSMFStillPOD(getLangOpts())) { + // C++03 [class]p4: + // A POD-struct is an aggregate class that has [...] no user-defined + // copy assignment operator and no user-defined destructor. + // + // Since the POD bit is meant to be C++03 POD-ness, and in C++03, + // aggregates could not have any constructors, clear it even for an + // explicitly defaulted or deleted constructor. + // type is technically an aggregate in C++0x since it wouldn't be in + // 03. + // + // Also, a user-declared move assignment operator makes a class + // non-POD. This is an extension in C++03. + data().PlainOldData = false; + } } // When instantiating a class, we delay updating the destructor and // triviality properties of the class until selecting a destructor and diff --git a/clang/lib/Basic/TargetInfo.cpp b/clang/lib/Basic/TargetInfo.cpp index 833e37b..8def4be 100644 --- a/clang/lib/Basic/TargetInfo.cpp +++ b/clang/lib/Basic/TargetInfo.cpp @@ -559,6 +559,10 @@ TargetInfo::getCallingConvKind(bool ClangABICompat4) const { return CCK_Default; } +bool TargetInfo::areDefaultedSMFStillPOD(const LangOptions &LangOpts) const { + return LangOpts.getClangABICompat() > LangOptions::ClangABI::Ver15; +} + LangAS TargetInfo::getOpenCLTypeAddrSpace(OpenCLTypeKind TK) const { switch (TK) { case OCLTK_Image: diff --git a/clang/lib/Basic/Targets/OSTargets.h b/clang/lib/Basic/Targets/OSTargets.h index 88f7c4a..5f5b461 100644 --- a/clang/lib/Basic/Targets/OSTargets.h +++ b/clang/lib/Basic/Targets/OSTargets.h @@ -161,6 +161,10 @@ public: : TargetInfo::UnsignedLongLong) : TargetInfo::getLeastIntTypeByWidth(BitWidth, IsSigned); } + + bool areDefaultedSMFStillPOD(const LangOptions &) const override { + return false; + } }; // DragonFlyBSD Target @@ -558,6 +562,10 @@ public: checkCallingConvention(CallingConv CC) const override { return (CC == CC_C) ? TargetInfo::CCCR_OK : TargetInfo::CCCR_Error; } + + bool areDefaultedSMFStillPOD(const LangOptions &) const override { + return false; + } }; // PS4 Target diff --git a/clang/test/AST/conditionally-trivial-smfs.cpp b/clang/test/AST/conditionally-trivial-smfs.cpp index 34db1cd..1dc1f6c 100644 --- a/clang/test/AST/conditionally-trivial-smfs.cpp +++ b/clang/test/AST/conditionally-trivial-smfs.cpp @@ -297,6 +297,7 @@ template struct MoveAssignmentCheck<1>; // CHECK-NEXT: "isAggregate": true, // CHECK-NEXT: "isEmpty": true, // CHECK-NEXT: "isLiteral": true, +// CHECK-NEXT: "isPOD": true, // CHECK-NEXT: "isStandardLayout": true, // CHECK-NEXT: "isTrivial": true, // CHECK-NEXT: "isTriviallyCopyable": true, @@ -316,6 +317,7 @@ template struct MoveAssignmentCheck<2>; // CHECK-NEXT: "isAggregate": true, // CHECK-NEXT: "isEmpty": true, // CHECK-NEXT: "isLiteral": true, +// CHECK-NEXT: "isPOD": true, // CHECK-NEXT: "isStandardLayout": true, // CHECK-NEXT: "isTrivial": true, // CHECK-NEXT: "isTriviallyCopyable": true, @@ -335,6 +337,7 @@ template struct MoveAssignmentCheck<3>; // CHECK-NEXT: "isAggregate": true, // CHECK-NEXT: "isEmpty": true, // CHECK-NEXT: "isLiteral": true, +// CHECK-NEXT: "isPOD": true, // CHECK-NEXT: "isStandardLayout": true, // CHECK-NEXT: "moveAssign": { // CHECK-NEXT: "exists": true, diff --git a/clang/test/SemaCXX/class-layout.cpp b/clang/test/SemaCXX/class-layout.cpp index f3106ec..3d710dc 100644 --- a/clang/test/SemaCXX/class-layout.cpp +++ b/clang/test/SemaCXX/class-layout.cpp @@ -1,10 +1,12 @@ -// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++98 -Wno-inaccessible-base +// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++98 -Wno-inaccessible-base -Wno-c++11-extensions // RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base // RUN: %clang_cc1 -triple x86_64-apple-darwin %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -DCLANG_ABI_COMPAT=15 // RUN: %clang_cc1 -triple x86_64-scei-ps4 %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -DCLANG_ABI_COMPAT=6 // RUN: %clang_cc1 -triple x86_64-sie-ps5 %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -DCLANG_ABI_COMPAT=6 // RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=6 -DCLANG_ABI_COMPAT=6 +// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=14 -DCLANG_ABI_COMPAT=14 // RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=15 -DCLANG_ABI_COMPAT=15 +// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=16 -DCLANG_ABI_COMPAT=16 // expected-no-diagnostics #define SA(n, p) int a##n[(p) ? 1 : -1] @@ -656,3 +658,25 @@ struct C { _Static_assert(__builtin_offsetof(C, b) == 3, ""); } +namespace cxx11_pod { +struct t1 { + t1() = default; + t1(const t1&) = delete; + ~t1() = delete; + t1(t1&&) = default; + int a; + char c; +}; +struct t2 { + t1 v1; +} __attribute__((packed)); +_Static_assert(_Alignof(t2) == 1, ""); +struct t3 : t1 { + char c; +}; +#if defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <= 15 +_Static_assert(sizeof(t3) == 8, ""); +#else +_Static_assert(sizeof(t3) == 12, ""); +#endif +} -- 2.7.4