[Clang] Implement CWG2598: Union of non-literal types #78195
Conversation
A union is considered a literal type unless it has no non-literal member. This resolves CWG2096 (which makes unions with literal members literal) and CWG2598 (empty unions are literal types). Fixes llvm#77924
cor3ntin
added
c++
clang:frontend
|
@llvm/pr-subscribers-clang Author: cor3ntin (cor3ntin) ChangesA union is considered a literal type unless it has no non-literal member. This resolves CWG2096 (which makes unions with literal members literal) and CWG2598 (empty unions are literal types). Fixes #77924 Full diff: https://github.com/llvm/llvm-project/pull/78195.diff 7 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 7e130304c5c08f..d967d904ba3fce 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -227,6 +227,11 @@ C++2c Feature Support
Resolutions to C++ Defect Reports
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+- Implemented `CWG2598 <https://wg21.link/CWG2598>`_ and `CWG2096 <https://wg21.link/CWG2096>`_,
+ making unions (that have either no members or at least one literal member) literal types.
+ (`#77924: <https://github.com/llvm/llvm-project/issues/77924>`_).
+
+
C Language Changes
------------------
- ``structs``, ``unions``, and ``arrays`` that are const may now be used as
diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h
index 648f5f94640870..75b73700c44d67 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -1439,31 +1439,20 @@ class CXXRecordDecl : public RecordDecl {
/// Determine whether this class is a literal type.
///
- /// C++11 [basic.types]p10:
+ /// C++20 [basic.types]p10:
/// A class type that has all the following properties:
- /// - it has a trivial destructor
- /// - every constructor call and full-expression in the
- /// brace-or-equal-intializers for non-static data members (if any) is
- /// a constant expression.
- /// - it is an aggregate type or has at least one constexpr constructor
- /// or constructor template that is not a copy or move constructor, and
- /// - all of its non-static data members and base classes are of literal
- /// types
- ///
- /// We resolve DR1361 by ignoring the second bullet. We resolve DR1452 by
- /// treating types with trivial default constructors as literal types.
- ///
- /// Only in C++17 and beyond, are lambdas literal types.
- bool isLiteral() const {
- const LangOptions &LangOpts = getLangOpts();
- return (LangOpts.CPlusPlus20 ? hasConstexprDestructor()
- : hasTrivialDestructor()) &&
- (!isLambda() || LangOpts.CPlusPlus17) &&
- !hasNonLiteralTypeFieldsOrBases() &&
- (isAggregate() || isLambda() ||
- hasConstexprNonCopyMoveConstructor() ||
- hasTrivialDefaultConstructor());
- }
+ /// - it has a constexpr destructor
+ /// - all of its non-static non-variant data members and base classes
+ /// are of non-volatile literal types, and it:
+ /// - is a closure type
+ /// - is an aggregate union type that has either no variant members
+ /// or at least one variant member of non-volatile literal type
+ /// - is a non-union aggregate type for which each of its anonymous
+ /// union members satisfies the above requirements for an aggregate
+ /// union type, or
+ /// - has at least one constexpr constructor or constructor template
+ /// that is not a copy or move constructor.
+ bool isLiteral() const;
/// Determine whether this is a structural type.
bool isStructural() const {
diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index 98b0a6dc28ea2f..0d49d54fd64197 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -1383,6 +1383,34 @@ void CXXRecordDecl::addedMember(Decl *D) {
}
}
+bool CXXRecordDecl::isLiteral() const {
+ const LangOptions &LangOpts = getLangOpts();
+ if (!(LangOpts.CPlusPlus20 ? hasConstexprDestructor()
+ : hasTrivialDestructor()))
+ return false;
+
+ if (isLambda() && !LangOpts.CPlusPlus17)
+ return false;
+
+ if (hasNonLiteralTypeFieldsOrBases()) {
+ // CWG2598
+ // is an aggregate union type that has either no variant
+ // members or at least one variant member of non-volatile literal type,
+ if (!isUnion())
+ return false;
+ bool HasAtLeastOneTrivialMember =
+ fields().empty() || any_of(fields(), [this](const FieldDecl *D) {
+ return !D->getType().isVolatileQualified() &&
+ D->getType().isTrivialType(getASTContext());
+ });
+ if (!HasAtLeastOneTrivialMember)
+ return false;
+ }
+
+ return isAggregate() || isLambda() || hasConstexprNonCopyMoveConstructor() ||
+ hasTrivialDefaultConstructor();
+}
+
void CXXRecordDecl::addedSelectedDestructor(CXXDestructorDecl *DD) {
DD->setIneligibleOrNotSelected(false);
addedEligibleSpecialMemberFunction(DD, SMF_Destructor);
diff --git a/clang/test/CXX/drs/dr20xx.cpp b/clang/test/CXX/drs/dr20xx.cpp
index 60ee7684440f54..f7f37379e61ad1 100644
--- a/clang/test/CXX/drs/dr20xx.cpp
+++ b/clang/test/CXX/drs/dr20xx.cpp
@@ -418,3 +418,5 @@ namespace dr2094 { // dr2094: 5
static_assert(__is_trivially_assignable(A, const A&), "");
static_assert(__is_trivially_assignable(B, const B&), "");
}
+
+// dr2096: dup 2598
diff --git a/clang/test/CXX/drs/dr25xx.cpp b/clang/test/CXX/drs/dr25xx.cpp
index 32bbfc63d0df4f..6f51e9c249040a 100644
--- a/clang/test/CXX/drs/dr25xx.cpp
+++ b/clang/test/CXX/drs/dr25xx.cpp
@@ -208,3 +208,50 @@ namespace dr2565 { // dr2565: 16 open
#endif
}
+
+
+namespace dr2598 { // dr2598: 18
+#if __cplusplus >= 201103L
+struct NonLiteral {
+ NonLiteral();
+};
+
+struct anonymous1 {
+ union {} a;
+};
+static_assert(__is_literal(anonymous1), "");
+
+struct anonymous2 {
+ union { char c; };
+};
+static_assert(__is_literal(anonymous2), "");
+
+struct anonymous3 {
+ union { char c; NonLiteral NL; };
+};
+static_assert(__is_literal(anonymous3), "");
+
+struct anonymous4 {
+ union { NonLiteral NL; };
+};
+static_assert(!__is_literal(anonymous4), "");
+
+union empty {};
+static_assert(__is_literal(empty), "");
+
+union union1 { char c; };
+static_assert(__is_literal(union1), "");
+
+union union2 { char c; NonLiteral NL;};
+static_assert(__is_literal(union2), "");
+
+union union3 { NonLiteral NL;};
+static_assert(!__is_literal(union3), "");
+
+union union4 { union4(); };
+static_assert(!__is_literal(union4), "");
+
+union union5 { static NonLiteral NL; };
+static_assert(__is_literal(union5), "");
+#endif
+}
diff --git a/clang/test/SemaCXX/literal-type.cpp b/clang/test/SemaCXX/literal-type.cpp
index 14a4094c45a1b8..88535c169fe01c 100644
--- a/clang/test/SemaCXX/literal-type.cpp
+++ b/clang/test/SemaCXX/literal-type.cpp
@@ -1,4 +1,6 @@
// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
+
static_assert(__is_literal(int), "fail");
static_assert(__is_literal_type(int), "fail"); // alternate spelling for GCC
@@ -75,3 +77,34 @@ template <typename T> class HasConstExprCtorT {
static_assert(__is_literal(HasConstExprCtor), "fail");
static_assert(__is_literal(HasConstExprCtorTemplate<int>), "fail");
static_assert(__is_literal(HasConstExprCtorT<NonLiteral>), "fail");
+
+
+#if __cplusplus >= 202003L
+namespace GH77924 {
+
+struct A { A(); };
+template <class T>
+struct opt {
+ union Data {
+ constexpr Data() : x{} {}
+ constexpr ~Data() {}
+ char x;
+ T data;
+ };
+
+ constexpr opt() : data{} {}
+ constexpr ~opt() { if (engaged) data.data.~T(); }
+ Data data;
+ bool engaged = false;
+};
+
+consteval void foo() {
+ opt<A> a;
+}
+
+void test() {
+ foo();
+}
+
+}
+#endif
diff --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html
index 397bf1357d3cb3..5e7c1a0fa2f246 100755
--- a/clang/www/cxx_dr_status.html
+++ b/clang/www/cxx_dr_status.html
@@ -12384,7 +12384,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
<td><a href="https://cplusplus.github.io/CWG/issues/2096.html">2096</a></td>
<td>CD4</td>
<td>Constraints on literal unions</td>
- <td class="unknown" align="center">Unknown</td>
+ <td class="unreleased" align="center">Duplicate of <a href="#2598">2598</a></td>
</tr>
<tr class="open" id="2097">
<td><a href="https://cplusplus.github.io/CWG/issues/2097.html">2097</a></td>
@@ -15396,7 +15396,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
<td><a href="https://cplusplus.github.io/CWG/issues/2598.html">2598</a></td>
<td>C++23</td>
<td>Unions should not require a non-static data member of literal type</td>
- <td class="unknown" align="center">Unknown</td>
+ <td class="unreleased" align="center">Clang 18</td>
</tr>
<tr id="2599">
<td><a href="https://cplusplus.github.io/CWG/issues/2599.html">2599</a></td>
|
clang/lib/AST/DeclCXX.cpp
Outdated
| bool HasAtLeastOneTrivialMember = | ||
| fields().empty() || any_of(fields(), [this](const FieldDecl *D) { | ||
| return !D->getType().isVolatileQualified() && | ||
| D->getType().isTrivialType(getASTContext()); |
There was a problem hiding this comment.
What about literal non-trivial types (e.g. struct Literal { constexpr Literal() {} }; union union6 { NonLiteral NL; Literal L; };?
.isLiteralType should work here
There was a problem hiding this comment.
Autocompletion blunder, Nice catch!
clang/lib/AST/DeclCXX.cpp
Outdated
| // members or at least one variant member of non-volatile literal type, | ||
| if (!isUnion()) | ||
| return false; | ||
| bool HasAtLeastOneTrivialMember = |
There was a problem hiding this comment.
Why use trivial here? The DRs don't refer to trivial it refers to non-volatile literal type.
There was a problem hiding this comment.
Yes, that makes no sense
| static_assert(__is_literal(union5), ""); | ||
|
|
||
| struct Literal { constexpr Literal() {} }; | ||
| union union6 { NonLiteral NL; Literal L; }; |
There was a problem hiding this comment.
Can we also include the exact example from CWG2598 as well, just to be complete.
There was a problem hiding this comment.
Also should we have a static_assert after this line?
There was a problem hiding this comment.
I also noticed that gcc does not seem to support __is_literal only __is_literal_type
There was a problem hiding this comment.
Yup, both are synonym in clang
Some expression will become constant expressions and presumably that can be SFINAE upon, but given that this is a DR, I don't think we are affecting conformance and I don't think we make strong sfinae guarantee. GCC is conforming with that DR in all language mode since 10.0 |
clang/lib/AST/DeclCXX.cpp
Outdated
| return false; | ||
| } | ||
|
|
||
| return isAggregate() || isLambda() || hasConstexprNonCopyMoveConstructor() || |
There was a problem hiding this comment.
Should these conditions be moved 'above' the union stuff? I would think they are 'cheaper'. Also, I see the 'isLambda' here, despite it only being in C++17 mode on line 1392.
clang/lib/AST/DeclCXX.cpp
Outdated
| @@ -1389,7 +1389,11 @@ bool CXXRecordDecl::isLiteral() const { | |||
| : hasTrivialDestructor())) | |||
| return false; | |||
|
|
|||
| if (isLambda() && !LangOpts.CPlusPlus17) | |||
| // Lambdas are literal types since C++17. | |||
There was a problem hiding this comment.
So why is isAggregate or hasConstexprNonCopyMoveConstructor/etc not just return true here? Am I missing something?
There was a problem hiding this comment.
And aggregate can have non literal members
There was a problem hiding this comment.
Hmm... this is worse then. Other than the concern about 'isLambda', I think you should revert the suggested change here. Its not worth the awkward inversions here.
|
Thanks for the fix @cor3ntin. There is still a problem however with struct A {
A ();
~A();
};
template <class T>
struct opt
{
union {
char c;
T data;
};
constexpr opt() {}
constexpr ~opt() {
if (engaged)
data.~T();
}
bool engaged = false;
};
consteval void foo()
{
opt<A> a;
}This fails with I suspect that the culprit is here : llvm-project/clang/lib/AST/DeclCXX.cpp Line 550 in fca6992 The implicit destructor of the anonymous union is not constexpr because one of its subobjects has a non-constexpr destructor. |
|
@groundswellaudio Thanks for letting me know. can you create a separate issue ? |
|
@groundswellaudio actually, that should be fixed by #77753 |
Yes, it does make an error to go away. However the function |
|
@Fznamznon |
|
|
|
By looking at your tests on anonymous unions, I'd say that indirect fields are handled correctly (perhaps they're not included in the fields range?, otherwise the union-like class would not be considered literal) and that rather the implicitly generated special member functions of the anonymous union are the problems (and it's unclear to me why these would be needed at all, but perhaps it's simpler to generate them that to make a special case anonymous unions). |
It should actually, CF comments in that PR (I got your code working locally) |
|
@cor3ntin , perhaps a dumb question, but #77753 should only help in post C++20 modes since only there destructor can be |
A union is considered a literal type unless it has no non-literal member.
This resolves CWG2096 (which makes unions with literal members literal) and CWG2598 (empty unions are literal types).
Fixes #77924