Jim M. R. =?utf-8?q?Teichgräber?=,Jim M. R. =?utf-8?q?Teichgräber?=,
Jim M. R. =?utf-8?q?Teichgräber?=,Jim M. R. =?utf-8?q?Teichgräber?Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/91...@github.com>


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Jim M. R. Teichgräber (J-MR-T)

<details>
<summary>Changes</summary>

C99-C23 6.5.2.5 says: The type name shall specify an object type or an array of 
unknown size, but not a variable length array type.

Closes issue #<!-- -->89835 .

I `git clang-format`'ed my changes and ran the clang tests, which passed after 
minor adjustments to the new behavior.

I mostly implemented everything as discussed in #<!-- -->89835, with the 
exception of 
[clang/test/C/C2x/n2900_n3011_2.c](https://github.com/llvm/llvm-project/compare/main...J-MR-T:llvm-project:main#diff-aa2b77ff42fcd2f2c02cc456f5023e923d44e5018dd2af7c046ebdcc8cb00a4a):
 in that file I deleted the part about VLA compound literals, because the file 
relies on LLVM-IR being emitted by clang, and checking for diagnostics here 
wouldn't have fit with the rest of the file. Adding another RUN line and 
handling this case independently in that file would be an option, but it felt 
out of place. Of course, I'm still open to doing it that way, or preserving the 
test another way, if that is preferred.

As I point out 
[here](https://github.com/llvm/llvm-project/compare/main...J-MR-T:llvm-project:main#diff-3c28567b5e0c77d68f174541a0b77f5a85d093f58b89cd3675ee04a550a44880R7278-R7283),
 the new behavior leads to a confusing reality brought about by the C23 
standard:
```c
int a[size] = {};
```
is valid code, while
```c
int a[size] = (int[size]){};
```
is not. As this seems to be what the standard requests, I implemented it that 
way for now.


Tagging @<!-- -->Sirraide and @<!-- -->AaronBallman as requested :).



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


6 Files Affected:

- (modified) clang/docs/ReleaseNotes.rst (+3) 
- (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+2) 
- (modified) clang/lib/Sema/SemaExpr.cpp (+13-6) 
- (modified) clang/test/C/C2x/n2900_n3011.c (+7-1) 
- (modified) clang/test/C/C2x/n2900_n3011_2.c (-16) 
- (modified) clang/test/Sema/compound-literal.c (+12-1) 


``````````diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 7c5dcc59c7016..1ffd29da0cba9 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -563,6 +563,9 @@ Bug Fixes in This Version
 - Clang will no longer emit a duplicate -Wunused-value warning for an 
expression
   `(A, B)` which evaluates to glvalue `B` that can be converted to non 
ODR-use. (#GH45783)
 
+- Clang now correctly disallows VLA type compound literals, e.g. 
``(int[size]){}``,
+  as the C standard mandates. (#GH89835)
+
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index d6863f90edb6e..25d925da7b748 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3371,6 +3371,8 @@ def err_field_with_address_space : Error<
   "field may not be qualified with an address space">;
 def err_compound_literal_with_address_space : Error<
   "compound literal in function scope may not be qualified with an address 
space">;
+def err_compound_literal_with_vla_type : Error<
+  "compound literal cannot be of variable-length array type">;
 def err_address_space_mismatch_templ_inst : Error<
   "conflicting address space qualifiers are provided between types %0 and %1">;
 def err_attr_objc_ownership_redundant : Error<
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index c688cb21f2364..e62e3f3285e5d 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -7274,12 +7274,19 @@ Sema::BuildCompoundLiteralExpr(SourceLocation 
LParenLoc, TypeSourceInfo *TInfo,
       // init a VLA in C++ in all cases (such as with non-trivial 
constructors).
       // FIXME: should we allow this construct in C++ when it makes sense to do
       // so?
-      std::optional<unsigned> NumInits;
-      if (const auto *ILE = dyn_cast<InitListExpr>(LiteralExpr))
-        NumInits = ILE->getNumInits();
-      if ((LangOpts.CPlusPlus || NumInits.value_or(0)) &&
-          !tryToFixVariablyModifiedVarType(TInfo, literalType, LParenLoc,
-                                           diag::err_variable_object_no_init))
+      //
+      // But: C99-C23 6.5.2.5 Compound literals constraint 1: The type name
+      // shall specify an object type or an array of unknown size, but not a
+      // variable length array type. This seems odd, as it allows int a[size] =
+      // {}; but forbids int a[size] = (int[size]){}; As this is what the
+      // standard says, this is what's implemented here for C (except for the
+      // extension that permits constant foldable size arrays)
+
+      auto diagID = LangOpts.CPlusPlus
+                        ? diag::err_variable_object_no_init
+                        : diag::err_compound_literal_with_vla_type;
+      if (!tryToFixVariablyModifiedVarType(TInfo, literalType, LParenLoc,
+                                           diagID))
         return ExprError();
     }
   } else if (!literalType->isDependentType() &&
diff --git a/clang/test/C/C2x/n2900_n3011.c b/clang/test/C/C2x/n2900_n3011.c
index 4350aa140691b..82a3b16c8acda 100644
--- a/clang/test/C/C2x/n2900_n3011.c
+++ b/clang/test/C/C2x/n2900_n3011.c
@@ -27,8 +27,14 @@ void test(void) {
                               compat-warning {{use of an empty initializer is 
incompatible with C standards before C23}}
   int vla[i] = {}; // compat-warning {{use of an empty initializer is 
incompatible with C standards before C23}} \
                       pedantic-warning {{use of an empty initializer is a C23 
extension}}
+  // C99 6.5.2.5 Compound literals constraint 1: The type name shall specify an
+  // object type or an array of unknown size, but not a variable length array
+  // type.
   int *compound_literal_vla = (int[i]){}; // compat-warning {{use of an empty 
initializer is incompatible with C standards before C23}} \
-                                             pedantic-warning {{use of an 
empty initializer is a C23 extension}}
+                                             pedantic-warning {{use of an 
empty initializer is a C23 extension}}\
+                                             compat-error {{compound literal 
cannot be of variable-length array type}} \
+                                             pedantic-error {{compound literal 
cannot be of variable-length array type}}\
+
 
   struct T {
        int i;
diff --git a/clang/test/C/C2x/n2900_n3011_2.c b/clang/test/C/C2x/n2900_n3011_2.c
index eb15fbf905c86..ab659d636d155 100644
--- a/clang/test/C/C2x/n2900_n3011_2.c
+++ b/clang/test/C/C2x/n2900_n3011_2.c
@@ -76,22 +76,6 @@ void test_zero_size_vla() {
   // CHECK-NEXT: call void @llvm.memset.p0.i64(ptr {{.*}} %[[VLA]], i8 0, i64 
%[[BYTES_TO_COPY]], i1 false)
 }
 
