-
Notifications
You must be signed in to change notification settings - Fork 143
Fix array compound literal parsing #309
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Implement proper array compound literal handling that emits element writes, counts initializers, and returns the temporary array pointer instead of collapsing to the first element. This restores correct pointer semantics and avoids discarding array literals during parsing. Struct and scalar compound literals are unchanged. The parser now tracks whether the closing brace was already consumed by the array helper to prevent double reads.
Introduce helpers to centralize array-literal scalar decay. Temporary array compound literals are detected and converted to a scalar only when a scalar is actually required; otherwise the expression yields the temporary array’s address, preserving pointer semantics. Update binary ops, direct/compound assignments, function-call arguments, and ternary results to use the unified helper instead of ad-hoc collapsing. This fixes cases where array literals were reduced to their first element in pointer contexts, while keeping struct and plain scalar behavior unchanged. Addresses sysprog21#299 (array compound literals).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 issues found across 1 file
Prompt for AI agents (all 2 issues)
Understand the root cause of the following 2 issues and fix them.
<file name="src/parser.c">
<violation number="1" location="src/parser.c:1653">
Passing an array compound literal to a variadic function now yields the first element value instead of the temporary array’s address, so variadic calls like `printf("%p", (int[]){1,2});` receive an integer instead of a pointer.</violation>
<violation number="2" location="src/parser.c:2901">
Array compound literals are scalarized before binary pointer arithmetic; e.g. `(int[]){1,2} + 1` now produces the integer `2` instead of a pointer to the second element.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai
to give feedback, ask questions, or re-run the review.
param = scalarize_array_literal(parent, bb, param, | ||
target->type); | ||
} else if (func->va_args) { | ||
param = scalarize_array_literal(parent, bb, param, TY_int); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Passing an array compound literal to a variadic function now yields the first element value instead of the temporary array’s address, so variadic calls like printf("%p", (int[]){1,2});
receive an integer instead of a pointer.
Prompt for AI agents
Address the following comment on src/parser.c at line 1653:
<comment>Passing an array compound literal to a variadic function now yields the first element value instead of the temporary array’s address, so variadic calls like `printf("%p", (int[]){1,2});` receive an integer instead of a pointer.</comment>
<file context>
@@ -1575,7 +1643,16 @@ void read_func_parameters(func_t *func, block_t *parent, basic_block_t **bb)
+ param = scalarize_array_literal(parent, bb, param,
+ target->type);
+ } else if (func->va_args) {
+ param = scalarize_array_literal(parent, bb, param, TY_int);
+ }
+ }
</file context>
✅ Addressed in d6b9fbf
} | ||
lex_expect(T_close_curly); | ||
var->array_size = count; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a new blank line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add some test cases to the test suite for validation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't use static
qualifier since shecc does not support. Check 'COMPLIANCE.md' carefully.
You MUST ensure bootstrapping is fully functional before submitting pull requests. |
IIUC, compound literals are a feature supported only since C99, and the shecc README mentions from the very beginning that this project aims to support ANSI C. Therefore, IMO, this at least does not "fix" anything. |
As far as I know, shecc is planned to fully support the C99 standard, so I think the term "fix" is acceptable. |
Fine, but since we haven't claimed to fully support C99 and, IIUC, array compound literals were never supported before, this seems more like supporting a new feature to me, rather than fixing an existing problem. |
In fact, shecc has ability to handle array compound literals, but it only captures the first element (#299). Therefore, this pull request specifically aims to fix it. |
Thanks, that resolves my doubt. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't add tests/array_ptr.c
. Instead, consolidate tests/driver.sh
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rebase your branch to keep the git history clean.
Commits that fix problems introduced within the same pull request should be avoided.
|
||
add_insn(parent, *bb, OP_read, scalar, array_var, NULL, elem_size, NULL); | ||
return scalar; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a line break.
Summary
Fix a segfault when evaluating array compound literals like
(int[]){1,2,3,4,5}
in expression context. The literal now yields the temporary array’s address (via decay) instead of collapsing to a single element, so indexing and reads work correctly.Motivation
Previously, code like the snippet below crashed because the array literal was reduced to a scalar and later treated as a pointer.
Reproduction (manual)
Before: segfault
After: prints
a[0..4]
and Sum = 6 as expectedApproach (high level)
(type[]){…}
produces a real temporary array object and the expression value decays to its address.Scope
Tests
Compatibility
Issue
Summary by cubic
Fix parsing of array compound literals so they allocate a temporary array and decay to its address, preserving pointer semantics and preventing segfaults.
Bug Fixes
Refactors