fix(codegen): use default coercion in folded string adds - #10996
proggeramlug wants to merge 2 commits into
Conversation
|
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 (6)
Included review availability: Your plan provides up to 8 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughChangesFolded string addition coercion
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to Folded string addition now applies default-hint coercion while preserving evaluation order and Symbol errors. No actionable merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 62.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 5 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1🧪 Generate unit tests (beta)
🛠️ Fix failing CI checks 💡
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 |
|
Landed on main in merge train 258 (#11078, v0.5.1641), main Carried at head This train was bisect-verified: after an earlier 35-PR assembly hit five gap regressions, the lowering-touching PRs were split into probes, and this set (#11070) came back with all six gap shards clean while the other half reproduced all five. Trains rebase-merge, so commits get new SHAs and GitHub cannot mark this merged. Closed as landed. |
Fixes #10775.
The fused N-way
+path previously evaluated every operand and then letjs_string_concat_chainperform ToString on objects. That skippedvalueOf, passed the wrong hint toSymbol.toPrimitive, and delayed coercion until after later operands were evaluated.This change keeps the fused allocation but converts object-capable operands with the existing default-hint Add coercion at each operator boundary. The first pair is evaluated before either conversion; each later right operand is evaluated and converted before the next one. Values remain rooted across later evaluations and coercions. Template substitutions retain their string-hint conversion. The Add path also throws when ToPrimitive yields a Symbol.
The regression fixture covers
valueOf, both coercion hints, evaluation order, mutation during conversion, and Symbol throwing. On the unmodified base, the filtered parity run failed (a[object Object]bversus Node'sa5b, plus the other mismatches). With this patch,./run_parity_tests.sh --filter test_issue_10775_string_add_default_hintpassed 1/1 on an isolatedperrymastercheckout, byte for byte against Node 26.8.1.Other checks:
cargo fmt --all --check,./scripts/check_file_size.sh,python3 scripts/check_test_registration.py, andgit diff --check.Summary by CodeRabbit
Bug Fixes
valueOfandSymbol.toPrimitivebehavior in chained additions.TypeErrorhandling when conversion produces aSymbol.Tests
Documentation