fix(codegen): stage no ancestor field initializers a delegated constructor installs itself (#11120) - #11129
fix(codegen): stage no ancestor field initializers a delegated constructor installs itself (#11120)#11129proggeramlug wants to merge 2 commits into
Conversation
…uctor installs itself
A construction stages ancestor field initializers before a constructor body
runs. It then hands the parent part to a parent constructor. On three paths
that parent constructor installed the same ancestors again. Public
initializers ran twice (side effects doubled), and a #private field threw
"Cannot initialize a private field twice on the same object":
- super(...spread) lowers to js_super_construct_apply, which runs the
parent's whole standalone constructor. The root was also staged up front.
This is the redis@6.1.0 createClient({ socket }) failure: a CommonJS module
body is a function, so its classes capture and get a synthesized
super(...args) constructor.
- An inherited constructor body inlined for a no-own-ctor class
(UpToInclusive) staged the whole prefix, including the inherited class
itself. That body's own super() applies the intermediates and the class
itself again. Plain class F extends E {} over a constructor-owning derived
E with a #field threw on new F().
- The standalone constructor of a no-own-ctor class staged the root, then
called a local ancestor's constructor symbol, which staged it again. It
also applied only SelfOnly afterwards, which dropped constructor-free
intermediates.
Fixes #11120
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 8 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe code generator now adjusts ancestor field initialization across constructor paths, including spread ChangesAncestor Field Initialization
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: ⚪ Minimal · up to This change stops parent-class field initializers from running twice when a subclass reaches its parent through a spread 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 69.23% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 4 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
(cherry picked from commit 1f9c509)
(cherry picked from commit 1f9c509)
|
Landed on main in merge train 270 (#11132, v0.5.1653). The train rebase gives commits new SHAs, so GitHub cannot close this automatically. |
2 participants
Fixes #11120
Root cause
Before a constructor body runs, a construction stages ancestor field initializers (
AncestorsOnly/UpToInclusive). It then hands the parent part to a parent constructor. On three codegen paths that parent constructor installs the same ancestors again. Public initializers then run twice, and the side effects double silently. A#privatefield throwsCannot initialize a private field twice on the same object.super(...spread): the argument count is dynamic, so the parent can't be inlined. The call lowers tojs_super_construct_apply, which runs the parent's whole registered standalone constructor. That constructor already installs every field from the root through the parent, but the root was also staged up front. This is the redis case. A CommonJS module body is a function, soEmptyAwareSinglyLinkedListcapturesevents_1and gets a synthesizedconstructor(...args) { super(...args) }. The events part of the issue title is incidental: any captured binding triggers it, and so does an explicit spread super. The fix:ctor_super_reruns_parent_ctor(new,field_init.rs) mirrors theSuperCallSpreadarm selection. It follows a plainsuper()into the inlined ancestor, soF { constructor() { super() } }over a spreadEis covered too. When it answers yes, nothing above the leaf is staged.UpToInclusive(stop_at)is used when a no-own-ctor class inlines an inherited constructor body. It staged the whole prefix,stop_atincluded. That body's ownsuper()then applies the intermediates andSelfOnly(stop_at)again. So a plainclass F extends E {}, overclass E extends S { #b = 1; constructor() { super(); } }, threw onnew F(). It now stages the same thingstop_at's constructor stages whenstop_atitself is constructed.new <classValue>()path) staged the root. It then called the local ancestor's constructor symbol, which staged the root again. Afterwards it applied onlySelfOnly, so constructor-free intermediates were dropped. For example, inS <- E(ctor) <- M <- F,new (F as any)()leftmundefined. When the synthesized super calls a local ancestor's constructor symbol, the standalone constructor now stages nothing up front and appliesBetweenExclusiveTo(ancestor)afterwards.This does not depend on #11122. The issue repro fails on
origin/main784ed8e and passes with this change, onmainalone.Tests
test-files/test_gap_11120_ancestor_field_init_staged_once.tscovers the capture-synthesized constructor, an explicit spread super, inherited constructor bodies (static and dynamicnew), a spread-super class reached as an inherited constructor, a captured spread-super class, and#x in obrand checks. It counts initializer runs.mainbuild:Cannot initialize a private field twice on the same object./opt/node-v26.5.1-linux-x64).require("events"), auto-optimize path) printsA 1/B 0with the fix, the same as Node.mainthrows.main.test-files/*.tswithextendsplus a private field, spread super, constructor or= new. Both arms were perry-dev builds,PERRY_NO_AUTO_OPTIMIZE=1.mainand the fix.main, 173 with the fix.run_parity_tests.shbecause port 17891 was held by another sweep on the host.cargo test -p perry-codegen: 2190 passed, 0 failed, 6 ignored (40 suites), no warnings. This ran on macOS, debug profile.cargo fmt --all -- --check: ok.scripts/check_file_size.sh: ok.SKIP_COMPILE_GATES=1 scripts/run_lint_gates.sh: 87 of 88 script gates passed. The failure iscargo xwin check, becausecargo-xwinis not installed on the Linux host. The compile tier was not run.git diff --statwas clean afterwards.Not run
cargo check --workspace --all-targets -D warnings: onlyperry-codegenis touched, and its test build is warning-free.Found along the way (separate, pre-existing, unchanged by this PR)
#privatefield on a subclass instance when both classes are function-local.pushmethod on an instance fromnew <any class value>loses its effects.Summary by CodeRabbit