-void test_compound_literal_vla() {
-  int num_elts = 12;
-  int *compound_literal_vla = (int[num_elts]){};
-  // CHECK: define {{.*}} void @test_compound_literal_vla
-  // CHECK-NEXT: entry:
-  // CHECK-NEXT: %[[NUM_ELTS_PTR:.+]] = alloca i32
-  // CHECK-NEXT: %[[COMP_LIT_VLA:.+]] = alloca ptr
-  // CHECK-NEXT: %[[COMP_LIT:.+]] = alloca i32
-  // CHECK-NEXT: store i32 12, ptr %[[NUM_ELTS_PTR]]
-  // CHECK-NEXT: %[[NUM_ELTS:.+]] = load i32, ptr %[[NUM_ELTS_PTR]]
-  // CHECK-NEXT: %[[NUM_ELTS_EXT:.+]] = zext i32 %[[NUM_ELTS]] to i64
-  // CHECK-NEXT: %[[BYTES_TO_COPY:.+]] = mul nuw i64 %[[NUM_ELTS_EXT]], 4
-  // CHECK-NEXT: call void @llvm.memset.p0.i64(ptr {{.*}} %[[COMP_LIT]], i8 0, 
i64 %[[BYTES_TO_COPY]], i1 false)
-  // CHECK-NEXT: store ptr %[[COMP_LIT]], ptr %[[COMP_LIT_VLA]]
-}
-
 void test_nested_structs() {
   struct T t1 = { 1, {} };
   struct T t2 = { 1, { 2, {} } };
diff --git a/clang/test/Sema/compound-literal.c 
b/clang/test/Sema/compound-literal.c
index a64b6f9e5dfa4..3ed53d670d38f 100644
--- a/clang/test/Sema/compound-literal.c
+++ b/clang/test/Sema/compound-literal.c
@@ -29,7 +29,7 @@ int main(int argc, char **argv) {
 struct Incomplete; // expected-note{{forward declaration of 'struct 
Incomplete'}}
 struct Incomplete* I1 = &(struct Incomplete){1, 2, 3}; // expected-error 
{{variable has incomplete type}}
 void IncompleteFunc(unsigned x) {
-  struct Incomplete* I2 = (struct foo[x]){1, 2, 3}; // expected-error 
{{variable-sized object may not be initialized}}
+  struct Incomplete* I2 = (struct foo[x]){1, 2, 3}; // expected-error 
{{compound literal cannot be of variable-length array type}}
   (void){1,2,3}; // expected-error {{variable has incomplete type}}
   (void(void)) { 0 }; // expected-error{{illegal initializer type 'void 
(void)'}}
 }
@@ -42,3 +42,14 @@ int (^block)(int) = ^(int i) {
   int *array = (int[]) {i, i + 2, i + 4};
   return array[i];
 };
+
+// C99 6.5.2.5 Compound literals constraint 1: The type name shall specify an 
object type or an array of unknown size, but not a variable length array type.
+// So check that VLA type compound literals are rejected (see 
https://github.com/llvm/llvm-project/issues/89835).
+void vla(int n) {
+  int size = 5;
+  (void)(int[size]){}; // expected-warning {{use of an empty initializer is a 
C23 extension}}
+                       // expected-error@-1 {{compound literal cannot be of 
variable-length array type}}
+  (void)(int[size]){1}; // expected-error {{compound literal cannot be of 
variable-length array type}}
+  (void)(int[size]){1,2,3}; // expected-error {{compound literal cannot be of 
variable-length array type}}
+  (void)(int[size]){1,2,3,4,5}; // expected-error {{compound literal cannot be 
of variable-length array type}}
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/91891
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to