Skip to content

Commit

Permalink
switch boolean flags on symbol to bitfield instead
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Dec 22, 2021
1 parent 665b06d commit 70d1943
Show file tree
Hide file tree
Showing 5 changed files with 116 additions and 103 deletions.
4 changes: 2 additions & 2 deletions internal/bundler/linker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5319,7 +5319,7 @@ func preventBindingsFromBeingRenamed(binding js_ast.Binding, symbols js_ast.Symb
case *js_ast.BMissing:

case *js_ast.BIdentifier:
symbols.Get(b.Ref).MustNotBeRenamed = true
symbols.Get(b.Ref).Flags |= js_ast.MustNotBeRenamed

case *js_ast.BArray:
for _, i := range b.Items {
Expand Down Expand Up @@ -5400,7 +5400,7 @@ func (c *linkerContext) preventExportsFromBeingRenamed(sourceIndex uint32) {
// <script> tag). All symbols in nested scopes are still minified.
if !hasImportOrExport {
for _, member := range repr.AST.ModuleScope.Members {
c.graph.Symbols.Get(member.Ref).MustNotBeRenamed = true
c.graph.Symbols.Get(member.Ref).Flags |= js_ast.MustNotBeRenamed
}
}
}
Expand Down
176 changes: 96 additions & 80 deletions internal/js_ast/js_ast.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1487,6 +1487,86 @@ const (
ImportItemMissing
)

type SymbolFlags uint8

const (
// Certain symbols must not be renamed or minified. For example, the
// "arguments" variable is declared by the runtime for every function.
// Renaming can also break any identifier used inside a "with" statement.
MustNotBeRenamed SymbolFlags = 1 << iota

// In React's version of JSX, lower-case names are strings while upper-case
// names are identifiers. If we are preserving JSX syntax (i.e. not
// transforming it), then we need to be careful to name the identifiers
// something with a capital letter so further JSX processing doesn't treat
// them as strings instead.
MustStartWithCapitalLetterForJSX

// If true, this symbol is the target of a "__name" helper function call.
// This call is special because it deliberately doesn't count as a use
// of the symbol (otherwise keeping names would disable tree shaking)
// so "UseCountEstimate" is not incremented. This flag helps us know to
// avoid optimizing this symbol when "UseCountEstimate" is 1 in this case.
DidKeepName

// Sometimes we lower private symbols even if they are supported. For example,
// consider the following TypeScript code:
//
// class Foo {
// #foo = 123
// bar = this.#foo
// }
//
// If "useDefineForClassFields: false" is set in "tsconfig.json", then "bar"
// must use assignment semantics instead of define semantics. We can compile
// that to this code:
//
// class Foo {
// constructor() {
// this.#foo = 123;
// this.bar = this.#foo;
// }
// #foo;
// }
//
// However, we can't do the same for static fields:
//
// class Foo {
// static #foo = 123
// static bar = this.#foo
// }
//
// Compiling these static fields to something like this would be invalid:
//
// class Foo {
// static #foo;
// }
// Foo.#foo = 123;
// Foo.bar = Foo.#foo;
//
// Thus "#foo" must be lowered even though it's supported. Another case is
// when we're converting top-level class declarations to class expressions
// to avoid the TDZ and the class shadowing symbol is referenced within the
// class body:
//
// class Foo {
// static #foo = Foo
// }
//
// This cannot be converted into something like this:
//
// var Foo = class {
// static #foo;
// };
// Foo.#foo = Foo;
//
PrivateSymbolMustBeLowered
)

func (flags SymbolFlags) Has(flag SymbolFlags) bool {
return (flags & flag) != 0
}

// Note: the order of values in this struct matters to reduce struct size.
type Symbol struct {
// This is the name that came from the parser. Printed names may be renamed
Expand Down Expand Up @@ -1538,25 +1618,6 @@ type Symbol struct {

Kind SymbolKind

// Certain symbols must not be renamed or minified. For example, the
// "arguments" variable is declared by the runtime for every function.
// Renaming can also break any identifier used inside a "with" statement.
MustNotBeRenamed bool

// In React's version of JSX, lower-case names are strings while upper-case
// names are identifiers. If we are preserving JSX syntax (i.e. not
// transforming it), then we need to be careful to name the identifiers
// something with a capital letter so further JSX processing doesn't treat
// them as strings instead.
MustStartWithCapitalLetterForJSX bool

// If true, this symbol is the target of a "__name" helper function call.
// This call is special because it deliberately doesn't count as a use
// of the symbol (otherwise keeping names would disable tree shaking)
// so "UseCountEstimate" is not incremented. This flag helps us know to
// avoid optimizing this symbol when "UseCountEstimate" is 1 in this case.
DidKeepName bool

// We automatically generate import items for property accesses off of
// namespace imports. This lets us remove the expensive namespace imports
// while bundling in many cases, replacing them with a cheap import item
Expand All @@ -1576,58 +1637,20 @@ type Symbol struct {
// undefined, which this status is also used for.
ImportItemStatus ImportItemStatus

// Sometimes we lower private symbols even if they are supported. For example,
// consider the following TypeScript code:
//
// class Foo {
// #foo = 123
// bar = this.#foo
// }
//
// If "useDefineForClassFields: false" is set in "tsconfig.json", then "bar"
// must use assignment semantics instead of define semantics. We can compile
// that to this code:
//
// class Foo {
// constructor() {
// this.#foo = 123;
// this.bar = this.#foo;
// }
// #foo;
// }
//
// However, we can't do the same for static fields:
//
// class Foo {
// static #foo = 123
// static bar = this.#foo
// }
//
// Compiling these static fields to something like this would be invalid:
//
// class Foo {
// static #foo;
// }
// Foo.#foo = 123;
// Foo.bar = Foo.#foo;
//
// Thus "#foo" must be lowered even though it's supported. Another case is
// when we're converting top-level class declarations to class expressions
// to avoid the TDZ and the class shadowing symbol is referenced within the
// class body:
//
// class Foo {
// static #foo = Foo
// }
//
// This cannot be converted into something like this:
//
// var Foo = class {
// static #foo;
// };
// Foo.#foo = Foo;
//
PrivateSymbolMustBeLowered bool
// Boolean values should all be flags instead to save space
Flags SymbolFlags
}

// You should call "MergeSymbols" instead of calling this directly
func (newSymbol *Symbol) MergeContentsWith(oldSymbol *Symbol) {
newSymbol.UseCountEstimate += oldSymbol.UseCountEstimate
if oldSymbol.Flags.Has(MustNotBeRenamed) {
newSymbol.OriginalName = oldSymbol.OriginalName
newSymbol.Flags |= MustNotBeRenamed
}
if oldSymbol.Flags.Has(MustStartWithCapitalLetterForJSX) {
newSymbol.Flags |= MustStartWithCapitalLetterForJSX
}
}

type SlotNamespace uint8
Expand All @@ -1640,7 +1663,7 @@ const (
)

func (s *Symbol) SlotNamespace() SlotNamespace {
if s.Kind == SymbolUnbound || s.MustNotBeRenamed {
if s.Kind == SymbolUnbound || s.Flags.Has(MustNotBeRenamed) {
return SlotMustNotBeRenamed
}
if s.Kind.IsPrivate() {
Expand Down Expand Up @@ -2242,14 +2265,7 @@ func MergeSymbols(symbols SymbolMap, old Ref, new Ref) Ref {
}

oldSymbol.Link = new
newSymbol.UseCountEstimate += oldSymbol.UseCountEstimate
if oldSymbol.MustNotBeRenamed {
newSymbol.OriginalName = oldSymbol.OriginalName
newSymbol.MustNotBeRenamed = true
}
if oldSymbol.MustStartWithCapitalLetterForJSX {
newSymbol.MustStartWithCapitalLetterForJSX = true
}
newSymbol.MergeContentsWith(oldSymbol)
return new
}

Expand Down
25 changes: 11 additions & 14 deletions internal/js_parser/js_parser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1275,7 +1275,7 @@ func (p *parser) popScope() {
continue
}

p.symbols[member.Ref.InnerIndex].MustNotBeRenamed = true
p.symbols[member.Ref.InnerIndex].Flags |= js_ast.MustNotBeRenamed
}
}

Expand Down Expand Up @@ -1370,10 +1370,7 @@ func (p *parser) mergeSymbols(old js_ast.Ref, new js_ast.Ref) {
oldSymbol := &p.symbols[old.InnerIndex]
newSymbol := &p.symbols[new.InnerIndex]
oldSymbol.Link = new
newSymbol.UseCountEstimate += oldSymbol.UseCountEstimate
if oldSymbol.MustNotBeRenamed {
newSymbol.MustNotBeRenamed = true
}
newSymbol.MergeContentsWith(oldSymbol)
}

type mergeResult int
Expand Down Expand Up @@ -1594,7 +1591,7 @@ func (p *parser) hoistSymbols(scope *js_ast.Scope) {
// assert(obj.foo === 2)
//
if s.Kind == js_ast.ScopeWith {
symbol.MustNotBeRenamed = true
symbol.Flags |= js_ast.MustNotBeRenamed
}

if existingMember, ok := s.Members[symbol.OriginalName]; ok {
Expand Down Expand Up @@ -5398,7 +5395,7 @@ func (p *parser) parseFn(name *js_ast.LocRef, data fnOrArrowDataParse) (fn js_as
// be called "arguments", in which case the real "arguments" is inaccessible.
if _, ok := p.currentScope.Members["arguments"]; !ok {
fn.ArgumentsRef = p.declareSymbol(js_ast.SymbolArguments, fn.OpenParenLoc, "arguments")
p.symbols[fn.ArgumentsRef.InnerIndex].MustNotBeRenamed = true
p.symbols[fn.ArgumentsRef.InnerIndex].Flags |= js_ast.MustNotBeRenamed
}

p.lexer.Expect(js_lexer.TCloseParen)
Expand Down Expand Up @@ -7225,7 +7222,7 @@ func (p *parser) findSymbol(loc logger.Loc, name string) findSymbolResult {
// property on the target object of the "with" statement. We must not rename
// it or we risk changing the behavior of the code.
if isInsideWithScope {
p.symbols[ref.InnerIndex].MustNotBeRenamed = true
p.symbols[ref.InnerIndex].Flags |= js_ast.MustNotBeRenamed
}

// Track how many times we've referenced this symbol
Expand Down Expand Up @@ -7603,7 +7600,7 @@ func (p *parser) mangleStmts(stmts []js_ast.Stmt, kind stmtsKind) []js_ast.Stmt
// case there is actually more than one use even though it says
// there is only one. The "__name" use isn't counted so that
// tree shaking still works when names are kept.
if symbol := p.symbols[id.Ref.InnerIndex]; symbol.UseCountEstimate == 1 && !symbol.DidKeepName {
if symbol := p.symbols[id.Ref.InnerIndex]; symbol.UseCountEstimate == 1 && !symbol.Flags.Has(js_ast.DidKeepName) {
// Try to substitute the identifier with the initializer. This will
// fail if something with side effects is in between the declaration
// and the usage.
Expand Down Expand Up @@ -8875,7 +8872,7 @@ func (p *parser) keepExprSymbolName(value js_ast.Expr, name string) js_ast.Expr
}

func (p *parser) keepStmtSymbolName(loc logger.Loc, ref js_ast.Ref, name string) js_ast.Stmt {
p.symbols[ref.InnerIndex].DidKeepName = true
p.symbols[ref.InnerIndex].Flags |= js_ast.DidKeepName

return js_ast.Stmt{Loc: loc, Data: &js_ast.SExpr{
Value: p.callRuntime(loc, "__name", []js_ast.Expr{
Expand Down Expand Up @@ -10002,7 +9999,7 @@ func (p *parser) visitClass(nameScopeLoc logger.Loc, class *js_ast.Class) js_ast
//
// The private getter must be lowered too.
if private, ok := prop.Key.Data.(*js_ast.EPrivateIdentifier); ok {
p.symbols[private.Ref.InnerIndex].PrivateSymbolMustBeLowered = true
p.symbols[private.Ref.InnerIndex].Flags |= js_ast.PrivateSymbolMustBeLowered
}
}
}
Expand All @@ -10013,7 +10010,7 @@ func (p *parser) visitClass(nameScopeLoc logger.Loc, class *js_ast.Class) js_ast
for _, prop := range class.Properties {
if private, ok := prop.Key.Data.(*js_ast.EPrivateIdentifier); ok {
if symbol := &p.symbols[private.Ref.InnerIndex]; p.classPrivateBrandChecksToLower[symbol.OriginalName] {
symbol.PrivateSymbolMustBeLowered = true
symbol.Flags |= js_ast.PrivateSymbolMustBeLowered
}
}
}
Expand Down Expand Up @@ -11345,10 +11342,10 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
// If the tag is an identifier, mark it as needing to be upper-case
switch tag := e.TagOrNil.Data.(type) {
case *js_ast.EIdentifier:
p.symbols[tag.Ref.InnerIndex].MustStartWithCapitalLetterForJSX = true
p.symbols[tag.Ref.InnerIndex].Flags |= js_ast.MustStartWithCapitalLetterForJSX

case *js_ast.EImportIdentifier:
p.symbols[tag.Ref.InnerIndex].MustStartWithCapitalLetterForJSX = true
p.symbols[tag.Ref.InnerIndex].Flags |= js_ast.MustStartWithCapitalLetterForJSX
}
} else {
// A missing tag is a fragment
Expand Down
2 changes: 1 addition & 1 deletion internal/js_parser/js_parser_lower.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ func (p *parser) markLoweredSyntaxFeature(feature compat.JSFeature, r logger.Ran

func (p *parser) privateSymbolNeedsToBeLowered(private *js_ast.EPrivateIdentifier) bool {
symbol := &p.symbols[private.Ref.InnerIndex]
return p.options.unsupportedJSFeatures.Has(symbol.Kind.Feature()) || symbol.PrivateSymbolMustBeLowered
return p.options.unsupportedJSFeatures.Has(symbol.Kind.Feature()) || symbol.Flags.Has(js_ast.PrivateSymbolMustBeLowered)
}

func (p *parser) captureThis() js_ast.Ref {
Expand Down
12 changes: 6 additions & 6 deletions internal/renamer/renamer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,13 @@ func ComputeReservedNames(moduleScopes []*js_ast.Scope, symbols js_ast.SymbolMap
func computeReservedNamesForScope(scope *js_ast.Scope, symbols js_ast.SymbolMap, names map[string]uint32) {
for _, member := range scope.Members {
symbol := symbols.Get(member.Ref)
if symbol.Kind == js_ast.SymbolUnbound || symbol.MustNotBeRenamed {
if symbol.Kind == js_ast.SymbolUnbound || symbol.Flags.Has(js_ast.MustNotBeRenamed) {
names[symbol.OriginalName] = 1
}
}
for _, ref := range scope.Generated {
symbol := symbols.Get(ref)
if symbol.Kind == js_ast.SymbolUnbound || symbol.MustNotBeRenamed {
if symbol.Kind == js_ast.SymbolUnbound || symbol.Flags.Has(js_ast.MustNotBeRenamed) {
names[symbol.OriginalName] = 1
}
}
Expand Down Expand Up @@ -212,7 +212,7 @@ func (r *MinifyRenamer) AccumulateSymbolCount(
// If it is, accumulate the count using a parallel-safe atomic increment
slot := &r.slots[ns][i.GetIndex()]
atomic.AddUint32(&slot.count, count)
if symbol.MustStartWithCapitalLetterForJSX {
if symbol.Flags.Has(js_ast.MustStartWithCapitalLetterForJSX) {
atomic.StoreUint32(&slot.needsCapitalForJSX, 1)
}
return
Expand All @@ -238,12 +238,12 @@ func (r *MinifyRenamer) AllocateTopLevelSymbolSlots(topLevelSymbols DeferredTopL
if i, ok := r.topLevelSymbolToSlot[stable.Ref]; ok {
slot := &(*slots)[i]
slot.count += stable.Count
if symbol.MustStartWithCapitalLetterForJSX {
if symbol.Flags.Has(js_ast.MustStartWithCapitalLetterForJSX) {
slot.needsCapitalForJSX = 1
}
} else {
needsCapitalForJSX := uint32(0)
if symbol.MustStartWithCapitalLetterForJSX {
if symbol.Flags.Has(js_ast.MustStartWithCapitalLetterForJSX) {
needsCapitalForJSX = 1
}
i = uint32(len(*slots))
Expand Down Expand Up @@ -444,7 +444,7 @@ func (r *NumberRenamer) assignName(scope *numberScope, ref js_ast.Ref) {

// Make sure names of symbols used in JSX elements start with a capital letter
originalName := symbol.OriginalName
if symbol.MustStartWithCapitalLetterForJSX {
if symbol.Flags.Has(js_ast.MustStartWithCapitalLetterForJSX) {
if first := rune(originalName[0]); first >= 'a' && first <= 'z' {
originalName = fmt.Sprintf("%c%s", first+('A'-'a'), originalName[1:])
}
Expand Down

0 comments on commit 70d1943

Please sign in to comment.