Skip to content

Commit 91a7791

Browse files
author
Ralph Küpper
committed
docs(codegen): state the site-slot publish obligation now that #9890 is fixed
The comment at the `Expr::RegExp` lowering described the artifact-discarding bail-out in `codegen/method.rs` in the present tense. #9896 fixed it: every return there now goes through `publish_lowered_fn_artifacts`, which drains all three collections and restores `llmod.ic_counter`, closing the duplicate site-id half as well. Rewritten as the obligation rather than the bug — every lowering exit must PUBLISH `typed_parse_rodata`, and a future early return that drops it breaks this site loudly at the in-process LLVM parse. A comment describing a hazard that no longer exists is a false lead, which is the thing it was written to prevent.
1 parent 78e8b9d commit 91a7791

1 file changed

Lines changed: 13 additions & 15 deletions

File tree

crates/perry-codegen/src/expr/logical_collections.rs

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1314,21 +1314,19 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
13141314
// same reason `inline_cache_global_name` does — codegen-unit
13151315
// splitting can promote a private global for cross-unit use.
13161316
//
1317-
// FAILURE MODE, stated so it is diagnosable rather than surprising:
1318-
// `typed_parse_rodata` is drained into the module by all five
1319-
// lowering entry points, but `codegen/method.rs`'s
1320-
// "parent class has no callable constructor symbol" bail-out
1321-
// DISCARDS it (along with `ic_globals` and `pending_declares`)
1322-
// after having lowered the body. A regex literal inside such a
1323-
// constructor would therefore reference a global that is never
1324-
// defined. That is a LOUD compile-time failure — the in-process
1325-
// LLVM parse rejects it with `use of undefined value`, exactly as
1326-
// it rejected #9859's undeclared externs — and never a wrong
1327-
// answer at runtime. The same bail-out already discards IC globals
1328-
// and pending declares, so the path is either unreachable for
1329-
// non-trivial constructors or already broken for them; this note
1330-
// is here so the next person to see that message knows where to
1331-
// look.
1317+
// The slot must reach the module, so every lowering exit has to
1318+
// PUBLISH `typed_parse_rodata` rather than drop it. That was not
1319+
// true when this landed: `codegen/method.rs`'s "parent class has
1320+
// no callable constructor symbol" bail-out lowered the body and
1321+
// then discarded the three artifact collections, so a regex
1322+
// literal inside such a constructor would have referenced a
1323+
// global that is never defined (#9890, fixed by #9896 — every
1324+
// return now goes through `publish_lowered_fn_artifacts`, which
1325+
// also restores `llmod.ic_counter` and so closes the duplicate
1326+
// site-id half). Kept as a note because the obligation is real
1327+
// and unenforced: a future early return that drops the artifacts
1328+
// breaks this site, loudly, at the in-process LLVM parse (`use of
1329+
// undefined value`) rather than at runtime.
13321330
let site_id = ctx.ic_site_counter;
13331331
ctx.ic_site_counter += 1;
13341332
let slot_name = {

0 commit comments

Comments
 (0)