Skip to content

Commit

Permalink
add optional mutation to value capture
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Mar 12, 2021
1 parent b1a394b commit 98ef59c
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 26 deletions.
28 changes: 19 additions & 9 deletions internal/js_parser/js_parser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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),
Expand Down
34 changes: 17 additions & 17 deletions internal/js_parser/js_parser_lower.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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()

Expand All @@ -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()
}

Expand All @@ -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()
}
}
Expand Down Expand Up @@ -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(),
Expand All @@ -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(),
Expand Down Expand Up @@ -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),
Expand All @@ -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),
Expand All @@ -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),
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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(),
Expand All @@ -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),
Expand Down Expand Up @@ -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()
Expand Down
51 changes: 51 additions & 0 deletions scripts/end-to-end-tests.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 98ef59c

Please sign in to comment.