Fix array compound literal parsing #309

lorettayao · 2025-10-06T13:22:56Z

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)

#include <stdio.h>

int main(void) {
    int *a = (int[]){1,2,3,4,5};   // used to crash later
    printf("Testing basic array compound literal:\n");
    for (int i = 0; i < 5; i++)
        printf("  a[%d] = %d\n", i, a[i]);

    int sum = a[0] + a[4];
    printf("Sum = %d (expect 6)\n", sum);
    return 0;
}

Before: segfault
After: prints a[0..4] and Sum = 6 as expected

Approach (high level)

Ensure (type[]){…} produces a real temporary array object and the expression value decays to its address.
Only collapse to a scalar when a scalar is actually required by context.
No changes to struct compound literals or plain scalars.

Scope

This PR addresses the array side of Fix compound literals to capture all field values, not just first #299 sufficient to eliminate the segfault in basic use.
Struct compound literals and broader initializer behaviors are intentionally out of scope here.

Tests

No test files are added in this PR. The manual snippet above demonstrates the fix.

Compatibility

Restores standard pointer semantics for array compound literals.
No behavioral changes for structs or scalars.

Issue

Partially addresses Fix compound literals to capture all field values, not just first #299 (array compound literals).

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
- Eliminates crashes when using (int[]){...} in expressions, assignments, and parameter passing.
- Correct behavior across binary ops, pointer arithmetic, ternary results, and function-call (incl. variadic) arguments.
- Zero-length array literals yield 0 for scalar uses to avoid garbage reads.
- Addresses Fix compound literals to capture all field values, not just first #299.
Refactors
- Added parse_array_compound_literal and scalarize_array_literal to centralize handling; scalarize only when a scalar is required.
- Emits element writes and counts initializers; tracks brace consumption to avoid double reads.
- Struct and scalar compound literals remain unchanged.

^{Written for commit 3e1f41a. Summary will update automatically on new commits.}

jserv · 2025-10-06T14:02:23Z

src/parser.c

+    }
+    lex_expect(T_close_curly);
+    var->array_size = count;
+}


Add a new blank line.

Add a blank line.

DrXiao

Add some test cases to the test suite for validation.

jserv

Don't use static qualifier since shecc does not support. Check 'COMPLIANCE.md' carefully.

src/parser.c

jserv · 2025-10-06T14:07:42Z

You MUST ensure bootstrapping is fully functional before submitting pull requests.

visitorckw · 2025-10-06T14:34:14Z

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.

DrXiao · 2025-10-06T14:49:42Z

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.

visitorckw · 2025-10-06T14:53:37Z

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.

DrXiao · 2025-10-06T15:10:25Z

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.

visitorckw · 2025-10-06T15:21:20Z

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.
However, should we also mention the issue in the first commit?

src/parser.c

jserv

Don't add tests/array_ptr.c. Instead, consolidate tests/driver.sh.

visitorckw

Please rebase your branch to keep the git history clean.
Commits that fix problems introduced within the same pull request should be avoided.

src/parser.c

cubic-dev-ai · 2025-11-07T12:08:23Z

Looking at the changes, the core logic for handling array compound literals looks solid—good job centralizing the decay behavior to avoid ad-hoc fixes everywhere. But yeah, there are some style inconsistencies and comment opportunities that could make this even tighter. Here's what I'd suggest, focused on parser.c (the bulk of the changes) and the test additions. I'll keep it practical: specific line tweaks for consistency with the rest of the codebase (e.g., brace placement, naming, spacing) and punchier comments.

Style Fixes for `parser.c`

Brace placement and indentation in new functions:

In parse_array_compound_literal, the if (!lex_peek(T_close_curly, NULL)) block has inconsistent indentation for the inner for loop—align it to match the function's style (2-space indents). Also, add braces around single-statement blocks for consistency (e.g., the count++ line).

if (!lex_peek(T_close_curly, NULL)) {
    for (;;) {
        read_expr(parent, bb);
        read_ternary_operation(parent, bb);
        var_t *value = opstack_pop();
        if (count == 0)
            var->init_val = value->init_val;
        if (emit_code) {
            // ... (existing code)
        }
        count++;  // Add braces if expanding later
        if (!lex_accept(T_comma))
            break;
        if (lex_peek(T_close_curly, NULL))
            break;
    }
}

This matches the style in read_expr and avoids future bugs.

Variable naming consistency:
- vd is overused as a generic var_t pointer—rename to something descriptive in read_ternary_operation (e.g., result_var for the final vd). It's clear in context, but vd feels like a holdover from declarations elsewhere.
```
var_t *result_var = require_var(parent);
gen_name_to(result_var->var_name);
add_insn(parent, then_, OP_assign, result_var, true_val, NULL, 0, NULL);
add_insn(parent, else_, OP_assign, result_var, false_val, NULL, 0, NULL);
// ...
result_var->is_ternary_ret = true;
opstack_push(result_var);
```
- Similarly, in read_body_assignment, rhs_val and rhs are redundant—use rhs consistently and drop the intermediate.
Spacing and operator alignment:
- In is_array_literal_placeholder, tighten the return condition: return var && var->array_size > 0 && !var->ptr_level && var->var_name[0] == '.';
- In binary op handling (around line 2908), the new rs1_is_ptr_like checks have good logic but uneven spacing—align the bool defs:
```
bool rs1_is_ptr_like = rs1 && (rs1->ptr_level || rs1->array_size);
bool rs2_is_ptr_like = rs2 && (rs2->ptr_level || rs2->array_size);
```
- Remove extra blank lines after function defs (e.g., after scalarize_array_literal) to match the compact style in the original parser.
Minor: Unused params and locals:
- In scalarize_array_literal, hint_type defaults to TY_int if invalid, but the null check for elem_type could be simplified: type_t *elem_type = hint_type ? hint_type : array_var->type ? array_var->type : TY_int;. Drops the nested if.
- var->init_val = 0; in a few spots (e.g., zero-length handling) is fine, but ensure it's set consistently—it's already good, just a nit.

Comment Improvements

The inline comments are mostly helpful, but some are wordy or could be more precise. Aim for concise, factual notes that explain why not just what.

Update the placeholder comment (around line 1364):
- Current: "Identify compiler-emitted temporaries that hold array compound literals. Parsing assigns these temporaries synthetic names via gen_name_to (".tN") and they keep array metadata without pointer indirection."
- Suggested: "Detect compiler temporaries ('.tN' names) holding array compound literals. These retain array metadata (size/type) without decaying to pointers until needed."
  - Shorter, focuses on the key invariant (no auto-decay).

Add a brief header comment to parse_array_compound_literal:

/* Parse array compound literal contents: emit element stores to a temporary array.
 * Counts initializers and consumes the closing brace. Used only for array types. */
void parse_array_compound_literal(var_t *var, /* ... */)

Explains purpose without repeating the PR description.

Clarify scalarization calls:
- In function params (around line 1650), add: // Decay array literals to scalars for non-pointer params, per C semantics.
- In ternary handling: // Scalarize array literals if the other branch isn't pointer-like, to match types.
- In assignment: // Decay RHS array literal to scalar if target is non-pointer (e.g., int x = (int[]){1}; uses first elem).
  - These tie back to the motivation (avoiding scalar collapse in pointer contexts) without verbosity.
Zero-length handling (in read_expr_operand):
- Current implicit via if (compound_var->array_size == 0). Add: // Zero-length arrays default to 0 constant (avoids garbage reads in scalar contexts).

For Tests in `driver.sh`

The new tests are great for coverage, but add a comment block at the top of the array sections:
```
# Array compound literal tests: verify decay to pointer in assignments and args
```
In the sum test, the 0, 0 padding is clever for partial init, but add an expected output comment: # Expects sum=6 from first 3 elements.
Consistent with existing tests: Use try_ 0 everywhere (you did), and keep EOF indents clean.

These tweaks keep the code readable and maintainable without overhauling anything. Total changes: ~20 lines. Run a style check (assuming you have one) after, and it'll align nicely with the rest of the parser. If you push an updated commit, I can re-review the diffs.

jserv · 2025-11-14T08:27:22Z

I defer to @ChAoSUnItY for confirmation.

ChAoSUnItY · 2025-11-14T16:09:47Z

src/arm-codegen.c

            fatal("Unsupported truncation operation with invalid target size");
        }
        return;
-    case OP_sign_ext: {


Why removing the curly braces while this case branch has variable declaration?

I didn’t really mean to drop those braces—they were just left behind while trying other tweaks. but it’s fine either way because C99 lets us declare source_size right after the case label without changing behavior.

Yes, although C99 allows this. But since in our compiler's standard workflow, stage 0 will be compiled by GCC with -Wpedantic compilation option included, and this will cause building process to output warning even in this case it didn't do anything in a harmful way.

I reverted it to the original order in my latest commit

src/parser.c

lorettayao · 2025-11-25T08:35:02Z

@jserv I have rebased to master, and I saw my branch is up to date with 'origin/master'. I am confused if the conflict is because of the rebase issue.

visitorckw · 2025-11-25T08:46:46Z

I guess your origin points to your own repository on github instead of the upstream repository under sysprog21?

lorettayao · 2025-11-25T08:51:00Z

I guess your origin points to your own repository on github instead of the upstream repository under sysprog21?

yes! Thank you for pointing out!

jserv

Use git rebase -i to rework commits, squashing intermediate ones.

DrXiao · 2025-11-27T14:30:00Z

@lorettayao, note that the commit message body should be wrapped at 72 characters.

Unify and correct the handling of array compound literals across parsing, semantic analysis, and lowering. The compiler now builds a temporary array by writing each element, tracking initializer counts, and returning the array’s address instead of collapsing the literal to its first element. Decay to a scalar is performed only when a scalar value is required. A centralized helper now governs literal decay, replacing scattered ad-hoc callers in binary operators, assignments, function-call arguments, and ternary expressions. This resolves cases where array literals were incorrectly forced to scalars in pointer contexts and brings the behavior closer to standard C expectations. The update also corrects scalarization in variadic and pointer arithmetic contexts, ensures pointer-typed ternary results, handles zero-length array literals as constant zero, avoids double-brace consumption in the parser, and regenerates ARM and RISC-V snapshots to match the corrected lowering. These changes restore correct pointer semantics for array compound literals and address sysprog21#299.

DrXiao · 2025-11-30T14:49:32Z

src/parser.c

+void parse_array_compound_literal(var_t *var,
+                                  block_t *parent,
+                                  basic_block_t **bb,
+                                  bool emit_code);


I think this forward declaration is unnecessary here.

Yes, I found it unnecessary, will remove

DrXiao · 2025-11-30T14:49:50Z

src/parser.c

+    }
+    lex_expect(T_close_curly);
+    var->array_size = count;
+}


Add a blank line.

src/parser.c

DrXiao

The second commit (a4aba54) modified both the C source code and the test suites, but its message only describes the latter. The message does not accurately reflect the changes made.

src/parser.c

DrXiao

I notice that the term "decay" appears in the commit messages, but I am not sure why it is used. Could you explain the reason behind this choice?

DrXiao

Notice that a blank line is required between two function definitions. For example:

Correct example:

void func1(void)
{
    /* ... */
}

/* (Comment)
 * ...
 * ...
 */
int func2(void)
{
    /* ... */
}

void func3(int a)
{
    /* ... */
}

Incorrect example:

void func1(void)
{
    /* ... */
}
/* (Comment)
 * ...
 * ...
 */
int func2(void)
{
    /* ... */
}
void func3(int a)
{
    /* ... */
}

lorettayao · 2025-12-23T10:51:30Z

I notice that the term "decay" appears in the commit messages, but I am not sure why it is used. Could you explain the reason behind this choice?

i use the word decay to specify fixes around treating array compound literals this way—e.g., assigning (int[]){1,2,3} to an int * or passing it to a function should transparently yield a pointer

Add regression tests that verify pointer and decay semantics of array compound literals. The tests cover decay during initialization, passing array literals to functions that expect pointer arguments, pointer-typed ternary expressions involving literal branches, and correctness for char[] and short[] literals in simple computations. These tests confirm the behavior introduced by the refined compound literal semantics implemented in the preceding commit. Parser cleanup: flattened the nested parameter-handling conditionals in parser.c so array-literal parameters are scalarized and promoted/resized without deep if-nesting, improving readability while preserving the decay semantics enforced by the new tests

jserv

The work touches too many places with similar boilerplate:

  if (!var->ptr_level && var->array_size == 0)
      rhs = scalarize_array_literal(parent, &bb, rhs, var->type);

This pattern appears in:

read_body_assignment (line 3815)
read_body_statement assignments (line 4563)
read_body_statement nv assignments (line 4661)
read_ternary_operation twice

Problem: The special case logic is multiplying, not disappearing.

jserv · 2025-12-24T03:35:25Z

src/parser.c

+bool is_array_literal_placeholder(var_t *var)
+{
+    return var && var->array_size > 0 && !var->ptr_level &&
+           var->var_name[0] == '.';
+}


Detecting temporaries by checking if name starts with . is fragile. This is pattern-matching on internal naming conventions rather than explicit type tracking.

Better approach: Add an explicit is_compound_literal flag to var_t.

jserv · 2025-12-24T03:36:16Z

src/parser.c

+        if (func && param_num < func->num_params) {
+            var_t *target = &func->param_defs[param_num];
+            if (!target->ptr_level && !target->array_size)
+                param =
+                    scalarize_array_literal(parent, bb, param, target->type);
+        }


This only scalarizes for defined params, but variadic args (func->va_args) might still misbehave. Test case needed:

printf("%p", (int[]){1,2,3}); // Should print pointer, not 1

jserv · 2025-12-24T03:37:03Z

src/parser.c

+    var->array_size = count;
+}
+/* Identify compiler-emitted temporaries that hold array compound literals.
+ * Parsing assigns these temporaries synthetic names via gen_name_to (".tN")


Add explicit compound literal tracking instead of .tN name inspection.

jserv · 2025-12-24T03:38:17Z

Consider test case below

  // Variadic function with array literal
  int main(void) {
      int *p = (int[]){42, 43, 44};
      return *p == 42 ? 0 : 1;
  }

The existing tests cover basic pointer decay but not all edge cases from the cubic analysis.

This comment was marked as outdated.

Sign in to view

jserv reviewed Oct 6, 2025

View reviewed changes

DrXiao suggested changes Oct 6, 2025

View reviewed changes

jserv requested changes Oct 6, 2025

View reviewed changes

jserv reviewed Oct 6, 2025

View reviewed changes

src/parser.c Outdated Show resolved Hide resolved

jserv reviewed Oct 8, 2025

View reviewed changes

src/parser.c Outdated Show resolved Hide resolved

jserv requested changes Oct 8, 2025

View reviewed changes

visitorckw suggested changes Oct 8, 2025

View reviewed changes

jserv reviewed Oct 8, 2025

View reviewed changes

src/parser.c Show resolved Hide resolved

This comment was marked as resolved.

Sign in to view

sysprog21 deleted a comment from cubic-dev-ai bot Nov 7, 2025

This comment was marked as resolved.

Sign in to view

lorettayao force-pushed the fix-array-init branch from bf5f3d0 to b88294c Compare November 7, 2025 11:55

sysprog21 deleted a comment from cubic-dev-ai bot Nov 7, 2025

lorettayao force-pushed the fix-array-init branch 2 times, most recently from 0de5d30 to 08f1e29 Compare November 9, 2025 09:41

jserv requested a review from ChAoSUnItY November 14, 2025 08:26

ChAoSUnItY reviewed Nov 15, 2025

View reviewed changes

lorettayao force-pushed the fix-array-init branch from 08f1e29 to b13f5b6 Compare November 18, 2025 09:14

sysprog21 deleted a comment from cubic-dev-ai bot Nov 18, 2025

lorettayao force-pushed the fix-array-init branch from dbea9ca to 4788e86 Compare November 25, 2025 08:20

lorettayao force-pushed the fix-array-init branch from 4788e86 to cc86cd4 Compare November 25, 2025 09:01

jserv requested changes Nov 26, 2025

View reviewed changes

lorettayao force-pushed the fix-array-init branch from cc86cd4 to b619761 Compare November 27, 2025 14:22

lorettayao force-pushed the fix-array-init branch from b619761 to 79847d5 Compare November 27, 2025 14:55

DrXiao reviewed Nov 30, 2025

View reviewed changes

lorettayao force-pushed the fix-array-init branch from 79847d5 to a4aba54 Compare December 2, 2025 03:55

jserv requested a review from DrXiao December 5, 2025 09:21

jserv reviewed Dec 5, 2025

View reviewed changes

src/parser.c Show resolved Hide resolved

jserv reviewed Dec 5, 2025

View reviewed changes

src/parser.c Outdated Show resolved Hide resolved

DrXiao reviewed Dec 6, 2025

View reviewed changes

src/parser.c Outdated Show resolved Hide resolved

DrXiao reviewed Dec 6, 2025

View reviewed changes

DrXiao suggested changes Dec 6, 2025

View reviewed changes

lorettayao force-pushed the fix-array-init branch from a4aba54 to 3e1f41a Compare December 23, 2025 11:40

Fix array compound literal parsing #309

Are you sure you want to change the base?

Fix array compound literal parsing #309

Conversation

lorettayao commented Oct 6, 2025 • edited by cubic-dev-ai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by cubic

Uh oh!

This comment was marked as outdated.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DrXiao left a comment

Choose a reason for hiding this comment

Uh oh!

jserv left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jserv commented Oct 6, 2025

Uh oh!

visitorckw commented Oct 6, 2025

Uh oh!

DrXiao commented Oct 6, 2025

Uh oh!

visitorckw commented Oct 6, 2025

Uh oh!

DrXiao commented Oct 6, 2025

Uh oh!

visitorckw commented Oct 6, 2025

Uh oh!

Uh oh!

jserv left a comment

Choose a reason for hiding this comment

Uh oh!

visitorckw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

cubic-dev-ai bot commented Nov 7, 2025 • edited by jserv Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Style Fixes for parser.c

Comment Improvements

For Tests in driver.sh

Uh oh!

jserv commented Nov 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ChAoSUnItY Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lorettayao Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lorettayao commented Nov 25, 2025

Uh oh!

visitorckw commented Nov 25, 2025

Uh oh!

lorettayao commented Nov 25, 2025

Uh oh!

jserv left a comment

Choose a reason for hiding this comment

Uh oh!

DrXiao commented Nov 27, 2025

Uh oh!

lorettayao commented Oct 6, 2025 •

edited by cubic-dev-ai bot

Loading

cubic-dev-ai bot commented Nov 7, 2025 •

edited by jserv

Loading

Style Fixes for `parser.c`

For Tests in `driver.sh`

ChAoSUnItY Nov 18, 2025 •

edited

Loading

lorettayao Nov 18, 2025 •

edited

Loading

DrXiao left a comment •

edited

Loading