Skip to content

Commit

Permalink
Initial implementation of ReplaceToggles pass, to support `goog.tog…
Browse files Browse the repository at this point in the history
…gle` (now called `goog.readToggleInternalDoNotCallDirectly`).

PiperOrigin-RevId: 542741312
  • Loading branch information
shicks authored and copybara-github committed Jun 23, 2023
1 parent e13f5cd commit a685a90
Show file tree
Hide file tree
Showing 8 changed files with 521 additions and 0 deletions.
12 changes: 12 additions & 0 deletions src/com/google/javascript/jscomp/AbstractCompiler.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.google.common.base.Strings;
import com.google.common.base.Supplier;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.errorprone.annotations.MustBeClosed;
import com.google.errorprone.annotations.OverridingMethodsMustInvokeSuper;
Expand Down Expand Up @@ -687,4 +688,15 @@ public final LogFile createOrReopenIndexedLog(
* rewriting has not occurred.
*/
abstract void mergeSyntheticCodeInput();

/**
* Records the mapping of toggle names to ordinals, which is read from a bootstrap JS file by the
* first (check) pass of {@link RewriteToggles}.
*/
void setToggleOrdinalMapping(@Nullable ImmutableMap<String, Integer> mapping) {}

/** Returns the recorded toggle-name-to-ordinal mapping. */
@Nullable ImmutableMap<String, Integer> getToggleOrdinalMapping() {
return null;
}
}
12 changes: 12 additions & 0 deletions src/com/google/javascript/jscomp/AstFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -960,6 +960,18 @@ Node createLessThan(Node left, Node right) {
return result;
}

Node createBitwiseAnd(Node left, Node right) {
Node result = IR.bitwiseAnd(left, right);
setJSTypeOrColor(type(JSTypeNative.NUMBER_TYPE, StandardColors.NUMBER), result);
return result;
}

Node createRightShift(Node left, Node right) {
Node result = IR.rightShift(left, right);
setJSTypeOrColor(type(JSTypeNative.NUMBER_TYPE, StandardColors.NUMBER), result);
return result;
}

