Skip to content

Commit

Permalink
remove canonicalization of equality order
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Jul 6, 2022
1 parent 4a00ba9 commit ffbc5c2
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 77 deletions.
48 changes: 24 additions & 24 deletions internal/bundler/snapshots/snapshots_dce.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -358,22 +358,14 @@ TestDCETypeOfEqualsStringGuardCondition
var keep_1 = typeof x != "object" && x;
var keep_1 = typeof x === "object" || x;
var keep_1 = typeof x == "object" || x;
var keep_1 = typeof x !== "object" ? x : null;
var keep_1 = typeof x != "object" ? x : null;
var keep_1 = typeof x === "object" ? null : x;
var keep_1 = typeof x == "object" ? null : x;
var keep_1 = typeof x !== "object" && x;
var keep_1 = typeof x != "object" && x;
var keep_1 = typeof x === "object" || x;
var keep_1 = typeof x == "object" || x;
var keep_2 = typeof x !== "undefined" ? y : null;
var keep_2 = typeof x != "undefined" ? y : null;
var keep_2 = typeof x === "undefined" ? null : y;
var keep_2 = typeof x == "undefined" ? null : y;
var keep_2 = typeof x !== "undefined" && y;
var keep_2 = typeof x != "undefined" && y;
var keep_2 = typeof x === "undefined" || y;
var keep_2 = typeof x == "undefined" || y;
var keep_1 = "object" !== typeof x ? x : null;
var keep_1 = "object" != typeof x ? x : null;
var keep_1 = "object" === typeof x ? null : x;
var keep_1 = "object" == typeof x ? null : x;
var keep_1 = "object" !== typeof x && x;
var keep_1 = "object" != typeof x && x;
var keep_1 = "object" === typeof x || x;
var keep_1 = "object" == typeof x || x;
var keep_2 = typeof x !== "undefined" ? y : null;
var keep_2 = typeof x != "undefined" ? y : null;
var keep_2 = typeof x === "undefined" ? null : y;
Expand All @@ -382,6 +374,14 @@ TestDCETypeOfEqualsStringGuardCondition
var keep_2 = typeof x != "undefined" && y;
var keep_2 = typeof x === "undefined" || y;
var keep_2 = typeof x == "undefined" || y;
var keep_2 = "undefined" !== typeof x ? y : null;
var keep_2 = "undefined" != typeof x ? y : null;
var keep_2 = "undefined" === typeof x ? null : y;
var keep_2 = "undefined" == typeof x ? null : y;
var keep_2 = "undefined" !== typeof x && y;
var keep_2 = "undefined" != typeof x && y;
var keep_2 = "undefined" === typeof x || y;
var keep_2 = "undefined" == typeof x || y;
var keep_3 = typeof x !== "undefined" ? null : x;
var keep_3 = typeof x != "undefined" ? null : x;
var keep_3 = typeof x === "undefined" ? x : null;
Expand All @@ -390,14 +390,14 @@ TestDCETypeOfEqualsStringGuardCondition
var keep_3 = typeof x != "undefined" || x;
var keep_3 = typeof x === "undefined" && x;
var keep_3 = typeof x == "undefined" && x;
var keep_3 = typeof x !== "undefined" ? null : x;
var keep_3 = typeof x != "undefined" ? null : x;
var keep_3 = typeof x === "undefined" ? x : null;
var keep_3 = typeof x == "undefined" ? x : null;
var keep_3 = typeof x !== "undefined" || x;
var keep_3 = typeof x != "undefined" || x;
var keep_3 = typeof x === "undefined" && x;
var keep_3 = typeof x == "undefined" && x;
var keep_3 = "undefined" !== typeof x ? null : x;
var keep_3 = "undefined" != typeof x ? null : x;
var keep_3 = "undefined" === typeof x ? x : null;
var keep_3 = "undefined" == typeof x ? x : null;
var keep_3 = "undefined" !== typeof x || x;
var keep_3 = "undefined" != typeof x || x;
var keep_3 = "undefined" === typeof x && x;
var keep_3 = "undefined" == typeof x && x;
})();

