Skip to content

Commit

Permalink
fix evanw#1600: "++" and "--" on class private fields
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Sep 14, 2021
1 parent 5c10033 commit 9346bc9
Show file tree
Hide file tree
Showing 6 changed files with 253 additions and 81 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@

As of this release, esbuild will now parse a superset of ES5 and ES6+ and will now quote identifier names when possible if it's not considered to be a valid identifier name in either ES5 or ES6+. In other words, a union of ES5 and ES6 rules is used for parsing and the intersection of ES5 and ES6 rules is used for printing.

* Fix `++` and `--` on class private fields when used with big integers ([#1600](https://github.com/evanw/esbuild/issues/1600))

Previously when esbuild lowered class private fields (e.g. `#foo`) to older JavaScript syntax, the transform of the `++` and `--` was not correct if the value is a big integer such as `123n`. The transform in esbuild is similar to Babel's transform which [has the same problem](https://github.com/babel/babel/issues/13756). Specifically, the code was transformed into code that either adds or subtracts the number `1` and `123n + 1` throws an exception in JavaScript. This problem has been fixed so this should now work fine starting with this release.

## 0.12.27

* Update JavaScript syntax feature compatibility tables ([#1594](https://github.com/evanw/esbuild/issues/1594))
Expand Down
54 changes: 24 additions & 30 deletions internal/bundler/snapshots/snapshots_lower.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -481,11 +481,10 @@ class Foo {
__privateAdd(this, _x, void 0);
}
unary() {
var _a, _b;
__privateSet(this, _x, (_a = +__privateGet(this, _x)) + 1), _a;
__privateSet(this, _x, (_b = +__privateGet(this, _x)) - 1), _b;
__privateSet(this, _x, +__privateGet(this, _x) + 1);
__privateSet(this, _x, +__privateGet(this, _x) - 1);
__privateWrapper(this, _x)._++;
__privateWrapper(this, _x)._--;
++__privateWrapper(this, _x)._;
--__privateWrapper(this, _x)._;
}
binary() {
var _a;
Expand Down Expand Up @@ -518,11 +517,10 @@ class Foo {
__privateAdd(this, _x, void 0);
}
unary() {
var _a, _b;
__privateSet(this, _x, (_a = +__privateGet(this, _x)) + 1), _a;
__privateSet(this, _x, (_b = +__privateGet(this, _x)) - 1), _b;
__privateSet(this, _x, +__privateGet(this, _x) + 1);
__privateSet(this, _x, +__privateGet(this, _x) - 1);
__privateWrapper(this, _x)._++;
__privateWrapper(this, _x)._--;
++__privateWrapper(this, _x)._;
--__privateWrapper(this, _x)._;
}
binary() {
var _a;
Expand Down Expand Up @@ -555,11 +553,10 @@ class Foo {
__privateAdd(this, _x, void 0);
}
unary() {
var _a, _b;
__privateSet(this, _x, (_a = +__privateGet(this, _x)) + 1), _a;
__privateSet(this, _x, (_b = +__privateGet(this, _x)) - 1), _b;
__privateSet(this, _x, +__privateGet(this, _x) + 1);
__privateSet(this, _x, +__privateGet(this, _x) - 1);
__privateWrapper(this, _x)._++;
__privateWrapper(this, _x)._--;
++__privateWrapper(this, _x)._;
--__privateWrapper(this, _x)._;
}
binary() {
__privateSet(this, _x, 1);
Expand Down Expand Up @@ -676,11 +673,10 @@ var Foo = class {
__privateSet(fn(), _prop, 2, prop_set);
}
unary(fn) {
var _a, _b, _c, _d, _e, _f;
__privateSet(_a = fn(), _prop, (_b = +__privateGet(_a, _prop, prop_get)) + 1, prop_set), _b;
__privateSet(_c = fn(), _prop, (_d = +__privateGet(_c, _prop, prop_get)) - 1, prop_set), _d;
__privateSet(_e = fn(), _prop, +__privateGet(_e, _prop, prop_get) + 1, prop_set);
__privateSet(_f = fn(), _prop, +__privateGet(_f, _prop, prop_get) - 1, prop_set);
__privateWrapper(fn(), _prop, prop_set, prop_get)._++;
__privateWrapper(fn(), _prop, prop_set, prop_get)._--;
++__privateWrapper(fn(), _prop, prop_set, prop_get)._;
--__privateWrapper(fn(), _prop, prop_set, prop_get)._;
}
binary(fn) {
var _a, _b, _c, _d, _e, _f, _g, _h, _i, _j, _k, _l, _m, _n, _o, _p;
Expand Down Expand Up @@ -736,11 +732,10 @@ var Foo = class {
__privateSet(fn(), _prop, 2, prop_set);
}
unary(fn) {
var _a, _b, _c, _d, _e, _f;
__privateSet(_a = fn(), _prop, (_b = +__privateGet(_a, _prop, prop_get)) + 1, prop_set), _b;
__privateSet(_c = fn(), _prop, (_d = +__privateGet(_c, _prop, prop_get)) - 1, prop_set), _d;
__privateSet(_e = fn(), _prop, +__privateGet(_e, _prop, prop_get) + 1, prop_set);
__privateSet(_f = fn(), _prop, +__privateGet(_f, _prop, prop_get) - 1, prop_set);
__privateWrapper(fn(), _prop, prop_set, prop_get)._++;
__privateWrapper(fn(), _prop, prop_set, prop_get)._--;
++__privateWrapper(fn(), _prop, prop_set, prop_get)._;
--__privateWrapper(fn(), _prop, prop_set, prop_get)._;
}
binary(fn) {
var _a, _b, _c, _d, _e, _f, _g, _h, _i, _j, _k, _l, _m, _n, _o, _p;
Expand Down Expand Up @@ -796,11 +791,10 @@ var Foo = class {
__privateSet(fn(), _prop, 2, prop_set);
}
unary(fn) {
var _a, _b, _c, _d, _e, _f;
__privateSet(_a = fn(), _prop, (_b = +__privateGet(_a, _prop, prop_get)) + 1, prop_set), _b;
__privateSet(_c = fn(), _prop, (_d = +__privateGet(_c, _prop, prop_get)) - 1, prop_set), _d;
__privateSet(_e = fn(), _prop, +__privateGet(_e, _prop, prop_get) + 1, prop_set);
__privateSet(_f = fn(), _prop, +__privateGet(_f, _prop, prop_get) - 1, prop_set);
__privateWrapper(fn(), _prop, prop_set, prop_get)._++;
__privateWrapper(fn(), _prop, prop_set, prop_get)._--;
++__privateWrapper(fn(), _prop, prop_set, prop_get)._;
--__privateWrapper(fn(), _prop, prop_set, prop_get)._;
}
binary(fn) {
var _a, _b, _c, _d, _e, _f, _g, _h, _i, _j, _k, _l, _m, _n, _o;
Expand Down
19 changes: 2 additions & 17 deletions internal/js_parser/js_parser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11553,24 +11553,9 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
////////////////////////////////////////////////////////////////////////////////
// All assignment operators below here

case js_ast.UnOpPreDec:
case js_ast.UnOpPreDec, js_ast.UnOpPreInc, js_ast.UnOpPostDec, js_ast.UnOpPostInc:
if target, loc, private := p.extractPrivateIndex(e.Value); private != nil {
return p.lowerPrivateSetUnOp(target, loc, private, js_ast.BinOpSub, false), exprOut{}
}

case js_ast.UnOpPreInc:
if target, loc, private := p.extractPrivateIndex(e.Value); private != nil {
return p.lowerPrivateSetUnOp(target, loc, private, js_ast.BinOpAdd, false), exprOut{}
}

case js_ast.UnOpPostDec:
if target, loc, private := p.extractPrivateIndex(e.Value); private != nil {
return p.lowerPrivateSetUnOp(target, loc, private, js_ast.BinOpSub, true), exprOut{}
}

case js_ast.UnOpPostInc:
if target, loc, private := p.extractPrivateIndex(e.Value); private != nil {
return p.lowerPrivateSetUnOp(target, loc, private, js_ast.BinOpAdd, true), exprOut{}
return p.lowerPrivateSetUnOp(target, loc, private, e.Op), exprOut{}
}
}
}
Expand Down
82 changes: 50 additions & 32 deletions internal/js_parser/js_parser_lower.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1084,35 +1084,53 @@ 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, valueDefinitelyNotMutated)
target = targetFunc()

// Load the private field and then use the unary "+" operator to force it to
// be a number. Otherwise the binary "+" operator may cause string
// concatenation instead of addition if one of the operands is not a number.
value := js_ast.Expr{Loc: target.Loc, Data: &js_ast.EUnary{
Op: js_ast.UnOpPos,
Value: p.lowerPrivateGet(targetFunc(), loc, private),
}}
func (p *parser) lowerPrivateSetUnOp(target js_ast.Expr, loc logger.Loc, private *js_ast.EPrivateIdentifier, op js_ast.OpCode) js_ast.Expr {
kind := p.symbols[private.Ref.InnerIndex].Kind

if isSuffix {
// "target.#private++" => "__privateSet(target, #private, _a = +__privateGet(target, #private) + 1), _a"
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(),
Right: js_ast.Expr{Loc: target.Loc, Data: &js_ast.ENumber{Value: 1}},
}})))
return js_ast.JoinWithComma(assign, valueFunc())
// Determine the setter, if any
var setter js_ast.Expr
switch kind {
case js_ast.SymbolPrivateSet, js_ast.SymbolPrivateStaticSet,
js_ast.SymbolPrivateGetSetPair, js_ast.SymbolPrivateStaticGetSetPair:
ref := p.privateSetters[private.Ref]
p.recordUsage(ref)
setter = js_ast.Expr{Loc: loc, Data: &js_ast.EIdentifier{Ref: ref}}
}

// "++target.#private" => "__privateSet(target, #private, +__privateGet(target, #private) + 1)"
return targetWrapFunc(p.lowerPrivateSet(target, loc, private, js_ast.Expr{Loc: target.Loc, Data: &js_ast.EBinary{
Op: op,
Left: value,
Right: js_ast.Expr{Loc: target.Loc, Data: &js_ast.ENumber{Value: 1}},
}}))
// Determine the getter, if any
var getter js_ast.Expr
switch kind {
case js_ast.SymbolPrivateGet, js_ast.SymbolPrivateStaticGet,
js_ast.SymbolPrivateGetSetPair, js_ast.SymbolPrivateStaticGetSetPair:
ref := p.privateGetters[private.Ref]
p.recordUsage(ref)
getter = js_ast.Expr{Loc: loc, Data: &js_ast.EIdentifier{Ref: ref}}
}

// Only include necessary arguments
args := []js_ast.Expr{
target,
{Loc: loc, Data: &js_ast.EIdentifier{Ref: private.Ref}},
}
if setter.Data != nil {
args = append(args, setter)
}
if getter.Data != nil {
if setter.Data == nil {
args = append(args, js_ast.Expr{Loc: loc, Data: js_ast.ENullShared})
}
args = append(args, getter)
}

// "target.#private++" => "__privateWrapper(target, #private, private_set, private_get)._++"
return js_ast.Expr{Loc: loc, Data: &js_ast.EUnary{
Op: op,
Value: js_ast.Expr{Loc: target.Loc, Data: &js_ast.EDot{
Target: p.callRuntime(target.Loc, "__privateWrapper", args),
NameLoc: target.Loc,
Name: "_",
}},
}}
}

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 {
Expand Down Expand Up @@ -1305,31 +1323,31 @@ func (p *parser) lowerPrivateInAssign(expr js_ast.Expr) (js_ast.Expr, bool) {
}

case *js_ast.EIndex:
// "[a.#b] = [c]" => "[__privateAssign(a, #b)._] = [c]"
// "[a.#b] = [c]" => "[__privateWrapper(a, #b)._] = [c]"
if private, ok := e.Index.Data.(*js_ast.EPrivateIdentifier); ok && p.privateSymbolNeedsToBeLowered(private) {
var target js_ast.Expr

switch p.symbols[private.Ref.InnerIndex].Kind {
case js_ast.SymbolPrivateSet, js_ast.SymbolPrivateStaticSet,
js_ast.SymbolPrivateGetSetPair, js_ast.SymbolPrivateStaticGetSetPair:
// "this.#setter" => "__privateAssign(this, #setter, setter_set)"
// "this.#setter" => "__privateWrapper(this, #setter, setter_set)"
fnRef := p.privateSetters[private.Ref]
p.recordUsage(fnRef)
target = p.callRuntime(expr.Loc, "__privateAssign", []js_ast.Expr{
target = p.callRuntime(expr.Loc, "__privateWrapper", []js_ast.Expr{
e.Target,
{Loc: expr.Loc, Data: &js_ast.EIdentifier{Ref: private.Ref}},
{Loc: expr.Loc, Data: &js_ast.EIdentifier{Ref: fnRef}},
})

default:
// "this.#field" => "__privateAssign(this, #field)"
target = p.callRuntime(expr.Loc, "__privateAssign", []js_ast.Expr{
// "this.#field" => "__privateWrapper(this, #field)"
target = p.callRuntime(expr.Loc, "__privateWrapper", []js_ast.Expr{
e.Target,
{Loc: expr.Loc, Data: &js_ast.EIdentifier{Ref: private.Ref}},
})
}

// "__privateAssign(this, #field)" => "__privateAssign(this, #field)._"
// "__privateWrapper(this, #field)" => "__privateWrapper(this, #field)._"
expr.Data = &js_ast.EDot{Target: target, Name: "_", NameLoc: expr.Loc}
didLower = true
}
Expand Down
7 changes: 5 additions & 2 deletions internal/runtime/runtime.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,8 +255,11 @@ func code(isES6 bool) string {
setter ? setter.call(obj, value) : member.set(obj, value)
return value
}
export var __privateAssign = (obj, member, setter) => {
return { set _(value) { __privateSet(obj, member, value, setter) } }
export var __privateWrapper = (obj, member, setter, getter) => {
return {
set _(value) { __privateSet(obj, member, value, setter) },
get _() { return __privateGet(obj, member, getter) },
}
}
export var __privateMethod = (obj, member, method) => {
__accessCheck(obj, member, 'access private method')
Expand Down
Loading

0 comments on commit 9346bc9

Please sign in to comment.