[Clang] Implement CWG2351 `void{}` #78060

MitalAshok · 2024-01-13T18:40:15Z

Per CWG2351, allow void{}, treated the same as void(): a prvalue expression of type void that performs no initialization.

Note that the AST for the expression T{} looks like:

// using T = int;
CXXFunctionalCastExpr 'T':'int' functional cast to T <NoOp>
`-InitListExpr 'T':'int'
// using T = const int;
CXXFunctionalCastExpr 'int' functional cast to T <NoOp>
`-InitListExpr 'int'
// using T = void;
CXXFunctionalCastExpr 'T':'void' functional cast to T <ToVoid>
`-InitListExpr 'void'
// using T = const void;
CXXFunctionalCastExpr 'void' functional cast to T <ToVoid>
`-InitListExpr 'void'

As for void()/T() [T = const void], that looked like CXXScalarValueInitExpr 'void' and is unchanged after this.

For reference, C++98 [5.2.3p2] says:

The expression T(), where T is a simple-type-specifier (7.1.5.2) for a non-array complete object type or the (possibly cv-qualified) void type, creates an rvalue of the specified type, whose value is determined by default-initialization (8.5; no initialization is done for the void() case). [Note: if T is a non-class type that is cv-qualified, the cv-qualifiers are ignored when determining the type of the resulting rvalue (3.10). ]

Though it is a bit of a misnomer that, for T = void, CXXScalarValueInitExpr does not perform value initialization, it would be a breaking change to change the AST node for void(), so I simply reworded the doc comment.

llvmbot · 2024-01-13T18:40:43Z

@llvm/pr-subscribers-clang

Author: Mital Ashok (MitalAshok)

Changes

As per CWG2351, allow void{}, treated the same as void(): a prvalue expression of type void that performs no initialization.

Note that the AST for the expression T{} looks like:

// using T = int;
CXXFunctionalCastExpr 'T':'int' functional cast to T &lt;NoOp&gt;
`-InitListExpr 'T':'int'
// using T = const int;
CXXFunctionalCastExpr 'int' functional cast to T &lt;NoOp&gt;
`-InitListExpr 'int'
// using T = void;
CXXFunctionalCastExpr 'void' functional cast to T &lt;NoOp&gt;
`-InitListExpr 'void'
// using T = const void;
CXXFunctionalCastExpr 'void' functional cast to T &lt;NoOp&gt;
`-InitListExpr 'void'

(Since the InitListExpr already has void before anything is done, the type doesn't need to be adjusted)

As for void()/T() [T = const void], that looked like CXXScalarValueInitExpr 'void' and is unchanged after this.

For reference, C++98 [5.2.3p2] says:

> The expression T(), where T is a simple-type-specifier (7.1.5.2) for a non-array complete object type or the (possibly cv-qualified) void type, creates an rvalue of the specified type, whose value is determined by default-initialization (8.5; no initialization is done for the void() case). [Note: if T is a non-class type that is cv-qualified, the cv-qualifiers are ignored when determining the type of the resulting rvalue (3.10). ]

Though it is a bit of a misnomer that, for T = void, CXXScalarValueInitExpr does not perform value initialization, it would be a breaking change to change the AST node for void(), so I simply reworded the doc comment.

Full diff: https://github.com/llvm/llvm-project/pull/78060.diff

5 Files Affected:

(modified) clang/docs/ReleaseNotes.rst (+2)
(modified) clang/include/clang/AST/ExprCXX.h (+3-2)
(modified) clang/lib/Sema/SemaExprCXX.cpp (+15-6)
(modified) clang/test/CXX/drs/dr23xx.cpp (+31)
(modified) clang/www/cxx_dr_status.html (+1-1)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 3cbce1be159437..00e200ea76e5bc 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -226,6 +226,8 @@ C++2c Feature Support
 
 Resolutions to C++ Defect Reports
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+- Implemented `CWG2351 <https://wg21.link/CWG2351>`_ which allows ``void{}``
+  as a prvalue of type ``void``.
 
 C Language Changes
 ------------------
diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h
index 24278016431837..e7b732e4181127 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -2160,8 +2160,9 @@ class LambdaExpr final : public Expr,
   const_child_range children() const;
 };
 