================================================================================
Expand Down
96 changes: 61 additions & 35 deletions internal/js_parser/js_parser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9666,7 +9666,7 @@ func (p *parser) visitAndAppendStmt(stmts []js_ast.Stmt, stmt js_ast.Stmt) []js_
if c.ValueOrNil.Data != nil {
c.ValueOrNil = p.visitExpr(c.ValueOrNil)
p.warnAboutEqualityCheck("case", c.ValueOrNil, c.ValueOrNil.Loc)
p.warnAboutTypeofAndString(s.Test, c.ValueOrNil)
p.warnAboutTypeofAndString(s.Test, c.ValueOrNil, onlyCheckOriginalOrder)
}
c.Body = p.visitStmts(c.Body, stmtsNormal)

Expand Down Expand Up @@ -10820,7 +10820,20 @@ func (p *parser) checkForUnrepresentableIdentifier(loc logger.Loc, name string)
}
}

func (p *parser) warnAboutTypeofAndString(a js_ast.Expr, b js_ast.Expr) {
type typeofStringOrder uint8

const (
onlyCheckOriginalOrder typeofStringOrder = iota
checkBothOrders
)

func (p *parser) warnAboutTypeofAndString(a js_ast.Expr, b js_ast.Expr, order typeofStringOrder) {
if order == checkBothOrders {
if _, ok := a.Data.(*js_ast.EString); ok {
a, b = b, a
}
}

if typeof, ok := a.Data.(*js_ast.EUnary); ok && typeof.Op == js_ast.UnOpTypeof {
if str, ok := b.Data.(*js_ast.EString); ok {
value := helpers.UTF16ToString(str.Value)
Expand Down Expand Up @@ -10856,15 +10869,22 @@ func canChangeStrictToLoose(a js_ast.Expr, b js_ast.Expr) bool {
}

func (p *parser) maybeSimplifyEqualityComparison(loc logger.Loc, e *js_ast.EBinary) (js_ast.Expr, bool) {
value, primitive := e.Left, e.Right

// Detect when the primitive comes first and flip the order of our checks
if isPrimitiveLiteral(value.Data) {
value, primitive = primitive, value
}

// "!x === true" => "!x"
// "!x === false" => "!!x"
// "!x !== true" => "!!x"
// "!x !== false" => "!x"
if boolean, ok := e.Right.Data.(*js_ast.EBoolean); ok && js_ast.KnownPrimitiveType(e.Left) == js_ast.PrimitiveBoolean {
if boolean, ok := primitive.Data.(*js_ast.EBoolean); ok && js_ast.KnownPrimitiveType(value) == js_ast.PrimitiveBoolean {
if boolean.Value == (e.Op == js_ast.BinOpLooseNe || e.Op == js_ast.BinOpStrictNe) {
return js_ast.Not(e.Left), true
return js_ast.Not(value), true
} else {
return e.Left, true
return value, true
}
}

Expand All @@ -10875,17 +10895,18 @@ func (p *parser) maybeSimplifyEqualityComparison(loc logger.Loc, e *js_ast.EBina
// return something random. The only case of this happening was Internet
// Explorer returning "unknown" for some objects, which messes with this
// optimization. So we don't do this when targeting Internet Explorer.
if typeof, ok := e.Left.Data.(*js_ast.EUnary); ok && typeof.Op == js_ast.UnOpTypeof {
if str, ok := e.Right.Data.(*js_ast.EString); ok && helpers.UTF16EqualsString(str.Value, "undefined") {
if typeof, ok := value.Data.(*js_ast.EUnary); ok && typeof.Op == js_ast.UnOpTypeof {
if str, ok := primitive.Data.(*js_ast.EString); ok && helpers.UTF16EqualsString(str.Value, "undefined") {
flip := value == e.Right
op := js_ast.BinOpLt
if e.Op == js_ast.BinOpLooseEq || e.Op == js_ast.BinOpStrictEq {
if (e.Op == js_ast.BinOpLooseEq || e.Op == js_ast.BinOpStrictEq) != flip {
op = js_ast.BinOpGt
}
return js_ast.Expr{Loc: loc, Data: &js_ast.EBinary{
Op: op,
Left: e.Left,
Right: js_ast.Expr{Loc: e.Right.Loc, Data: &js_ast.EString{Value: []uint16{'u'}}},
}}, true
primitive.Data = &js_ast.EString{Value: []uint16{'u'}}
if flip {
value, primitive = primitive, value
}
return js_ast.Expr{Loc: loc, Data: &js_ast.EBinary{Op: op, Left: value, Right: primitive}}, true
}
}
}
Expand Down Expand Up @@ -11477,18 +11498,29 @@ func stringToEquivalentNumberValue(value []uint16) (float64, bool) {
func isBinaryNullAndUndefined(left js_ast.Expr, right js_ast.Expr, op js_ast.OpCode) (js_ast.Expr, js_ast.Expr, bool) {
if a, ok := left.Data.(*js_ast.EBinary); ok && a.Op == op {
if b, ok := right.Data.(*js_ast.EBinary); ok && b.Op == op {
if idA, ok := a.Left.Data.(*js_ast.EIdentifier); ok {
if idB, ok := b.Left.Data.(*js_ast.EIdentifier); ok && idA.Ref == idB.Ref {
idA, eqA := a.Left, a.Right
idB, eqB := b.Left, b.Right

// Detect when the identifier comes second and flip the order of our checks
if _, ok := eqA.Data.(*js_ast.EIdentifier); ok {
idA, eqA = eqA, idA
}
if _, ok := eqB.Data.(*js_ast.EIdentifier); ok {
idB, eqB = eqB, idB
}

if idA, ok := idA.Data.(*js_ast.EIdentifier); ok {
if idB, ok := idB.Data.(*js_ast.EIdentifier); ok && idA.Ref == idB.Ref {
// "a === null || a === void 0"
if _, ok := a.Right.Data.(*js_ast.ENull); ok {
if _, ok := b.Right.Data.(*js_ast.EUndefined); ok {
if _, ok := eqA.Data.(*js_ast.ENull); ok {
if _, ok := eqB.Data.(*js_ast.EUndefined); ok {
return a.Left, a.Right, true
}
}

// "a === void 0 || a === null"
if _, ok := a.Right.Data.(*js_ast.EUndefined); ok {
if _, ok := b.Right.Data.(*js_ast.ENull); ok {
if _, ok := eqA.Data.(*js_ast.EUndefined); ok {
if _, ok := eqB.Data.(*js_ast.ENull); ok {
return b.Left, b.Right, true
}
}
Expand Down Expand Up @@ -12164,16 +12196,6 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
}
p.fnOnlyDataVisit.silenceWarningAboutThisBeingUndefined = oldSilenceWarningAboutThisBeingUndefined

// Always put constants on the right for equality comparisons to help
// reduce the number of cases we have to check during pattern matching. We
// can only reorder expressions that do not have any side effects.
switch e.Op {
case js_ast.BinOpLooseEq, js_ast.BinOpLooseNe, js_ast.BinOpStrictEq, js_ast.BinOpStrictNe:
if isPrimitiveLiteral(e.Left.Data) && !isPrimitiveLiteral(e.Right.Data) {
e.Left, e.Right = e.Right, e.Left
}
}

// Post-process the binary expression
switch e.Op {
case js_ast.BinOpComma:
Expand All @@ -12200,11 +12222,13 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
if !p.warnAboutEqualityCheck("==", e.Left, afterOpLoc) {
p.warnAboutEqualityCheck("==", e.Right, afterOpLoc)
}
p.warnAboutTypeofAndString(e.Left, e.Right)
p.warnAboutTypeofAndString(e.Left, e.Right, checkBothOrders)

if p.options.minifySyntax {
// "x == void 0" => "x == null"
if _, ok := e.Right.Data.(*js_ast.EUndefined); ok {
if _, ok := e.Left.Data.(*js_ast.EUndefined); ok {
e.Left.Data = js_ast.ENullShared
} else if _, ok := e.Right.Data.(*js_ast.EUndefined); ok {
e.Right.Data = js_ast.ENullShared
}

Expand All @@ -12221,7 +12245,7 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
if !p.warnAboutEqualityCheck("===", e.Left, afterOpLoc) {
p.warnAboutEqualityCheck("===", e.Right, afterOpLoc)
}
p.warnAboutTypeofAndString(e.Left, e.Right)
p.warnAboutTypeofAndString(e.Left, e.Right, checkBothOrders)

if p.options.minifySyntax {
// "typeof x === 'undefined'" => "typeof x == 'undefined'"
Expand All @@ -12242,11 +12266,13 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
if !p.warnAboutEqualityCheck("!=", e.Left, afterOpLoc) {
p.warnAboutEqualityCheck("!=", e.Right, afterOpLoc)
}
p.warnAboutTypeofAndString(e.Left, e.Right)
p.warnAboutTypeofAndString(e.Left, e.Right, checkBothOrders)

if p.options.minifySyntax {
// "x != void 0" => "x != null"
if _, ok := e.Right.Data.(*js_ast.EUndefined); ok {
if _, ok := e.Left.Data.(*js_ast.EUndefined); ok {
e.Left.Data = js_ast.ENullShared
} else if _, ok := e.Right.Data.(*js_ast.EUndefined); ok {
e.Right.Data = js_ast.ENullShared
}

Expand All @@ -12263,7 +12289,7 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
if !p.warnAboutEqualityCheck("!==", e.Left, afterOpLoc) {
p.warnAboutEqualityCheck("!==", e.Right, afterOpLoc)
}
p.warnAboutTypeofAndString(e.Left, e.Right)
p.warnAboutTypeofAndString(e.Left, e.Right, checkBothOrders)

if p.options.minifySyntax {
// "typeof x !== 'undefined'" => "typeof x != 'undefined'"
Expand Down
36 changes: 18 additions & 18 deletions internal/js_parser/js_parser_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3172,20 +3172,20 @@ func TestMangleIf(t *testing.T) {

expectPrintedMangle(t, "let b; a = null == b ? c : b", "let b;\na = b ?? c;\n")
expectPrintedMangle(t, "let b; a = null != b ? b : c", "let b;\na = b ?? c;\n")
expectPrintedMangle(t, "let b; a = null == b ? b : c", "let b;\na = b == null ? b : c;\n")
expectPrintedMangle(t, "let b; a = null != b ? c : b", "let b;\na = b != null ? c : b;\n")
expectPrintedMangle(t, "let b; a = null == b ? b : c", "let b;\na = null == b ? b : c;\n")
expectPrintedMangle(t, "let b; a = null != b ? c : b", "let b;\na = null != b ? c : b;\n")

// Don't do this if the condition has side effects
expectPrintedMangle(t, "let b; a = b.x == null ? c : b.x", "let b;\na = b.x == null ? c : b.x;\n")
expectPrintedMangle(t, "let b; a = b.x != null ? b.x : c", "let b;\na = b.x != null ? b.x : c;\n")
expectPrintedMangle(t, "let b; a = null == b.x ? c : b.x", "let b;\na = b.x == null ? c : b.x;\n")
expectPrintedMangle(t, "let b; a = null != b.x ? b.x : c", "let b;\na = b.x != null ? b.x : c;\n")
expectPrintedMangle(t, "let b; a = null == b.x ? c : b.x", "let b;\na = null == b.x ? c : b.x;\n")
expectPrintedMangle(t, "let b; a = null != b.x ? b.x : c", "let b;\na = null != b.x ? b.x : c;\n")

// Don't do this for strict equality comparisons
expectPrintedMangle(t, "let b; a = b === null ? c : b", "let b;\na = b === null ? c : b;\n")
expectPrintedMangle(t, "let b; a = b !== null ? b : c", "let b;\na = b !== null ? b : c;\n")
expectPrintedMangle(t, "let b; a = null === b ? c : b", "let b;\na = b === null ? c : b;\n")
expectPrintedMangle(t, "let b; a = null !== b ? b : c", "let b;\na = b !== null ? b : c;\n")
expectPrintedMangle(t, "let b; a = null === b ? c : b", "let b;\na = null === b ? c : b;\n")
expectPrintedMangle(t, "let b; a = null !== b ? b : c", "let b;\na = null !== b ? b : c;\n")

expectPrintedMangle(t, "let b; a = null === b || b === undefined ? c : b", "let b;\na = b ?? c;\n")
expectPrintedMangle(t, "let b; a = b !== undefined && b !== null ? b : c", "let b;\na = b ?? c;\n")
Expand Down Expand Up @@ -3317,8 +3317,8 @@ func TestMangleOptionalChain(t *testing.T) {

expectPrintedMangle(t, "a == null && a.b()", "a == null && a.b();\n")
expectPrintedMangle(t, "a != null || a.b()", "a != null || a.b();\n")
expectPrintedMangle(t, "null == a && a.b()", "a == null && a.b();\n")
expectPrintedMangle(t, "null != a || a.b()", "a != null || a.b();\n")
expectPrintedMangle(t, "null == a && a.b()", "null == a && a.b();\n")
expectPrintedMangle(t, "null != a || a.b()", "null != a || a.b();\n")

expectPrintedMangle(t, "x = a != null && a.b()", "x = a != null && a.b();\n")
expectPrintedMangle(t, "x = a == null || a.b()", "x = a == null || a.b();\n")
Expand Down Expand Up @@ -3746,13 +3746,13 @@ func TestMangleTypeofIdentifier(t *testing.T) {
func TestMangleTypeofEqualsUndefined(t *testing.T) {
expectPrintedMangle(t, "return typeof x !== 'undefined'", "return typeof x < \"u\";\n")
expectPrintedMangle(t, "return typeof x != 'undefined'", "return typeof x < \"u\";\n")
expectPrintedMangle(t, "return 'undefined' !== typeof x", "return typeof x < \"u\";\n")
expectPrintedMangle(t, "return 'undefined' != typeof x", "return typeof x < \"u\";\n")
expectPrintedMangle(t, "return 'undefined' !== typeof x", "return \"u\" > typeof x;\n")
expectPrintedMangle(t, "return 'undefined' != typeof x", "return \"u\" > typeof x;\n")

expectPrintedMangle(t, "return typeof x === 'undefined'", "return typeof x > \"u\";\n")
expectPrintedMangle(t, "return typeof x == 'undefined'", "return typeof x > \"u\";\n")
expectPrintedMangle(t, "return 'undefined' === typeof x", "return typeof x > \"u\";\n")
expectPrintedMangle(t, "return 'undefined' == typeof x", "return typeof x > \"u\";\n")
expectPrintedMangle(t, "return 'undefined' === typeof x", "return \"u\" < typeof x;\n")
expectPrintedMangle(t, "return 'undefined' == typeof x", "return \"u\" < typeof x;\n")
}

func TestMangleEquals(t *testing.T) {
Expand All @@ -3763,8 +3763,8 @@ func TestMangleEquals(t *testing.T) {

expectPrintedMangle(t, "return typeof x === 'string'", "return typeof x == \"string\";\n")
expectPrintedMangle(t, "return typeof x !== 'string'", "return typeof x != \"string\";\n")
expectPrintedMangle(t, "return 'string' === typeof x", "return typeof x == \"string\";\n")
expectPrintedMangle(t, "return 'string' !== typeof x", "return typeof x != \"string\";\n")
expectPrintedMangle(t, "return 'string' === typeof x", "return \"string\" == typeof x;\n")
expectPrintedMangle(t, "return 'string' !== typeof x", "return \"string\" != typeof x;\n")

expectPrintedMangle(t, "return a === 0", "return a === 0;\n")
expectPrintedMangle(t, "return a !== 0", "return a !== 0;\n")
Expand Down Expand Up @@ -3890,13 +3890,13 @@ func TestMangleNestedLogical(t *testing.T) {
func TestMangleEqualsUndefined(t *testing.T) {
expectPrintedMangle(t, "return a === void 0", "return a === void 0;\n")
expectPrintedMangle(t, "return a !== void 0", "return a !== void 0;\n")
expectPrintedMangle(t, "return void 0 === a", "return a === void 0;\n")
expectPrintedMangle(t, "return void 0 !== a", "return a !== void 0;\n")
expectPrintedMangle(t, "return void 0 === a", "return void 0 === a;\n")
expectPrintedMangle(t, "return void 0 !== a", "return void 0 !== a;\n")

expectPrintedMangle(t, "return a == void 0", "return a == null;\n")
expectPrintedMangle(t, "return a != void 0", "return a != null;\n")
expectPrintedMangle(t, "return void 0 == a", "return a == null;\n")
expectPrintedMangle(t, "return void 0 != a", "return a != null;\n")
expectPrintedMangle(t, "return void 0 == a", "return null == a;\n")
expectPrintedMangle(t, "return void 0 != a", "return null != a;\n")

expectPrintedMangle(t, "return a === null || a === undefined", "return a == null;\n")
expectPrintedMangle(t, "return a === null || a !== undefined", "return a === null || a !== void 0;\n")
Expand Down

0 comments on commit ffbc5c2

Please sign in to comment.