Node createCall(Node callee, Type resultType, Node... args) {
Node result = NodeUtil.newCallNode(callee, args);
setJSTypeOrColor(resultType, result);
Expand Down
12 changes: 12 additions & 0 deletions src/com/google/javascript/jscomp/Compiler.java
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,8 @@ public SourceFile loadSource(String filename) {

private final Timeline<Node> changeTimeline = new Timeline<>();

private @Nullable ImmutableMap<String, Integer> toggleOrdinalMapping = null;

/**
* When mapping symbols from a source map, we must repeatedly combine the path of the original
* file with the path from the source map to compute the SourceFile of the underlying code. When
Expand Down Expand Up @@ -4286,4 +4288,14 @@ private static Node checkNotModule(Node script, String msg, Object... args) {
checkState(!script.getFirstChild().isModuleBody(), msg, args);
return script;
}

@Override
void setToggleOrdinalMapping(@Nullable ImmutableMap<String, Integer> mapping) {
this.toggleOrdinalMapping = mapping;
}

@Override
@Nullable ImmutableMap<String, Integer> getToggleOrdinalMapping() {
return toggleOrdinalMapping;
}
}
20 changes: 20 additions & 0 deletions src/com/google/javascript/jscomp/DefaultPassConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,7 @@ protected PassListBuilder getChecks() {

if (options.closurePass) {
checks.maybeAdd(closurePrimitives);
checks.maybeAdd(replaceTogglesCheck);
}

// It's important that the PolymerPass run *after* the ClosurePrimitives and ChromePass rewrites
Expand Down Expand Up @@ -634,6 +635,7 @@ protected PassListBuilder getOptimizations() {
passes.maybeAdd(j2clAssertRemovalPass);
}

passes.maybeAdd(replaceToggles);
passes.maybeAdd(inlineAndCollapseProperties);

if (options.getTweakProcessing().shouldStrip()
Expand Down Expand Up @@ -2079,6 +2081,24 @@ public void process(Node externs, Node jsRoot) {
.setInternalFactory(ConstParamCheck::new)
.build();

private final PassFactory replaceTogglesCheck = createReplaceToggles(true);
private final PassFactory replaceToggles = createReplaceToggles(false);

/** Replaces goog.toggle calls with toggle lookups. */
private final PassFactory createReplaceToggles(boolean check) {
return PassFactory.builder()
.setName("replaceToggles" + (check ? "Check" : ""))
.setInternalFactory(
(compiler) ->
new CompilerPass() {
@Override
public void process(Node externs, Node root) {
new ReplaceToggles(compiler, check).process(externs, root);
}
})
.build();
}

/** Generates unique ids. */
private final PassFactory replaceIdGenerators =
PassFactory.builder()
Expand Down
185 changes: 185 additions & 0 deletions src/com/google/javascript/jscomp/ReplaceToggles.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,185 @@
/*
* Copyright 2023 The Closure Compiler Authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.google.javascript.jscomp;

import com.google.common.collect.ImmutableMap;
import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback;
import com.google.javascript.rhino.Node;
import com.google.javascript.rhino.QualifiedName;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Set;

/**
* Replaces calls to id generators with ids.
*
* <p>Use this to get unique and short ids.
*/
class ReplaceToggles implements CompilerPass {
static final DiagnosticType INVALID_TOGGLE_PARAMETER =
DiagnosticType.error(
"JSC_INVALID_TOGGLE_PARAMETER",
"goog.readToggleInternalDoNotCallDirectly must be called with a string literal.");

static final DiagnosticType INVALID_ORDINAL_MAPPING =
DiagnosticType.error(
"JSC_INVALID_ORDINAL_MAPPING",
"_F_toggleOrdinals must be initialized with an object literal mapping strings to unique"
+ " integers: {0}");

static final DiagnosticType UNKNOWN_TOGGLE =
DiagnosticType.error(
"JSC_UNKNOWN_TOGGLE",
"goog.readToggleInternalDoNotCallDirectly called with an unknown toggle. If a toggle"
+ " list is given, it must be exhaustive.");

private final AbstractCompiler compiler;
private final AstFactory astFactory;
private final boolean check;

ReplaceToggles(AbstractCompiler compiler, boolean check) {
this.compiler = compiler;
this.astFactory = compiler.createAstFactory();
this.check = check;
}

@Override
public void process(Node externs, Node root) {
NodeTraversal.traverse(compiler, root, new Traversal());
}

private static final String ORDINAL_VAR_NAME = "_F_toggleOrdinals";
private static final QualifiedName readToggleFunctionName =
QualifiedName.of("goog.readToggleInternalDoNotCallDirectly");

private class Traversal extends AbstractPostOrderCallback {

@Override
public void visit(NodeTraversal t, Node n, Node parent) {
// Look for `var _F_toggleOrdinals = {...};`. Note that we only do this in check mode.
// It's not our responsibility to delete the unused variable in optimized mode - if all
// the calls to readToggle are deleted, then the bootstrap will be unused and deleted, too.
if (check
&& NodeUtil.isNameDeclaration(n)
&& n.getFirstChild().matchesName(ORDINAL_VAR_NAME)) {
Node rhs = n.getFirstFirstChild();

if (rhs == null || !rhs.isObjectLit()) {
compiler.report(JSError.make(n, INVALID_ORDINAL_MAPPING, "not an object literal"));
return;
}

Map<String, Integer> mapping = new LinkedHashMap<>();
Set<Integer> ordinals = new HashSet<>();
for (Node c = rhs.getFirstChild(); c != null; c = c.getNext()) {
if (!c.isStringKey() && !c.isStringLit()) {
compiler.report(JSError.make(c, INVALID_ORDINAL_MAPPING, "non-string key"));
return;
}
String key = c.getString();
if (mapping.containsKey(key)) {
compiler.report(JSError.make(c, INVALID_ORDINAL_MAPPING, "duplicate key: " + key));
return;
}
Node value = c.getFirstChild();
if (value == null || !value.isNumber()) {
compiler.report(
JSError.make(c, INVALID_ORDINAL_MAPPING, "value not a whole number literal"));
return;
}
double doubleValue = value.getDouble();
int intValue = (int) doubleValue;
if (doubleValue != intValue) {
compiler.report(
JSError.make(c, INVALID_ORDINAL_MAPPING, "value not a whole number literal"));
return;
} else if (ordinals.contains(intValue)) {
compiler.report(
JSError.make(c, INVALID_ORDINAL_MAPPING, "duplicate ordinal: " + intValue));
return;
}
mapping.put(key, intValue);
ordinals.add(intValue);
}
compiler.setToggleOrdinalMapping(ImmutableMap.copyOf(mapping));

// NOTE: We do not support a simple assignment without `var` since reassignment or later
// augmentation is not allowed.
return;
}

if (!n.isCall()) {
return;
}
Node qname = NodeUtil.getCallTargetResolvingIndirectCalls(n);
if (!readToggleFunctionName.matches(qname)) {
return;
}

Node arg = n.getSecondChild();
if (arg == null || !arg.isStringLit() || !n.hasTwoChildren()) {
compiler.report(JSError.make(n, INVALID_TOGGLE_PARAMETER));
return;
}

ImmutableMap<String, Integer> toggles = compiler.getToggleOrdinalMapping();
if (check) {
if (toggles != null && !toggles.containsKey(arg.getString())) {
compiler.report(JSError.make(n, UNKNOWN_TOGGLE));
}
return;
}

Integer ordinal = toggles != null ? toggles.get(arg.getString()) : null;
if (ordinal == null) {
// No ordinals given: hard-code `false` for all toggles
n.replaceWith(astFactory.createBoolean(false).srcrefTreeIfMissing(n));
t.reportCodeChange();
return;
}

// Replace with a lookup into the data structure
// Note: the choice of 30 here is per spec, since the bootstrap must be written in agreement
// with this convention. 30 was chosen over 32 to ensure all numbers are SMI and will fit
// neatly in 4 bytes, whereas larger ints require more space in the VM.
int index = ordinal / 30;
int bit = ordinal % 30;
Node getElem =
astFactory.createGetElem(
astFactory.createQNameWithUnknownType("goog.TOGGLES_"),
astFactory.createNumber(index));
// There are two different ways to write the bitwiseAnd:
// 1. `getElem & mask`
// 2. `getElem >> bit & 1` (note: sign extension is irrelevant here, and `>>` is shorter)
// where x is the `goog.TOGGLES_[index]` lookup, and `mask` is `1 << bit`.
// When `bit < 14`, option 1 is shorter. When `bit > 16`, option 2 is shorter.
// We arbitrarily prefer option 2 in the break-even range 14..16.
Node bitAnd;
if (bit < 14) {
bitAnd = astFactory.createBitwiseAnd(getElem, astFactory.createNumber(1 << bit));
} else {
bitAnd =
astFactory.createBitwiseAnd(
astFactory.createRightShift(getElem, astFactory.createNumber(bit)),
astFactory.createNumber(1));
}
n.replaceWith(astFactory.createNot(astFactory.createNot(bitAnd)).srcrefTreeIfMissing(n));
t.reportCodeChange();
}
}
}
10 changes: 10 additions & 0 deletions src/com/google/javascript/rhino/IR.java
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,16 @@ public static Node assignCoalesce(Node expr1, Node expr2) {
return binaryOp(Token.ASSIGN_COALESCE, expr1, expr2);
}

/** "&" */
public static Node bitwiseAnd(Node expr1, Node expr2) {
return binaryOp(Token.BITAND, expr1, expr2);
}

/** ">>" */
public static Node rightShift(Node expr1, Node expr2) {
return binaryOp(Token.RSH, expr1, expr2);
}

// TODO(johnlenz): the rest of the ops

// literals
Expand Down
56 changes: 56 additions & 0 deletions test/com/google/javascript/jscomp/AstFactoryTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -938,6 +938,62 @@ public void testCreateLessThan_colors() {
assertNode(lt).hasColorThat().isEqualTo(StandardColors.BOOLEAN);
}

@Test
public void testCreateBitwiseAnd_jstypes() {
AstFactory astFactory = createTestAstFactory();

Node zero = astFactory.createNumber(0);
Node one = astFactory.createNumber(1);

Node bitAnd = astFactory.createBitwiseAnd(zero, one);

assertNode(bitAnd).hasToken(Token.BITAND);
assertThat(childList(bitAnd)).containsExactly(zero, one);
assertNode(bitAnd).hasJSTypeThat().isEqualTo(getNativeType(JSTypeNative.NUMBER_TYPE));
}

@Test
public void testCreateBitwiseAnd_colors() {
AstFactory astFactory = createTestAstFactoryWithColors();

Node zero = astFactory.createNumber(0);
Node one = astFactory.createNumber(1);

Node bitAnd = astFactory.createBitwiseAnd(zero, one);

assertNode(bitAnd).hasToken(Token.BITAND);
assertThat(childList(bitAnd)).containsExactly(zero, one);
assertNode(bitAnd).hasColorThat().isEqualTo(StandardColors.NUMBER);
}

@Test
public void testCreateRightShift_jstypes() {
AstFactory astFactory = createTestAstFactory();

Node zero = astFactory.createNumber(0);
Node one = astFactory.createNumber(1);

Node rightShift = astFactory.createRightShift(zero, one);

assertNode(rightShift).hasToken(Token.RSH);
assertThat(childList(rightShift)).containsExactly(zero, one);
assertNode(rightShift).hasJSTypeThat().isEqualTo(getNativeType(JSTypeNative.NUMBER_TYPE));
}

@Test
public void testCreateRightShift_colors() {
AstFactory astFactory = createTestAstFactoryWithColors();

Node zero = astFactory.createNumber(0);
Node one = astFactory.createNumber(1);

Node rightShift = astFactory.createRightShift(zero, one);

assertNode(rightShift).hasToken(Token.RSH);
assertThat(childList(rightShift)).containsExactly(zero, one);
assertNode(rightShift).hasColorThat().isEqualTo(StandardColors.NUMBER);
}

@Test
public void testCreateInc_prefix_jstypes() {
AstFactory astFactory = createTestAstFactory();
Expand Down
Loading

0 comments on commit a685a90

Please sign in to comment.