-/// An expression "T()" which creates a value-initialized rvalue of type
-/// T, which is a non-class type.  See (C++98 [5.2.3p2]).
+/// An expression "T()" which creates an rvalue of type T, which is a
+/// non-class type. For non-void T, the rvalue is value-initialized.
+/// See (C++98 [5.2.3p2]).
 class CXXScalarValueInitExpr : public Expr {
   friend class ASTStmtReader;
 
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 4ae04358d5df7c..1792ec6bb292d5 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -1600,12 +1600,21 @@ Sema::BuildCXXTypeConstructExpr(TypeSourceInfo *TInfo,
     return ExprError(Diag(TyBeginLoc, diag::err_init_for_function_type)
                        << Ty << FullRange);
 
-  // C++17 [expr.type.conv]p2:
-  //   If the type is cv void and the initializer is (), the expression is a
-  //   prvalue of the specified type that performs no initialization.
-  if (!Ty->isVoidType() &&
-      RequireCompleteType(TyBeginLoc, ElemTy,
-                          diag::err_invalid_incomplete_type_use, FullRange))
+  // C++17 [expr.type.conv]p2, per DR2351:
+  //   If the type is cv void and the initializer is () or {}, the expression is
+  //   a prvalue of the specified type that performs no initialization.
+  if (Ty->isVoidType()) {
+    if (Exprs.empty())
+      return new (Context) CXXScalarValueInitExpr(Context.VoidTy, TInfo,
+                                                  Kind.getRange().getEnd());
+    if (ListInitialization && cast<InitListExpr>(Exprs[0])->getNumInits() == 0)
+      return CXXFunctionalCastExpr::Create(
+          Context, Context.VoidTy, VK_PRValue, TInfo, CK_NoOp, Exprs[0],
+          /*Path=*/nullptr, CurFPFeatureOverrides(), Exprs[0]->getBeginLoc(),
+          Exprs[0]->getEndLoc());
+  } else if (RequireCompleteType(TyBeginLoc, ElemTy,
+                                 diag::err_invalid_incomplete_type_use,
+                                 FullRange))
     return ExprError();
 
   //   Otherwise, the expression is a prvalue of the specified type whose
diff --git a/clang/test/CXX/drs/dr23xx.cpp b/clang/test/CXX/drs/dr23xx.cpp
index d2f4e7652ab568..1bf30a80f743e5 100644
--- a/clang/test/CXX/drs/dr23xx.cpp
+++ b/clang/test/CXX/drs/dr23xx.cpp
@@ -69,6 +69,37 @@ namespace dr2346 { // dr2346: 11
   }
 }
 
+namespace dr2351 { // dr2351: 18
+#if __cplusplus >= 201103L
+  static_assert((void{}, true), "");
+  // since-cxx11-warning@-1 {{left operand of comma operator has no effect}}
+
+  void f() {
+    return void{};
+  }
+
+  template<typename T>
+  void g() {
+    return T{};
+  }
+  template void g<void>();
+
+  void h() {
+    return {};
+    // since-cxx11-error@-1 {{void function 'h' must not return a value}}
+  }
+
+  template<typename T, int... I>
+  T i() {
+    return T{I...};
+  }
+  template void i<void>();
+
+  static_assert((void({}), true), "");
+  // since-cxx11-error@-1 {{cannot initialize non-class type 'void' with a parenthesized initializer list}}
+#endif
+}
+
 namespace dr2352 { // dr2352: 10
   int **p;
   const int *const *const &f1() { return p; }
diff --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html
index 4a3ed19161f9a5..207f032a5d45e4 100755
--- a/clang/www/cxx_dr_status.html
+++ b/clang/www/cxx_dr_status.html
@@ -13914,7 +13914,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
     <td><a href="https://cplusplus.github.io/CWG/issues/2351.html">2351</a></td>
     <td>CD5</td>
     <td><TT>void{}</TT></td>
-    <td class="unknown" align="center">Unknown</td>
+    <td class="unreleased" align="center">Clang 18</td>
   </tr>
   <tr id="2352">
     <td><a href="https://cplusplus.github.io/CWG/issues/2352.html">2352</a></td>

Endilll

DR testing part looks fine.
I'm worried there are no regular tests. It's also not clear what happens in 98 mode. New code doesn't seem to care about language mode.

MitalAshok · 2024-01-13T19:01:21Z

Initializer list syntax isn't available in C++98 mode (even as an extension? I can't find the option)
Even so, as a defect report it should apply to all prior C++ versions.

Endilll · 2024-01-13T19:29:34Z

Initializer list syntax isn't available in C++98 mode (even as an extension? I can't find the option)

