Skip to content

Fix call argument evaluation order and enable await in call arguments#1034

Merged
xeioex merged 8 commits intonginx:masterfrom
xeioex:parser_refactor_method_frame
Mar 12, 2026
Merged

Fix call argument evaluation order and enable await in call arguments#1034
xeioex merged 8 commits intonginx:masterfrom
xeioex:parser_refactor_method_frame

Conversation

@xeioex
Copy link
Copy Markdown
Contributor

@xeioex xeioex commented Mar 5, 2026

Why

Spec compliance: ECMAScript requires arguments (including their side effects)
to be evaluated before the callee's callability is checked. The old call lowering
created the frame first, which threw non-callable errors before argument side
effects ran.

Enable await in call arguments and tagged templates: The old frame-first
design made it impossible to suspend on await during argument evaluation
because the frame was already half-constructed. The parser rejected both as a
workaround.

Preserve this for grouped optional calls: (o?.m)() resolved the callee
through the optional chain but dispatched via plain FUNCTION_CALL, losing the
receiver.

Behavioral changes

  • Arguments with side effects are now fully evaluated before non-callable errors
    are thrown.
  • Constructor arguments are evaluated before checking whether the callee is
    constructable.
  • Error messages change from (intermediate value)["x"] is not a function to
    type is not a function (e.g., "number is not a function").
  • Backtraces no longer show native frames when arguments throw before reaching
    the native function.
  • await in call arguments and tagged templates is now allowed.
  • (o?.m)() correctly preserves the receiver.

Examples

// Call ordering — previously threw before evaluating foo()
var fooCalled = false;
function foo() { fooCalled = true; }
var o = {};
try { o.bar(foo()) } catch (e) {}
fooCalled  // true (was false)

// Await in call arguments — previously SyntaxError
async function f() {
    return ({
        g(v) { return v; }
    }).g(await Promise.resolve(2));
}

// Grouped optional chain this — previously TypeError
var o = {x: 5, m: function() { return this.x }};
(o?.m)()  // 5 (was TypeError: cannot get property "x" of undefined)

Test262: +12 tests passed (14457 → 14469)

  • language/expressions/call/11.2.3-3_{1,2,4,6,7} — call argument ordering
  • language/expressions/new/ctorExpr-isCtor-after-args-eval{,-fn-wrapup}
    constructor ordering
  • language/expressions/await/await-{awaits-thenables,monkey-patched-promise}
    await in args
  • language/expressions/optional-chaining/{member-expression-async-literal,optional-chain-async-*-square-brackets}
    await + optional chain

xeioex added 5 commits March 3, 2026 20:59
Previously, the generator inferred reference intent from raw AST shape.

Now the parser marks the relevant property node as PROPERTY_REF before
building METHOD_CALL, assignment, or update nodes, and the generator
accepts both PROPERTY and PROPERTY_REF via
njs_generate_is_property_lvalue().

Introduce NJS_TOKEN_PROPERTY_REF as an explicit parser-side marker for
property accesses that carry reference semantics (assignment targets,
delete operands, increment/decrement, method-call receivers).
Previously, grouped optional calls like (o?.m)() resolved the callee
through the optional chain but dispatched via plain FUNCTION_CALL,
losing the original receiver.

The fix stores the receiver on the call node so the upcoming
call-argument reorder can emit METHOD_FRAME with an explicit "this".
Call-expression setup and optional-chain preserve lookup are routed
through named helpers with generator-side validation.
Introduce NJS_TOKEN_OPTIONAL_PRESERVE for optional-chain preserve nodes
instead of reusing OBJECT_VALUE, so OBJECT_VALUE remains strictly an
object/array literal structure token.

Route optional-preserve, object-value, and optional method-preserve
access through dedicated helper functions with narrow assertions,
removing direct u.object/left/right access from general parser and
generator paths.

No behavior change.
Previously, the "in" operator swap flag was relayed through generator
context.  This broke when call-end handlers switched to
njs_generator_stack_pop(NULL), which released the context before the
swap was read in njs_generate_3addr_operation_end().

The fix derives the swap directly from the node token type
(NJS_TOKEN_IN), eliminating the context relay.
@xeioex xeioex requested a review from VadimZhestikov March 5, 2026 07:30
xeioex added 3 commits March 6, 2026 22:06
Previously, call lowering created FUNCTION_FRAME and METHOD_FRAME before
argument evaluation.  This made call ordering observably wrong: for
non-callable callees, the error was thrown before arguments with side
effects were evaluated, violating the ECMAScript specification.  It also
prevented await expressions in call arguments, which were rejected at
parse time because suspending inside a half-created frame was not
supported.

The fix evaluates arguments first, then emits the frame, PUT_ARG, and
FUNCTION_CALL.  Callee and receiver values are captured into temporaries
before argument evaluation to guard against argument side effects.
Method properties are resolved via PROPERTY_GET before arguments.

METHOD_FRAME is redefined from a composite opcode (property lookup +
callability check + frame creation) to a pure frame-creation opcode
that takes an already-resolved function and explicit "this" value.
The parser always wraps call expressions in a NJS_TOKEN_FUNCTION_CALL
node, removing the NJS_TOKEN_NAME special case.
@xeioex xeioex force-pushed the parser_refactor_method_frame branch from c76899f to 9872442 Compare March 7, 2026 06:16
@VadimZhestikov
Copy link
Copy Markdown
Contributor

Looks good. May be useful add next testcases:

// 1. new + awaited args + constructor order

async function f(v, log) { log.push(v); return v; }
async function t1() {
    let log = [];
    function C(a, b) { log.push("ctor"); this.r = a + b; }
    let o = new C(await f(1, log), await f(2, log));
    return JSON.stringify([o.r, log]);
}
t1().then(console.log);
// [3,[1,2,"ctor"]]

// 2. grouped optional call preserves this with awaited arg

async function t2() {
    let o = { x: 5, m(y) { return this.x + y; } };
    return (o?.m)(await Promise.resolve(7));
}
t2().then(console.log);
// 12


// 3. non-callable still evaluates args before throw

async function side(log) { log.push("arg"); return 1; }
async function t3() {
    let log = [];
    try { (0)(await side(log)); } catch (e) { log.push(e.name); }
    return JSON.stringify(log);
}
t3().then(console.log);
// ["arg","TypeError"]


// 4. computed method key evaluated before arg, receiver preserved

async function t4() {
    let log = [];
    let o = {
        x: 9,
        m(v) { log.push("this:" + this.x); return v; }
    };
    async function k(){ log.push("key"); return "m"; }
    async function a(){ log.push("arg"); return 1; }
    await o[await k()](await a());
    return JSON.stringify(log);
}
t4().then(console.log);
// ["key","arg","this:9"]

Copy link
Copy Markdown
Contributor

@VadimZhestikov VadimZhestikov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@xeioex xeioex merged commit e496636 into nginx:master Mar 12, 2026
2 checks passed
@xeioex xeioex deleted the parser_refactor_method_frame branch March 12, 2026 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants