fix(hir): #830 — obj.method?.(args) short-circuits on null callee, not receiver#834
Merged
Merged
Conversation
…not receiver The OptChain(Call) handler — which corresponds to `<expr>?.(args)` where the `?.` is between the callee and the call parens — was lowering `obj.method?.(args)` as `obj == null ? undefined : obj.method(args)`. That checks the receiver, but the `?.` is on the call, so the test must be on the function value. When `obj` was a valid object with no `method` property, the else branch tried to invoke `undefined` and threw `TypeError: method is not a function`. (The misleading comment `// obj?.method() -> obj == null ? undefined : obj.method()` had the AST shape backwards — that form parses as `Call(OptChain(Member))` and goes through the regular Call lowering path; it never reaches this branch.) Fix: in the simple Member case, set `check_expr = callee_expr.clone()` so the conditional tests the function value. callee_expr remains a `PropertyGet`, so the codegen still binds `this = obj` in the call branch. The inner-OptChain case (`foo?.bar?.()`) keeps the existing `check_expr = obj` behavior to preserve the `foo == null` short-circuit; a separate `foo.bar == null` check on that chained form is a pre-existing gap, documented inline. Closes #830.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
obj.method?.(args)no longer throwsTypeError: method is not a functionwhenobj.methodis undefined — it now correctly short-circuits toundefined.obj.fn?.(...)throws when fn is undefined (should short-circuit to undefined) #830 (surfaced by the compat: auto-generated TypeScript feature matrix #801 feature-matrix probetest_feat_optional_chaining.ts).Root cause
The
OptChainBase::Call(call)branch incrates/perry-hir/src/lower.rswas loweringobj.method?.(args)asobj == null ? undefined : obj.method(args)— checking the receiver for null when the?.is actually on the call, so the check must be on the function value. The leading comment// obj?.method() -> obj == null ? undefined : obj.method()had the AST shape backwards:obj?.method()is parsed asCall(OptChain(Member))(regular Call path with an OptChain callee), notOptChain(Call). TheOptChain(Call)shape is exclusively<expr>?.(args).Fix
For the simple Member callee, set
check_expr = callee_expr.clone().callee_exprstays aPropertyGet, so the codegen still bindsthis = objin the call branch. The inner-OptChain branch (foo?.bar?.()) keeps the existingcheck_expr = objbehavior to preserve thefoo == nullshort-circuit — a separatefoo.bar == nullcheck on that chained form is a pre-existing gap, documented inline.Test plan
node --experimental-strip-types:full.greet?.('bob')→hi bob,sparse.greet?.('bob')→undefined.obj.contact?.email,obj.tags?.[0],obj?.method()via the different AST path,??,??=) byte-identical to node.test_gap_console_methodsandtest_gap_regexp_advanced/ String.prototype.replace(/.../g, fn) — replacer-fn return value is dropped (replaces with empty string) #833, both unrelated).cargo test --release -p perry-hirgreen (10 + 6 / 0 failed).Closes #830. Refs #793, #801.