I'm not confident enough to properly review your changes, but my line of thinking is the following: void() is available in all language modes, but you're adding C++11-specific functionality (as uniform initializer syntax is not a thing in 98) without ever checking language mode. This sound suspicious enough to me to bring attention to, even if there are other reasons why it's not really necessary to check language mode here (which I'm not aware of, but my knowledge is limited).

Even so, as a defect report it should apply to all prior C++ versions.

Formally, defect reports apply to then latest publication, which is C++14, if not 17. Implementation indeed try to backport them as far back as they reasonably can. Start accepting uniform initialization syntax in 98 mode because of void{} DR doesn't sound reasonable to me.

MitalAshok · 2024-01-13T20:02:25Z

Looking at other places, it looks like init-list stuff is guarded behind getLangOpts().CPlusPlus11, so I'll add that check.

It looks like this DR is CD5 (after C++17, applies to C++17), but void{} in C++11/14 without a warning seems fine.

As for "regular tests", do you mean in /clang/test/CXX? Should I also add tests there?

I also found this: https://stackoverflow.com/q/41131806 that void({}) apparently used to (erroneously) be accepted in Clang 3.8 but I can't find any tests for it

Endilll · 2024-01-13T20:08:57Z

Looking at other places, it looks like init-list stuff is guarded behind getLangOpts().CPlusPlus11, so I'll add that check.

Corentin told me offline that check for list initialization that you do might be sufficient, as it can't pass in 98.

It looks like this DR is CD5 (after C++17, applies to C++17), but void{} in C++11/14 without a warning seems fine.

Yes, 11 and 14 are totally fine. We may want a pedantic warning, though.

As for "regular tests", do you mean in /clang/test/CXX? Should I also add tests there?

No, this is C++ conformance test suite. I meant clang/test/SemaCXX.

cor3ntin · 2024-01-13T20:09:15Z

I would recommend adding a test in C++98 mode. I don't think gating is necessary, the code should not parse.

Adding a test for ({}) is also a good idea.

Endilll · 2024-01-13T21:24:46Z

LGTM

cor3ntin

LGTM

shafik

Just a quick question

shafik · 2024-02-09T01:38:09Z

clang/lib/Sema/SemaExprCXX.cpp

