https://github.com/MitalAshok updated https://github.com/llvm/llvm-project/pull/100548
>From 2c1d87c025a8caa818dd275560f31d0d4f8d64b8 Mon Sep 17 00:00:00 2001 From: Mital Ashok <mi...@mitalashok.co.uk> Date: Thu, 25 Jul 2024 10:56:16 +0100 Subject: [PATCH 1/2] [Clang] Initializer list on RHS of assignment --- clang/docs/ReleaseNotes.rst | 4 ++ clang/lib/Sema/SemaExpr.cpp | 65 +++++++++++++++++++------ clang/test/CXX/drs/cwg27xx.cpp | 26 +++++++--- clang/test/SemaCXX/assign-init-list.cpp | 20 ++++++++ clang/www/cxx_dr_status.html | 2 +- 5 files changed, 95 insertions(+), 22 deletions(-) create mode 100644 clang/test/SemaCXX/assign-init-list.cpp diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 0b79e952b48af..bc048e8bfacba 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -84,6 +84,9 @@ C++2c Feature Support Resolutions to C++ Defect Reports ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +- Reject assigning to enums with an initailizer list containing an integer. + (`CWG2768: Assignment to enumeration variable with a braced-init-list <https://cplusplus.github.io/CWG/issues/2768.html>`_). + C Language Changes ------------------ @@ -150,6 +153,7 @@ Bug Fixes to C++ Support ^^^^^^^^^^^^^^^^^^^^^^^^ - Fixed a crash when an expression with a dependent ``__typeof__`` type is used as the operand of a unary operator. (#GH97646) +- Reject compound assignment operators with a braced-init-list. Bug Fixes to AST Handling ^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 74c0e01705905..08ee28c108952 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -14578,21 +14578,56 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc, BinaryOperatorKind Opc, Expr *LHSExpr, Expr *RHSExpr) { if (getLangOpts().CPlusPlus11 && isa<InitListExpr>(RHSExpr)) { - // The syntax only allows initializer lists on the RHS of assignment, - // so we don't need to worry about accepting invalid code for - // non-assignment operators. - // C++11 5.17p9: - // The meaning of x = {v} [...] is that of x = T(v) [...]. The meaning - // of x = {} is x = T(). - InitializationKind Kind = InitializationKind::CreateDirectList( - RHSExpr->getBeginLoc(), RHSExpr->getBeginLoc(), RHSExpr->getEndLoc()); - InitializedEntity Entity = - InitializedEntity::InitializeTemporary(LHSExpr->getType()); - InitializationSequence InitSeq(*this, Entity, Kind, RHSExpr); - ExprResult Init = InitSeq.Perform(*this, Entity, Kind, RHSExpr); - if (Init.isInvalid()) - return Init; - RHSExpr = Init.get(); + // C++11 [expr.ass]p9, per CWG2768: + // A braced-init-list B may appear on the right-hand side of + // - an assignment to a scalar of type T, in which case the initializer + // list shall have at most a single element. The meaning of x = B is + // x = t, where t is an invented temporary variable declared and + // initialized as T t = B. + switch (Opc) { + case BO_Assign: { + QualType LHSTy = LHSExpr->getType(); + assert(!LHSTy->isDependentType() && + "Should not have tried to create a builtin binary operator"); + if (LHSTy->isScalarType()) { + InitializationKind Kind = + InitializationKind::CreateCopy(RHSExpr->getBeginLoc(), OpLoc); + InitializedEntity Entity = + InitializedEntity::InitializeTemporary(LHSExpr->getType()); + InitializationSequence InitSeq(*this, Entity, Kind, RHSExpr); + ExprResult InventedTemporary = + InitSeq.Perform(*this, Entity, Kind, RHSExpr); + if (InventedTemporary.isInvalid()) + return InventedTemporary; + assert(cast<InitListExpr>(RHSExpr)->getNumInits() <= 1 && + "The initialization should have failed"); + RHSExpr = InventedTemporary.get(); + } + break; + } + case BO_MulAssign: + case BO_DivAssign: + case BO_RemAssign: + case BO_AddAssign: + case BO_SubAssign: + case BO_ShlAssign: + case BO_ShrAssign: + case BO_AndAssign: + case BO_XorAssign: + case BO_OrAssign: { + // A compound assignment like `i += {0}` is equivalent to `i = i + {0}`, + // which is a parsing error + StringRef Op = BinaryOperator::getOpcodeStr(Opc); + [[maybe_unused]] bool AssignmentStripped = Op.consume_back("="); + assert(AssignmentStripped); + Diag(OpLoc, diag::err_init_list_bin_op) + << 1 << Op << getExprRange(RHSExpr); + return ExprError(); + } + default: + llvm_unreachable("Non-assignment binary operator with braced-init-list " + "should not be parsed"); + } } ExprResult LHS = LHSExpr, RHS = RHSExpr; diff --git a/clang/test/CXX/drs/cwg27xx.cpp b/clang/test/CXX/drs/cwg27xx.cpp index 406c8ea41f3b2..abf669c2c9691 100644 --- a/clang/test/CXX/drs/cwg27xx.cpp +++ b/clang/test/CXX/drs/cwg27xx.cpp @@ -1,10 +1,10 @@ // RUN: %clang_cc1 -triple x86_64-linux-gnu -std=c++98 -pedantic-errors -verify=expected %s -// RUN: %clang_cc1 -triple x86_64-linux-gnu -std=c++11 -pedantic-errors -verify=expected %s -// RUN: %clang_cc1 -triple x86_64-linux-gnu -std=c++14 -pedantic-errors -verify=expected %s -// RUN: %clang_cc1 -triple x86_64-linux-gnu -std=c++17 -pedantic-errors -verify=expected %s -// RUN: %clang_cc1 -triple x86_64-linux-gnu -std=c++20 -pedantic-errors -verify=expected %s -// RUN: %clang_cc1 -triple x86_64-linux-gnu -std=c++23 -pedantic-errors -verify=expected,since-cxx23 %s -// RUN: %clang_cc1 -triple x86_64-linux-gnu -std=c++2c -pedantic-errors -verify=expected,since-cxx23,since-cxx26 %s +// RUN: %clang_cc1 -triple x86_64-linux-gnu -std=c++11 -pedantic-errors -verify=expected,since-cxx11,cxx11-14 %s +// RUN: %clang_cc1 -triple x86_64-linux-gnu -std=c++14 -pedantic-errors -verify=expected,since-cxx11,cxx11-14 %s +// RUN: %clang_cc1 -triple x86_64-linux-gnu -std=c++17 -pedantic-errors -verify=expected,since-cxx11 %s +// RUN: %clang_cc1 -triple x86_64-linux-gnu -std=c++20 -pedantic-errors -verify=expected,since-cxx11 %s +// RUN: %clang_cc1 -triple x86_64-linux-gnu -std=c++23 -pedantic-errors -verify=expected,since-cxx11,since-cxx23 %s +// RUN: %clang_cc1 -triple x86_64-linux-gnu -std=c++2c -pedantic-errors -verify=expected,since-cxx11,since-cxx23,since-cxx26 %s namespace cwg2718 { // cwg2718: 2.7 struct B {}; @@ -101,6 +101,20 @@ static_assert(!__is_layout_compatible(StructWithAnonUnion, StructWithAnonUnion3) #endif } // namespace cwg2759 +namespace cwg2768 { // cwg2768: 20 +#if __cplusplus >= 201103L +enum class E {E1}; + +void f() { + E e; + e = E{0}; // #1 + // cxx11-14-error@-1 {{cannot initialize a value of type 'E' with an rvalue of type 'int'}} + e = {0}; // #2 + // since-cxx11-error@-1 {{cannot initialize a value of type 'E' with an rvalue of type 'int'}} +} +#endif +} // namespace cwg2768 + namespace cwg2789 { // cwg2789: 18 #if __cplusplus >= 202302L template <typename T = int> diff --git a/clang/test/SemaCXX/assign-init-list.cpp b/clang/test/SemaCXX/assign-init-list.cpp new file mode 100644 index 0000000000000..293348092b684 --- /dev/null +++ b/clang/test/SemaCXX/assign-init-list.cpp @@ -0,0 +1,20 @@ +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s + +template<typename T> +void f(T dependent) { + int i; + i = { dependent, dependent }; +} + +template<typename T> +void f2(T dependent) { + int i; + i = { dependent, dependent }; // expected-error {{excess elements in scalar initializer}} +} +template void f2(int); // expected-note {{in instantiation of function template specialization 'f2<int>' requested here}} + +void g() { + int i; + i = {0}; + i += {0}; // expected-error {{initializer list cannot be used on the right hand side of operator '+'}} +} diff --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html index 937f67981e296..359f98c95062b 100755 --- a/clang/www/cxx_dr_status.html +++ b/clang/www/cxx_dr_status.html @@ -16424,7 +16424,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2> <td><a href="https://cplusplus.github.io/CWG/issues/2768.html">2768</a></td> <td>DRWP</td> <td>Assignment to enumeration variable with a <I>braced-init-list</I></td> - <td class="unknown" align="center">Unknown</td> + <td class="unreleased" align="center">Clang 20</td> </tr> <tr class="open" id="2769"> <td><a href="https://cplusplus.github.io/CWG/issues/2769.html">2769</a></td> >From d7c97a71591a1ae1efe552f70f76b3983856a45c Mon Sep 17 00:00:00 2001 From: Mital Ashok <mi...@mitalashok.co.uk> Date: Thu, 25 Jul 2024 17:27:07 +0100 Subject: [PATCH 2/2] Address comments + fix test failure * Use the invented temporary variable for all types (except array types) to support gnu::vector_size * Remove switch over opcodes * Add more tests --- clang/lib/Sema/SemaExpr.cpp | 79 ++++++++++++------------- clang/test/CXX/drs/cwg27xx.cpp | 1 + clang/test/SemaCXX/assign-init-list.cpp | 65 +++++++++++++++++++- 3 files changed, 100 insertions(+), 45 deletions(-) diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 08ee28c108952..143b3d41fea2d 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -14580,54 +14580,49 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc, if (getLangOpts().CPlusPlus11 && isa<InitListExpr>(RHSExpr)) { // C++11 [expr.ass]p9, per CWG2768: // A braced-init-list B may appear on the right-hand side of - // - an assignment to a scalar of type T, in which case the initializer - // list shall have at most a single element. The meaning of x = B is - // x = t, where t is an invented temporary variable declared and - // initialized as T t = B. - switch (Opc) { - case BO_Assign: { - QualType LHSTy = LHSExpr->getType(); - assert(!LHSTy->isDependentType() && - "Should not have tried to create a builtin binary operator"); - if (LHSTy->isScalarType()) { - InitializationKind Kind = - InitializationKind::CreateCopy(RHSExpr->getBeginLoc(), OpLoc); - InitializedEntity Entity = - InitializedEntity::InitializeTemporary(LHSExpr->getType()); - InitializationSequence InitSeq(*this, Entity, Kind, RHSExpr); - ExprResult InventedTemporary = - InitSeq.Perform(*this, Entity, Kind, RHSExpr); - if (InventedTemporary.isInvalid()) - return InventedTemporary; - assert(cast<InitListExpr>(RHSExpr)->getNumInits() <= 1 && - "The initialization should have failed"); - RHSExpr = InventedTemporary.get(); - } - break; - } - case BO_MulAssign: - case BO_DivAssign: - case BO_RemAssign: - case BO_AddAssign: - case BO_SubAssign: - case BO_ShlAssign: - case BO_ShrAssign: - case BO_AndAssign: - case BO_XorAssign: - case BO_OrAssign: { + // - an assignment to a scalar of type T, in which case B shall have at + // most a single element. The meaning of x = B is x = t, where t is an + // invented temporary variable declared and initialized as T t = B. + if (Opc != BO_Assign) { // A compound assignment like `i += {0}` is equivalent to `i = i + {0}`, // which is a parsing error - StringRef Op = BinaryOperator::getOpcodeStr(Opc); - [[maybe_unused]] bool AssignmentStripped = Op.consume_back("="); - assert(AssignmentStripped); + assert(BinaryOperator::isCompoundAssignmentOp(Opc) && + "Non-assignment binary operator with braced-init-list should not " + "be parsed"); Diag(OpLoc, diag::err_init_list_bin_op) - << 1 << Op << getExprRange(RHSExpr); + << 1 + << BinaryOperator::getOpcodeStr( + BinaryOperator::getOpForCompoundAssignment(Opc)) + << getExprRange(RHSExpr); return ExprError(); } - default: - llvm_unreachable("Non-assignment binary operator with braced-init-list " - "should not be parsed"); + + QualType LHSTy = LHSExpr->getType(); + assert(!LHSTy->isDependentType() && + "Should not have tried to create a builtin binary operator"); + // Extend this to be done for non-scalars too. This is so extension types + // like vectors can be assigned to with an initializer list + if (LHSTy->isArrayType()) { + // Except arrays, which we know this will fail for, but fail early to + // prevent trying to convert the initializer list elements to the array + // type + Diag(OpLoc, diag::err_typecheck_array_not_modifiable_lvalue) + << LHSTy << getExprRange(LHSExpr); + return ExprError(); } + InitializationKind Kind = + InitializationKind::CreateCopy(RHSExpr->getBeginLoc(), OpLoc); + InitializedEntity Entity = + InitializedEntity::InitializeTemporary(LHSExpr->getType()); + InitializationSequence InitSeq(*this, Entity, Kind, RHSExpr); + ExprResult InventedTemporary = + InitSeq.Perform(*this, Entity, Kind, RHSExpr); + if (InventedTemporary.isInvalid()) + return InventedTemporary; + // The "at most a single element" condition should be checked by this + // initialization succeeding, but allow multiple initializers for the + // extension type _Complex T / [[gnu::vector_size]] + RHSExpr = InventedTemporary.get(); } ExprResult LHS = LHSExpr, RHS = RHSExpr; diff --git a/clang/test/CXX/drs/cwg27xx.cpp b/clang/test/CXX/drs/cwg27xx.cpp index abf669c2c9691..80e0f72f6af5b 100644 --- a/clang/test/CXX/drs/cwg27xx.cpp +++ b/clang/test/CXX/drs/cwg27xx.cpp @@ -111,6 +111,7 @@ void f() { // cxx11-14-error@-1 {{cannot initialize a value of type 'E' with an rvalue of type 'int'}} e = {0}; // #2 // since-cxx11-error@-1 {{cannot initialize a value of type 'E' with an rvalue of type 'int'}} + e = {E::E1}; } #endif } // namespace cwg2768 diff --git a/clang/test/SemaCXX/assign-init-list.cpp b/clang/test/SemaCXX/assign-init-list.cpp index 293348092b684..8381d8c06bb6f 100644 --- a/clang/test/SemaCXX/assign-init-list.cpp +++ b/clang/test/SemaCXX/assign-init-list.cpp @@ -1,20 +1,79 @@ -// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fenable-matrix -verify %s +// RUN: %clang_cc1 -std=c++23 -fsyntax-only -fenable-matrix -verify %s template<typename T> void f(T dependent) { int i; + i = {}; + i = { dependent }; i = { dependent, dependent }; } template<typename T> void f2(T dependent) { int i; + i = {}; + i = { dependent }; i = { dependent, dependent }; // expected-error {{excess elements in scalar initializer}} } template void f2(int); // expected-note {{in instantiation of function template specialization 'f2<int>' requested here}} void g() { int i; - i = {0}; - i += {0}; // expected-error {{initializer list cannot be used on the right hand side of operator '+'}} + i = {}; + i = { 0 }; + i = { int{} }; + i = { {} }; // expected-warning {{too many braces around scalar initializer}} + i = { { 0 } }; // expected-warning {{too many braces around scalar initializer}} + i += { 0 }; // expected-error {{initializer list cannot be used on the right hand side of operator '+'}} + + auto np = nullptr; + np = {}; + np = { nullptr }; + np = { 0 }; + np = { 1 }; // expected-error {{cannot initialize a value of type 'std::nullptr_t' with an rvalue of type 'int'}} + + void* vp; + vp = {}; + vp = { (void*)nullptr }; + vp = { nullptr }; + vp = { (int*)nullptr }; + vp = { 0 }; + + const void* arr[1] = { nullptr }; + arr = { nullptr }; // expected-error {{array type 'const void *[1]' is not assignable}} + arr = { arr }; // expected-error {{array type 'const void *[1]' is not assignable}} + arr = {}; // expected-error {{array type 'const void *[1]' is not assignable}} + arr = { 1 }; // expected-error {{array type 'const void *[1]' is not assignable}} + + typedef int i1_t [[gnu::vector_size(sizeof(int))]]; + i1_t i1; + i1 = {}; + i1 = { 0 }; + + typedef int i4_t [[gnu::vector_size(4*sizeof(int))]]; + i4_t i4; + i4 = {}; + i4 = { 0, 1, 2, 3 }; + + // TODO: when matrix_type initialization is specified, try to + // make these work/not work as required +#if 0 + typedef double [[clang::matrix_type(1,1)]] d11_t; + d11_t d11; + d11 = {}; + d11 = { 0.0 }; + d11 = { { 0.0 } }; + + typedef double [[clang::matrix_type(2,2)]] d22_t; + d22_t d22; + d22 = {}; + d22 = { 0.0 }; + d22 = { { 0.0, 1.0 }, { 2.0, 3.0 } }; +#endif + + double _Complex dc; + dc = {}; + dc = { 0.0 }; + dc = { 0.0, 0.0 }; } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits