From c024c09bc6dc77c989514e24b3ff8744167881dd Mon Sep 17 00:00:00 2001 From: Hojun-Cho Date: Fri, 19 Jun 2026 14:41:57 +0900 Subject: [PATCH] check: reject duplicate struct field name in wwstage (catB-7/14) The wwstage checker silently accepted a struct with repeated field names; cstage already rejects it (cmd/wcc/check.c:925-947). Add validatestructfields, dispatched once per struct decl from resolvewalk's eager type-decl arm (check.ww:792, sibling to the N_TENUM stampenumvals fire): a pure read-only O(n^2) named-field dup walk that emits "duplicate field 'X'" via cerr + c.errs, with no mutation -- valid-program codegen is unchanged so cstage==wwstage byte-id holds. Named fields only; ww has no struct embedding, so cstage's embed-collision arm is intentionally not ported (separate parser gap, catB-89). Test: new table-driven both-stage reject test 849_dupfield_reject (adjacent / non-adjacent / different-type dup rows + a distinct-field control that builds and runs). Full make test: 451 green incl. 990-997 byte-id. --- Makefile | 7 ++ selfhost/cmd/wcc/check.ww | 35 ++++++ test/wcc/849_dupfield_reject.c | 187 +++++++++++++++++++++++++++++++++ 3 files changed, 229 insertions(+) create mode 100644 test/wcc/849_dupfield_reject.c diff --git a/Makefile b/Makefile index e735fe06..01f7b832 100644 --- a/Makefile +++ b/Makefile @@ -247,6 +247,7 @@ TESTS = $(BIN)/test_smoke $(BIN)/test_lex $(BIN)/test_parse $(BIN)/test_check \ $(BIN)/test_tuple_trailing_comma_reject \ $(BIN)/test_voidless_return_reject \ $(BIN)/test_const_reassign_reject \ + $(BIN)/test_dupfield_reject \ $(BIN)/test_size_untyped_int \ $(BIN)/test_tagged_staticinit \ $(BIN)/test_arr_infer_len \ @@ -1542,6 +1543,12 @@ $(BIN)/test_const_reassign_reject: test/wcc/847_const_reassign_reject.c $(BIN)/w $(LIB)/libwwrt.a | $(BIN) $(CC) $(CFLAGS) -o $@ $< +$(BIN)/test_dupfield_reject: test/wcc/849_dupfield_reject.c $(BIN)/ww \ + $(BIN)/w6c $(BIN)/w6a $(BIN)/w6l \ + $(BIN)/ww_ww $(BIN)/w6c_ww $(BIN)/w6a_ww $(BIN)/w6l_ww \ + $(LIB)/libwwrt.a | $(BIN) + $(CC) $(CFLAGS) -o $@ $< + $(BIN)/test_tagged_staticinit: test/wcc/843_tagged_staticinit.c $(BIN)/ww \ $(BIN)/w6c $(BIN)/w6a $(BIN)/w6l \ $(BIN)/ww_ww $(BIN)/w6c_ww $(BIN)/w6a_ww $(BIN)/w6l_ww \ diff --git a/selfhost/cmd/wcc/check.ww b/selfhost/cmd/wcc/check.ww index 0e50473d..e022e30e 100644 --- a/selfhost/cmd/wcc/check.ww +++ b/selfhost/cmd/wcc/check.ww @@ -789,6 +789,7 @@ fn resolvewalk(c: *checker, n: *syntax.node) void = { }; if (k == syntax.nkind.N_TENUM) { stampenumvals(c, n); }; + if (k == syntax.nkind.N_TSTRUCT) { validatestructfields(c, n); }; // #42's size/align/offset fold trigger lived here pre-A.6.0; the // A.6.0 end-of-fn general dispatch (below) now fires exprtype on @@ -1859,6 +1860,40 @@ fn stampenumvals(c: *checker, n: *syntax.node) void = { }; }; +// validatestructfields — reject a struct decl carrying two fields with +// the same name. Pure diagnostic: a read-only O(n^2) walk over the named +// field list, no n.type_ / offset / checker-state mutation (the byte-id +// safety condition — valid programs have no dup, so codegen is untouched). +// Cstage twin: cmd/wcc/check.c:943-950 (the regular-named-field arm of +// resolve_type's N_TSTRUCT). The embed-collision arm there (check.c:961- +// 985) is NOT ported — ww has no struct embedding (parser gap catB-89), +// so every N_TFIELD is a regular named field. Fires once per struct decl +// from resolvewalk's eager type-decl dispatch (rule-10 symmetric with +// cstage's once-per-resolve_type), not from the per-query size/align/ +// offset recompute arms (use-site = double-fire, drew-dv ruling). +fn validatestructfields(c: *checker, n: *syntax.node) void = { + let f: *syntax.node = n.list; + for (f != nil) { + if (f.kind == syntax.nkind.N_TFIELD && f.str.len != 0) { + let e: *syntax.node = n.list; + for (e != f) { + if (e.kind == syntax.nkind.N_TFIELD + && e.str.len != 0 + && syntax.streq(e.str, f.str)) { + cerr(f.file); + cerr(": error: duplicate field '"); + cerr(f.str); + cerr("'\n"); + c.errs += 1; + break; + }; + e = e.next; + }; + }; + f = f.next; + }; +}; + // stampnilexpr — stamp nil-typed nodes in a constant expr subtree to // `ti`. lhs/rhs cover the enum constexpr grammar enumvalfold accepts // (literals, unary, binary, sibling backref); non-nil nodes keep the diff --git a/test/wcc/849_dupfield_reject.c b/test/wcc/849_dupfield_reject.c new file mode 100644 index 00000000..de89a7fb --- /dev/null +++ b/test/wcc/849_dupfield_reject.c @@ -0,0 +1,187 @@ +/* + * 849_dupfield_reject (catB-7/14) — cstage and wwstage LOUD-REJECT a + * struct declaration that carries two fields with the same name. + * + * cstage already rejected at cmd/wcc/check.c:943-950 (the regular-named- + * field arm of resolve_type's N_TSTRUCT). wwstage silently ACCEPTED — it + * never compared field names. The fix adds validatestructfields(), a pure + * read-only O(n^2) named-field dup walk, fired once per struct decl from + * resolvewalk's eager type-decl dispatch (selfhost/cmd/wcc/check.ww), the + * rule-10 twin of cstage's once-per-resolve_type site. ww has no struct + * embedding (parser gap catB-89), so cstage's embed-collision arm + * (check.c:961-985) is intentionally NOT ported — named fields only. + * + * Byte-id: pure diagnostic, no n.type_ / offset / checker-state mutation; + * the valid corpus has no duplicate fields, so codegen of every valid + * program is unchanged → cstage == wwstage byte-id holds. + * + * row | shape | expect + * -------------+------------------------------------+-------- + * adj_dup | struct { a: i32, a: i32 } | REJECT + * nonadj_dup | struct { a: i32, b: i32, a: i32 } | REJECT + * dup_difftype | struct { a: i32, a: bool } | REJECT + * distinct | struct { x: i32, y: i32 }, use x+y | BUILD, run 7 + */ +#include +#include +#include +#include +#include +#include + +static int +runwait(const char *cmd) +{ + int rc = system(cmd); + if (rc == -1) return -1; + if (WIFEXITED(rc)) return WEXITSTATUS(rc); + return -1; +} + +static void +outbin(char *dst, size_t n, const char *tmpdir, const char *src) +{ + const char *base = strrchr(src, '/'); + base = base ? base + 1 : src; + snprintf(dst, n, "%s/%s", tmpdir, base); + char *dot = strrchr(dst, '.'); + if (dot && strcmp(dot, ".ww") == 0) *dot = '\0'; +} + +static int +build_should_fail(const char *driver, const char *label, const char *src, + int i) +{ + char s[64], tmpdir[64], cmd[1024], bin[128]; + snprintf(s, sizeof s, "/tmp/dupf_%d_%d.ww", getpid(), i); + snprintf(tmpdir, sizeof tmpdir, "/tmp/dupf_%d_d_%d", getpid(), i); + FILE *f = fopen(s, "wb"); + if (!f) return -1; + fputs(src, f); + fclose(f); + mkdir(tmpdir, 0755); + snprintf(cmd, sizeof cmd, "cd %s && %s build %s 2>/dev/null", + tmpdir, driver, s); + int rc = runwait(cmd); + outbin(bin, sizeof bin, tmpdir, s); + unlink(s); + unlink(bin); + rmdir(tmpdir); + if (rc == 0) + fprintf(stderr, "dupf[%s][%s]: built ok, expected a reject\n", + driver, label); + return rc == 0 ? -1 : 0; +} + +static int +run_build(const char *driver, const char *label, const char *src, int i) +{ + char s[64], tmpdir[64], cmd[1024], bin[128]; + snprintf(s, sizeof s, "/tmp/dupf_ok_%d_%d.ww", getpid(), i); + snprintf(tmpdir, sizeof tmpdir, "/tmp/dupf_ok_%d_d_%d", getpid(), i); + FILE *f = fopen(s, "wb"); + if (!f) return -1; + fputs(src, f); + fclose(f); + mkdir(tmpdir, 0755); + snprintf(cmd, sizeof cmd, "cd %s && %s build %s 2>/dev/null", + tmpdir, driver, s); + if (runwait(cmd) != 0) { + fprintf(stderr, "dupf[%s][%s]: build failed (distinct fields " + "must compile)\n", driver, label); + unlink(s); rmdir(tmpdir); + return -1; + } + outbin(bin, sizeof bin, tmpdir, s); + int got = runwait(bin); + unlink(s); unlink(bin); rmdir(tmpdir); + return got; +} + +struct rejrow { const char *label; const char *src; }; + +static const struct rejrow reject_rows[] = { + { "adj_dup", + "package main;\n" + "type d = struct { a: i32, a: i32 };\n" + "fn main() i32 = { let p: *d = nil; return 0; };\n" }, + { "nonadj_dup", + "package main;\n" + "type d = struct { a: i32, b: i32, a: i32 };\n" + "fn main() i32 = { let p: *d = nil; return 0; };\n" }, + { "dup_difftype", + "package main;\n" + "type d = struct { a: i32, a: bool };\n" + "fn main() i32 = { let p: *d = nil; return 0; };\n" }, +}; + +struct okrow { const char *label; const char *src; int want; }; + +static const struct okrow ok_rows[] = { + { "distinct", + "package main;\n" + "type pt = struct { x: i32, y: i32 };\n" + "fn main() i32 = { let p: pt = pt{x=3, y=4}; return p.x + p.y; };\n", + 7 }, +}; + +int +main(void) +{ + const char *bin = getenv("BIN"); + if (!bin) bin = "out/bin"; + char absbin[1024]; + if (bin[0] != '/') { + char cwd[1024]; + if (getcwd(cwd, sizeof cwd) == NULL) return 1; + snprintf(absbin, sizeof absbin, "%s/%s", cwd, bin); + bin = absbin; + } + + char cdrv[1024], wdrv[1024]; + snprintf(cdrv, sizeof cdrv, "%s/ww", bin); + snprintf(wdrv, sizeof wdrv, "%s/ww_ww", bin); + + struct { const char *name; const char *path; int gated; } drivers[] = { + { "cstage", cdrv, 0 }, + { "wwstage", wdrv, 1 }, + { NULL, NULL, 0 }, + }; + + int nrej = (int)(sizeof reject_rows / sizeof reject_rows[0]); + int nok = (int)(sizeof ok_rows / sizeof ok_rows[0]); + int total = 0, fail = 0; + + for (int d = 0; drivers[d].name; d++) { + if (drivers[d].gated && access(drivers[d].path, X_OK) != 0) { + fprintf(stderr, "dupf: skip %s (no %s)\n", + drivers[d].name, drivers[d].path); + continue; + } + for (int i = 0; i < nrej; i++) { + total++; + if (build_should_fail(drivers[d].path, + reject_rows[i].label, reject_rows[i].src, + d * 100 + i) != 0) + fail++; + } + for (int i = 0; i < nok; i++) { + total++; + int got = run_build(drivers[d].path, ok_rows[i].label, + ok_rows[i].src, d * 100 + i); + if (got != ok_rows[i].want) { + fprintf(stderr, "dupf[%s][%s]: exit=%d want=%d\n", + drivers[d].name, ok_rows[i].label, + got, ok_rows[i].want); + fail++; + } + } + } + + if (fail) { + fprintf(stderr, "dupf: %d/%d fixtures failed\n", fail, total); + return 1; + } + printf("dupf: %d/%d ok\n", total, total); + return 0; +}