+  // C++17 [expr.type.conv]p2, per DR2351:
+  //   If the type is cv void and the initializer is () or {}, the expression is
+  //   a prvalue of the specified type that performs no initialization.
+  if (Ty->isVoidType()) {


I am trying to understand why the check is here, and I think it is b/c the diagnostic is coming from one of these two lines:

InitializationSequence InitSeq(*this, Entity, Kind, Exprs); ExprResult Result = InitSeq.Perform(*this, Entity, Kind, Exprs);

is that correct? So then we need to handle this case before we attempt the initialization sequence.

[expr.type.conv]p2 reads "T(expression-list) or T braced-init-list is a cast expression, a prvalue void, or direct-initialized from the initializer". So when we encounter void() or void{}, we shouldn't actually use initialization semantics.

Previously, void() worked because

llvm-project/clang/lib/Sema/SemaInit.cpp

Lines 6255 to 6260 in 2d09ac4

// - If the initializer is (), the object is value-initialized.

if (Kind.getKind() == InitializationKind::IK_Value ||

(Kind.getKind() == InitializationKind::IK_Direct && Args.empty())) {

TryValueInitialization(S, Entity, Kind, *this);

return;

}

made it a value initialization by accident, and we allowed value initializing void

I've now added an assertion for void in TryValueInitialization because you shouldn't actually be able to get there, since there's no standard way to call for the value-intialization of void

cor3ntin · 2024-03-07T14:26:03Z

@MitalAshok ping

shafik · 2024-04-29T23:14:38Z

@MitalAshok ping

zygoloid · 2024-04-29T23:31:51Z

Note that the AST for the expression T{} looks like:

// using T = int;
CXXFunctionalCastExpr 'T':'int' functional cast to T <NoOp>
`-InitListExpr 'T':'int'
// using T = const int;
CXXFunctionalCastExpr 'int' functional cast to T <NoOp>
`-InitListExpr 'int'
// using T = void;
CXXFunctionalCastExpr 'void' functional cast to T <NoOp>
`-InitListExpr 'void'
// using T = const void;
CXXFunctionalCastExpr 'void' functional cast to T <NoOp>
`-InitListExpr 'void'

(Since the InitListExpr already has void before anything is done, the type doesn't need to be adjusted)

Nonetheless, I think it'd be more reasonable to use CK_ToVoid here rather than CK_NoOp. The void type for the InitListExpr is just a placeholder and not meant to mean that it's a real expression of type void. (We really ought to use a better placeholder there, so we can print the type out as <braced initializer list> instead of as void in diagnostics.)

MitalAshok · 2024-05-21T16:19:03Z

@zygoloid The commit message was a bit outdated, it now takes the void type from T instead of the initializer list (this also preserves extra info like if it came from a typedef)

I've also changed it to CK_ToVoid

zygoloid

LG

MitalAshok requested a review from Endilll as a code owner January 13, 2024 18:40

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jan 13, 2024

Endilll reviewed Jan 13, 2024

View reviewed changes

Endilll requested a review from cor3ntin January 13, 2024 18:56

MitalAshok force-pushed the cwg2351 branch from bdb4b24 to b33b2bc Compare January 13, 2024 20:49

cor3ntin approved these changes Jan 13, 2024

View reviewed changes

shafik reviewed Feb 9, 2024

View reviewed changes

[SemaCXX] Implement CWG2351 void{}

6ed7cad

MitalAshok force-pushed the cwg2351 branch from b33b2bc to 6ed7cad Compare April 26, 2024 15:43

Merge branch 'main' into cwg2351

a70855e

MitalAshok changed the title ~~[SemaCXX] Implement CWG2351 void{}~~ [Clang] Implement CWG2351 void{} May 21, 2024

Change cast kind to CK_ToVoid instead of CK_NoOp

70b62c2

zygoloid approved these changes May 21, 2024

View reviewed changes

Merge branch 'main' into cwg2351

2146e36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Clang] Implement CWG2351 `void{}` #78060

[Clang] Implement CWG2351 `void{}` #78060

MitalAshok commented Jan 13, 2024 •

edited

llvmbot commented Jan 13, 2024

Endilll left a comment

MitalAshok commented Jan 13, 2024

Endilll commented Jan 13, 2024

MitalAshok commented Jan 13, 2024

Endilll commented Jan 13, 2024

cor3ntin commented Jan 13, 2024

Endilll commented Jan 13, 2024

cor3ntin left a comment

shafik left a comment

shafik Feb 9, 2024

MitalAshok Apr 26, 2024

cor3ntin commented Mar 7, 2024

shafik commented Apr 29, 2024

zygoloid commented Apr 29, 2024

MitalAshok commented May 21, 2024

zygoloid left a comment

	// - If the initializer is (), the object is value-initialized.
	if (Kind.getKind() == InitializationKind::IK_Value \|\|
	(Kind.getKind() == InitializationKind::IK_Direct && Args.empty())) {
	TryValueInitialization(S, Entity, Kind, *this);
	return;
	}

[Clang] Implement CWG2351 void{} #78060

Are you sure you want to change the base?

[Clang] Implement CWG2351 void{} #78060

Conversation

MitalAshok commented Jan 13, 2024 • edited

llvmbot commented Jan 13, 2024

Endilll left a comment

Choose a reason for hiding this comment

MitalAshok commented Jan 13, 2024

Endilll commented Jan 13, 2024

MitalAshok commented Jan 13, 2024

Endilll commented Jan 13, 2024

cor3ntin commented Jan 13, 2024

Endilll commented Jan 13, 2024

cor3ntin left a comment

Choose a reason for hiding this comment

shafik left a comment

Choose a reason for hiding this comment

shafik Feb 9, 2024

Choose a reason for hiding this comment

MitalAshok Apr 26, 2024

Choose a reason for hiding this comment

cor3ntin commented Mar 7, 2024

shafik commented Apr 29, 2024

zygoloid commented Apr 29, 2024

MitalAshok commented May 21, 2024

zygoloid left a comment

Choose a reason for hiding this comment

[Clang] Implement CWG2351 `void{}` #78060

[Clang] Implement CWG2351 `void{}` #78060

MitalAshok commented Jan 13, 2024 •

edited