From 98ef59c4fb15a980dded81a1ba26f32bf3d274b5 Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Thu, 11 Mar 2021 17:50:17 -0800 Subject: [PATCH] add optional mutation to value capture --- internal/js_parser/js_parser.go | 28 ++++++++++----- internal/js_parser/js_parser_lower.go | 34 +++++++++--------- scripts/end-to-end-tests.js | 51 +++++++++++++++++++++++++++ 3 files changed, 87 insertions(+), 26 deletions(-) diff --git a/internal/js_parser/js_parser.go b/internal/js_parser/js_parser.go index f1c69360eac..802af06174b 100644 --- a/internal/js_parser/js_parser.go +++ b/internal/js_parser/js_parser.go @@ -8811,6 +8811,13 @@ func maybeJoinWithComma(a js_ast.Expr, b js_ast.Expr) js_ast.Expr { return js_ast.JoinWithComma(a, b) } +type captureValueMode uint8 + +const ( + valueDefinitelyNotMutated captureValueMode = iota + valueCouldBeMutated +) + // This is a helper function to use when you need to capture a value that may // have side effects so you can use it multiple times. It guarantees that the // side effects take place exactly once. @@ -8834,6 +8841,7 @@ func (p *parser) captureValueWithPossibleSideEffects( loc logger.Loc, // The location to use for the generated references count int, // The expected number of references to generate value js_ast.Expr, // The value that might have side effects + mode captureValueMode, // Say if "value" might be mutated and must be captured ) ( func() js_ast.Expr, // Generates reference expressions "_a" func(js_ast.Expr) js_ast.Expr, // Call this on the final expression @@ -8865,14 +8873,16 @@ func (p *parser) captureValueWithPossibleSideEffects( case *js_ast.EString: valueFunc = func() js_ast.Expr { return js_ast.Expr{Loc: loc, Data: &js_ast.EString{Value: e.Value}} } case *js_ast.EIdentifier: - valueFunc = func() js_ast.Expr { - // Make sure we record this usage in the usage count so that duplicating - // a single-use reference means it's no longer considered a single-use - // reference. Otherwise the single-use reference inlining code may - // incorrectly inline the initializer into the first reference, leaving - // the second reference without a definition. - p.recordUsage(e.Ref) - return js_ast.Expr{Loc: loc, Data: &js_ast.EIdentifier{Ref: e.Ref}} + if mode == valueDefinitelyNotMutated { + valueFunc = func() js_ast.Expr { + // Make sure we record this usage in the usage count so that duplicating + // a single-use reference means it's no longer considered a single-use + // reference. Otherwise the single-use reference inlining code may + // incorrectly inline the initializer into the first reference, leaving + // the second reference without a definition. + p.recordUsage(e.Ref) + return js_ast.Expr{Loc: loc, Data: &js_ast.EIdentifier{Ref: e.Ref}} + } } } if valueFunc != nil { @@ -11313,7 +11323,7 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO if !containsOptionalChain { if target, loc, private := p.extractPrivateIndex(e.Target); private != nil { // "foo.#bar(123)" => "__privateGet(foo, #bar).call(foo, 123)" - targetFunc, targetWrapFunc := p.captureValueWithPossibleSideEffects(target.Loc, 2, target) + targetFunc, targetWrapFunc := p.captureValueWithPossibleSideEffects(target.Loc, 2, target, valueDefinitelyNotMutated) return targetWrapFunc(js_ast.Expr{Loc: target.Loc, Data: &js_ast.ECall{ Target: js_ast.Expr{Loc: target.Loc, Data: &js_ast.EDot{ Target: p.lowerPrivateGet(targetFunc(), loc, private), diff --git a/internal/js_parser/js_parser_lower.go b/internal/js_parser/js_parser_lower.go index 72d9383914a..71bd7cab5c6 100644 --- a/internal/js_parser/js_parser_lower.go +++ b/internal/js_parser/js_parser_lower.go @@ -562,7 +562,7 @@ flatten: // thisArg = js_ast.Expr{Loc: loc, Data: &js_ast.EThis{}} } else { - targetFunc, wrapFunc := p.captureValueWithPossibleSideEffects(loc, 2, e.Target) + targetFunc, wrapFunc := p.captureValueWithPossibleSideEffects(loc, 2, e.Target, valueDefinitelyNotMutated) expr = js_ast.Expr{Loc: loc, Data: &js_ast.EDot{ Target: targetFunc(), Name: e.Name, @@ -582,7 +582,7 @@ flatten: // See the comment above about a similar special case for EDot thisArg = js_ast.Expr{Loc: loc, Data: &js_ast.EThis{}} } else { - targetFunc, wrapFunc := p.captureValueWithPossibleSideEffects(loc, 2, e.Target) + targetFunc, wrapFunc := p.captureValueWithPossibleSideEffects(loc, 2, e.Target, valueDefinitelyNotMutated) targetWrapFunc = wrapFunc // Capture the value of "this" if the target of the starting call @@ -608,7 +608,7 @@ flatten: // to capture it if it doesn't have any side effects (e.g. it's just a bare // identifier). Skipping the capture reduces code size and matches the output // of the TypeScript compiler. - exprFunc, exprWrapFunc := p.captureValueWithPossibleSideEffects(loc, 2, expr) + exprFunc, exprWrapFunc := p.captureValueWithPossibleSideEffects(loc, 2, expr, valueDefinitelyNotMutated) expr = exprFunc() result := exprFunc() @@ -622,7 +622,7 @@ flatten: for i := len(chain) - 1; i >= 0; i-- { // Save a reference to the value of "this" for our parent ECall if i == 0 && in.storeThisArgForParentOptionalChain && endsWithPropertyAccess { - parentThisArgFunc, parentThisArgWrapFunc = p.captureValueWithPossibleSideEffects(result.Loc, 2, result) + parentThisArgFunc, parentThisArgWrapFunc = p.captureValueWithPossibleSideEffects(result.Loc, 2, result, valueDefinitelyNotMutated) result = parentThisArgFunc() } @@ -642,7 +642,7 @@ flatten: // for "this" for the call. Example for this case: "foo.#bar?.()" if i > 0 { if _, ok := chain[i-1].Data.(*js_ast.ECall); ok { - privateThisFunc, privateThisWrapFunc = p.captureValueWithPossibleSideEffects(loc, 2, result) + privateThisFunc, privateThisWrapFunc = p.captureValueWithPossibleSideEffects(loc, 2, result, valueDefinitelyNotMutated) result = privateThisFunc() } } @@ -742,7 +742,7 @@ func (p *parser) lowerAssignmentOperator(value js_ast.Expr, callback func(js_ast switch left := value.Data.(type) { case *js_ast.EDot: if left.OptionalChain == js_ast.OptionalChainNone { - referenceFunc, wrapFunc := p.captureValueWithPossibleSideEffects(value.Loc, 2, left.Target) + referenceFunc, wrapFunc := p.captureValueWithPossibleSideEffects(value.Loc, 2, left.Target, valueDefinitelyNotMutated) return wrapFunc(callback( js_ast.Expr{Loc: value.Loc, Data: &js_ast.EDot{ Target: referenceFunc(), @@ -759,8 +759,8 @@ func (p *parser) lowerAssignmentOperator(value js_ast.Expr, callback func(js_ast case *js_ast.EIndex: if left.OptionalChain == js_ast.OptionalChainNone { - targetFunc, targetWrapFunc := p.captureValueWithPossibleSideEffects(value.Loc, 2, left.Target) - indexFunc, indexWrapFunc := p.captureValueWithPossibleSideEffects(value.Loc, 2, left.Index) + targetFunc, targetWrapFunc := p.captureValueWithPossibleSideEffects(value.Loc, 2, left.Target, valueDefinitelyNotMutated) + indexFunc, indexWrapFunc := p.captureValueWithPossibleSideEffects(value.Loc, 2, left.Index, valueDefinitelyNotMutated) return targetWrapFunc(indexWrapFunc(callback( js_ast.Expr{Loc: value.Loc, Data: &js_ast.EIndex{ Target: targetFunc(), @@ -789,7 +789,7 @@ func (p *parser) lowerAssignmentOperator(value js_ast.Expr, callback func(js_ast func (p *parser) lowerExponentiationAssignmentOperator(loc logger.Loc, e *js_ast.EBinary) js_ast.Expr { if target, privateLoc, private := p.extractPrivateIndex(e.Left); private != nil { // "a.#b **= c" => "__privateSet(a, #b, __pow(__privateGet(a, #b), c))" - targetFunc, targetWrapFunc := p.captureValueWithPossibleSideEffects(loc, 2, target) + targetFunc, targetWrapFunc := p.captureValueWithPossibleSideEffects(loc, 2, target, valueDefinitelyNotMutated) return targetWrapFunc(p.lowerPrivateSet(targetFunc(), privateLoc, private, p.callRuntime(loc, "__pow", []js_ast.Expr{ p.lowerPrivateGet(targetFunc(), privateLoc, private), @@ -807,14 +807,14 @@ func (p *parser) lowerNullishCoalescingAssignmentOperator(loc logger.Loc, e *js_ if target, privateLoc, private := p.extractPrivateIndex(e.Left); private != nil { if p.options.unsupportedJSFeatures.Has(compat.NullishCoalescing) { // "a.#b ??= c" => "(_a = __privateGet(a, #b)) != null ? _a : __privateSet(a, #b, c)" - targetFunc, targetWrapFunc := p.captureValueWithPossibleSideEffects(loc, 2, target) + targetFunc, targetWrapFunc := p.captureValueWithPossibleSideEffects(loc, 2, target, valueDefinitelyNotMutated) left := p.lowerPrivateGet(targetFunc(), privateLoc, private) right := p.lowerPrivateSet(targetFunc(), privateLoc, private, e.Right) return targetWrapFunc(p.lowerNullishCoalescing(loc, left, right)) } // "a.#b ??= c" => "__privateGet(a, #b) ?? __privateSet(a, #b, c)" - targetFunc, targetWrapFunc := p.captureValueWithPossibleSideEffects(loc, 2, target) + targetFunc, targetWrapFunc := p.captureValueWithPossibleSideEffects(loc, 2, target, valueDefinitelyNotMutated) return targetWrapFunc(js_ast.Expr{Loc: loc, Data: &js_ast.EBinary{ Op: js_ast.BinOpNullishCoalescing, Left: p.lowerPrivateGet(targetFunc(), privateLoc, private), @@ -841,7 +841,7 @@ func (p *parser) lowerLogicalAssignmentOperator(loc logger.Loc, e *js_ast.EBinar if target, privateLoc, private := p.extractPrivateIndex(e.Left); private != nil { // "a.#b &&= c" => "__privateGet(a, #b) && __privateSet(a, #b, c)" // "a.#b ||= c" => "__privateGet(a, #b) || __privateSet(a, #b, c)" - targetFunc, targetWrapFunc := p.captureValueWithPossibleSideEffects(loc, 2, target) + targetFunc, targetWrapFunc := p.captureValueWithPossibleSideEffects(loc, 2, target, valueDefinitelyNotMutated) return targetWrapFunc(js_ast.Expr{Loc: loc, Data: &js_ast.EBinary{ Op: op, Left: p.lowerPrivateGet(targetFunc(), privateLoc, private), @@ -863,7 +863,7 @@ func (p *parser) lowerLogicalAssignmentOperator(loc logger.Loc, e *js_ast.EBinar func (p *parser) lowerNullishCoalescing(loc logger.Loc, left js_ast.Expr, right js_ast.Expr) js_ast.Expr { // "x ?? y" => "x != null ? x : y" // "x() ?? y()" => "_a = x(), _a != null ? _a : y" - leftFunc, wrapFunc := p.captureValueWithPossibleSideEffects(loc, 2, left) + leftFunc, wrapFunc := p.captureValueWithPossibleSideEffects(loc, 2, left, valueDefinitelyNotMutated) return wrapFunc(js_ast.Expr{Loc: loc, Data: &js_ast.EIf{ Test: js_ast.Expr{Loc: loc, Data: &js_ast.EBinary{ Op: js_ast.BinOpLooseNe, @@ -1019,7 +1019,7 @@ func (p *parser) lowerPrivateSet( } func (p *parser) lowerPrivateSetUnOp(target js_ast.Expr, loc logger.Loc, private *js_ast.EPrivateIdentifier, op js_ast.OpCode, isSuffix bool) js_ast.Expr { - targetFunc, targetWrapFunc := p.captureValueWithPossibleSideEffects(target.Loc, 2, target) + targetFunc, targetWrapFunc := p.captureValueWithPossibleSideEffects(target.Loc, 2, target, valueDefinitelyNotMutated) target = targetFunc() // Load the private field and then use the unary "+" operator to force it to @@ -1032,7 +1032,7 @@ func (p *parser) lowerPrivateSetUnOp(target js_ast.Expr, loc logger.Loc, private if isSuffix { // "target.#private++" => "__privateSet(target, #private, _a = +__privateGet(target, #private) + 1), _a" - valueFunc, valueWrapFunc := p.captureValueWithPossibleSideEffects(value.Loc, 2, value) + valueFunc, valueWrapFunc := p.captureValueWithPossibleSideEffects(value.Loc, 2, value, valueDefinitelyNotMutated) assign := valueWrapFunc(targetWrapFunc(p.lowerPrivateSet(target, loc, private, js_ast.Expr{Loc: target.Loc, Data: &js_ast.EBinary{ Op: op, Left: valueFunc(), @@ -1051,7 +1051,7 @@ func (p *parser) lowerPrivateSetUnOp(target js_ast.Expr, loc logger.Loc, private func (p *parser) lowerPrivateSetBinOp(target js_ast.Expr, loc logger.Loc, private *js_ast.EPrivateIdentifier, op js_ast.OpCode, value js_ast.Expr) js_ast.Expr { // "target.#private += 123" => "__privateSet(target, #private, __privateGet(target, #private) + 123)" - targetFunc, targetWrapFunc := p.captureValueWithPossibleSideEffects(target.Loc, 2, target) + targetFunc, targetWrapFunc := p.captureValueWithPossibleSideEffects(target.Loc, 2, target, valueDefinitelyNotMutated) return targetWrapFunc(p.lowerPrivateSet(targetFunc(), loc, private, js_ast.Expr{Loc: value.Loc, Data: &js_ast.EBinary{ Op: op, Left: p.lowerPrivateGet(targetFunc(), loc, private), @@ -1618,7 +1618,7 @@ func (p *parser) lowerClass(stmt js_ast.Stmt, expr js_ast.Expr, shadowRef js_ast // outside the class body. classExpr := &js_ast.EClass{Class: *class} class = &classExpr.Class - nameFunc, wrapFunc = p.captureValueWithPossibleSideEffects(classLoc, 2, js_ast.Expr{Loc: classLoc, Data: classExpr}) + nameFunc, wrapFunc = p.captureValueWithPossibleSideEffects(classLoc, 2, js_ast.Expr{Loc: classLoc, Data: classExpr}, valueDefinitelyNotMutated) expr = nameFunc() didCaptureClassExpr = true name := nameFunc() diff --git a/scripts/end-to-end-tests.js b/scripts/end-to-end-tests.js index 011923a8b78..5f3d59a274b 100644 --- a/scripts/end-to-end-tests.js +++ b/scripts/end-to-end-tests.js @@ -1767,6 +1767,57 @@ new Foo().bar() `, }), + test(['in.js', '--outfile=node.js', '--target=es6'], { + 'in.js': ` + let bar + class Foo { + get #foo() { bar = new Foo; return this.result } + set #foo(x) { this.result = x } + bar() { + bar = this + bar.result = 2 + ++bar.#foo + } + } + let foo = new Foo() + foo.bar() + if (foo === bar || foo.result !== 3 || bar.result !== void 0) throw 'fail' + `, + }), + test(['in.js', '--outfile=node.js', '--target=es6'], { + 'in.js': ` + let bar + class Foo { + get #foo() { bar = new Foo; return this.result } + set #foo(x) { this.result = x } + bar() { + bar = this + bar.result = 2 + bar.#foo *= 3 + } + } + let foo = new Foo() + foo.bar() + if (foo === bar || foo.result !== 6 || bar.result !== void 0) throw 'fail' + `, + }), + test(['in.js', '--outfile=node.js', '--target=es6'], { + 'in.js': ` + let bar + class Foo { + get #foo() { bar = new Foo; return this.result } + set #foo(x) { this.result = x } + bar() { + bar = this + bar.result = 2 + bar.#foo **= 3 + } + } + let foo = new Foo() + foo.bar() + if (foo === bar || foo.result !== 8 || bar.result !== void 0) throw 'fail' + `, + }), test(['in.js', '--outfile=node.js', '--target=es6'], { 'in.js': ` function expect(fn, msg) {