[C23] Fix compound literals within function prototype #132097
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WG14 N2819 clarified that a compound literal within a function prototype has a lifetime similar to that of a local variable within the function, not a file scope variable.
AaronBallman
added
clang
AaronBallman
requested review from
zygoloid,
jyknight,
erichkeane,
tbaederr and
efriedma-quic
llvmbot
added
the
clang:frontend
|
@llvm/pr-subscribers-clang-codegen @llvm/pr-subscribers-clang Author: Aaron Ballman (AaronBallman) ChangesWG14 N2819 clarified that a compound literal within a function prototype has a lifetime similar to that of a local variable within the function, not a file scope variable. Full diff: https://github.com/llvm/llvm-project/pull/132097.diff 5 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 666bbf22acc93..768ba480a7ee7 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -148,6 +148,10 @@ C23 Feature Support
better diagnostic behavior for the ``va_start()`` macro in C23 and later.
This also updates the definition of ``va_start()`` in ``<stdarg.h>`` to use
the new builtin. Fixes #GH124031.
+- Implemented ``WG14 N2819 <https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2819.pdf`_
+ which clarified that a compound literal used within a function prototype is
+ treated as if the compound literal were within the body rather than at file
+ scope.
Non-comprehensive list of changes in this release
-------------------------------------------------
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index a03acb5fbd273..fca3a2e8c19af 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -7141,7 +7141,13 @@ Sema::BuildCompoundLiteralExpr(SourceLocation LParenLoc, TypeSourceInfo *TInfo,
return ExprError();
LiteralExpr = Result.get();
- bool isFileScope = !CurContext->isFunctionOrMethod();
+ // We treat the compound literal as being at file scope if it's not in a
+ // function or method body, or within the function's prototype scope. This
+ // means the following compound literal is not at file scope:
+ // void func(char *para[(int [1]){ 0 }[0]);
+ const Scope *S = getCurScope();
+ bool isFileScope = !CurContext->isFunctionOrMethod() &&
+ (!S || !S->isFunctionPrototypeScope());
// In C, compound literals are l-values for some reason.
// For GCC compatibility, in C++, file-scope array compound literals with
diff --git a/clang/test/AST/ByteCode/literals.cpp b/clang/test/AST/ByteCode/literals.cpp
index f206f020ecb47..a9b787892eff2 100644
--- a/clang/test/AST/ByteCode/literals.cpp
+++ b/clang/test/AST/ByteCode/literals.cpp
@@ -849,13 +849,11 @@ namespace CompoundLiterals {
}
static_assert(get5() == 5, "");
- constexpr int get6(int f = (int[]){1,2,6}[2]) { // ref-note {{subexpression not valid in a constant expression}} \
- // ref-note {{declared here}}
+ constexpr int get6(int f = (int[]){1,2,6}[2]) {
return f;
}
static_assert(get6(6) == 6, "");
- // FIXME: Who's right here?
- static_assert(get6() == 6, ""); // ref-error {{not an integral constant expression}}
+ static_assert(get6() == 6, "");
constexpr int x = (int){3};
static_assert(x == 3, "");
diff --git a/clang/test/C/C23/n2819.c b/clang/test/C/C23/n2819.c
index c428fbba80245..eb0f64a69f375 100644
--- a/clang/test/C/C23/n2819.c
+++ b/clang/test/C/C23/n2819.c
@@ -1,4 +1,4 @@
-// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --version 3
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --version 5
// RUN: %clang_cc1 -triple=x86_64 -emit-llvm -o - -std=c23 %s | FileCheck %s
/* WG14 N2819: No
@@ -6,9 +6,10 @@
*/
int *escaped;
+
// CHECK-LABEL: define dso_local i32 @f(
// CHECK-SAME: ptr noundef [[PTR:%.*]]) #[[ATTR0:[0-9]+]] {
-// CHECK-NEXT: entry:
+// CHECK-NEXT: [[ENTRY:.*:]]
// CHECK-NEXT: [[PTR_ADDR:%.*]] = alloca ptr, align 8
// CHECK-NEXT: store ptr [[PTR]], ptr [[PTR_ADDR]], align 8
// CHECK-NEXT: [[TMP0:%.*]] = load ptr, ptr [[PTR_ADDR]], align 8
@@ -19,23 +20,23 @@ int f(int *ptr) { escaped = ptr; return 1; }
// CHECK-LABEL: define dso_local i32 @g(
// CHECK-SAME: ptr noundef [[PARA:%.*]]) #[[ATTR0]] {
-// CHECK-NEXT: entry:
+// CHECK-NEXT: [[ENTRY:.*:]]
// CHECK-NEXT: [[PARA_ADDR:%.*]] = alloca ptr, align 8
+// CHECK-NEXT: [[DOTCOMPOUNDLITERAL:%.*]] = alloca [27 x i32], align 4
// CHECK-NEXT: store ptr [[PARA]], ptr [[PARA_ADDR]], align 8
-// CHECK-NEXT: [[CALL:%.*]] = call i32 @f(ptr noundef @.compoundliteral)
+// CHECK-NEXT: call void @llvm.memset.p0.i64(ptr align 4 [[DOTCOMPOUNDLITERAL]], i8 0, i64 108, i1 false)
+// CHECK-NEXT: [[ARRAYDECAY:%.*]] = getelementptr inbounds [27 x i32], ptr [[DOTCOMPOUNDLITERAL]], i64 0, i64 0
+// CHECK-NEXT: [[CALL:%.*]] = call i32 @f(ptr noundef [[ARRAYDECAY]])
// CHECK-NEXT: [[TMP0:%.*]] = zext i32 [[CALL]] to i64
// CHECK-NEXT: ret i32 0
//
-// FIXME: notice the we are using the global .compoundliteral object, not
-// allocating a new object on each call to g(). That's what was clarified by
-// N2819.
int g(char *para [f(( int [27]) { 0 })]) {
return 0;
}
// CHECK-LABEL: define dso_local i32 @main(
// CHECK-SAME: ) #[[ATTR0]] {
-// CHECK-NEXT: entry:
+// CHECK-NEXT: [[ENTRY:.*:]]
// CHECK-NEXT: [[RETVAL:%.*]] = alloca i32, align 4
// CHECK-NEXT: store i32 0, ptr [[RETVAL]], align 4
// CHECK-NEXT: [[CALL:%.*]] = call i32 @g(ptr noundef null)
diff --git a/clang/www/c_status.html b/clang/www/c_status.html
index 7cf50bfdb6639..c9e2eda4304f3 100644
--- a/clang/www/c_status.html
+++ b/clang/www/c_status.html
@@ -715,7 +715,7 @@ <h2 id="c2x">C23 implementation status</h2>
<tr>
<td>Disambiguate the storage class of some compound literals</td>
<td><a href="https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2819.pdf">N2819</a></td>
- <td class="none" align="center">No</td>
+ <td class="unreleased" align="center">Clang 21</td>
</tr>
<tr>
<td>Add annotations for unreachable control flow v2</td>
|
erichkeane
approved these changes
Mar 19, 2025
| @@ -849,13 +849,11 @@ namespace CompoundLiterals { | |||
| } | |||
| static_assert(get5() == 5, ""); | |||
|
|
|||
| constexpr int get6(int f = (int[]){1,2,6}[2]) { // ref-note {{subexpression not valid in a constant expression}} \ | |||
| // ref-note {{declared here}} | |||
| constexpr int get6(int f = (int[]){1,2,6}[2]) { | |||
There was a problem hiding this comment.
Maybe also worth testing a few more interactions with C++ constexpr:
constexpr int* f(int*a=(int[]){1,2,3}) {return a;}
constinit int* a1 = f(); // error: pointer points to local variable
static_assert(f()[0] == 1); // Should pass.
constexpr int f2(int *x, int (*y)[*(x=(int[]){1,2,3})]) {
return x[0];
}
constexpr int g = f2(0,0); // Should evaluate to 1?
There was a problem hiding this comment.
Thanks, I'll add more tests.
constinit int* a1 = f(); // error: pointer points to local variable
Yes, this should fail (and it does).
constexpr int f2(int *x, int (*y)[*(x=(int[]){1,2,3})]) {
You are a bad person who should feel bad. :-D I do think that should evaluate to 1 (but only when calling f2(0)), but that's a pile of VLA code which we don't always handle well, especially in C++. I think a non-VLA example that's similar would be:
constexpr int f2(int *x =(int[]){1,2,3}) {
return x[0];
}
constexpr int g = f2(); // Should evaluate to 1?
static_assert(g == 1, "");
which does behave how you'd expect.
The VLA example says object backing the pointer x will be destroyed at the end of the full-expression which may be a C++'ism not impacted by the C wording?
There was a problem hiding this comment.
With the VLA example, there are a few different weird interactions here... but the primary silly thing I'm doing is abusing VLAs to modify argument values. So, for example:
constexpr int f2(int x, int (*y)[(x=3)]) { return x; }
static_assert(f2(0,0)==3);
On top of that, we have to consider the question of the lifetime of compound literals defined inside the array bound.
But maybe we should just reject this sort of thing in C++ more aggressively, like gcc does, so we don't have to figure out all these weird cases.
There was a problem hiding this comment.
With the VLA example, there are a few different weird interactions here... but the primary silly thing I'm doing is abusing VLAs to modify argument values.
Yeah, which we already seem to get wrong: https://godbolt.org/z/Y4xEGWhM3
On top of that, we have to consider the question of the lifetime of compound literals defined inside the array bound.
I think that's what object backing the pointer x will be destroyed at the end of the full-expression is about in your previous VLA example. There is an ExprWithCleanups node for the assignment operator and that is a FullExpr. So I think we do get that correct -- I'll add it as a test case with comments explaining why it's rejected.
But maybe we should just reject this sort of thing in C++ more aggressively, like gcc does, so we don't have to figure out all these weird cases.
I tend to agree we should be far more restrictive with VLAs in C++ than we are today. But I also think that's a different PR than this one. WDYT?
tbaederr
reviewed
Mar 20, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/30/builds/18034 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
WG14 N2819 clarified that a compound literal within a function prototype has a lifetime similar to that of a local variable within the function, not a file scope variable.