Skip to content

Commit

Permalink
Use java.util.function.Supplier instead of google.common.base.Supplier
Browse files Browse the repository at this point in the history
Now that Java defines Supplier, it is preferred to use that one directly.
This change fixes a few occurrences of the Guava version.

PiperOrigin-RevId: 558212024
  • Loading branch information
brad4d authored and copybara-github committed Aug 18, 2023
1 parent b817001 commit 6fd3617
Show file tree
Hide file tree
Showing 12 changed files with 126 additions and 173 deletions.
4 changes: 2 additions & 2 deletions src/com/google/javascript/jscomp/AbstractCompiler.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import com.google.common.base.Optional;
import com.google.common.base.Splitter;
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;
Expand Down Expand Up @@ -52,6 +51,7 @@
import java.util.EnumSet;
import java.util.List;
import java.util.Set;
import java.util.function.Supplier;
import org.jspecify.nullness.Nullable;

/**
Expand Down Expand Up @@ -81,7 +81,7 @@ void afterPass(String passName) {}
// Many of them are just accessors that should be passed to the
// CompilerPass's constructor.

abstract java.util.function.Supplier<Node> getTypedAstDeserializer(SourceFile file);
abstract Supplier<Node> getTypedAstDeserializer(SourceFile file);

/** Looks up an input (possibly an externs input) by input id. May return null. */
@Override
Expand Down
2 changes: 1 addition & 1 deletion src/com/google/javascript/jscomp/ExpressionDecomposer.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.base.Supplier;
import com.google.common.collect.ImmutableSet;
import com.google.javascript.jscomp.MakeDeclaredNamesUnique.ContextualRenamer;
import com.google.javascript.jscomp.colors.StandardColors;
Expand All @@ -35,6 +34,7 @@
import com.google.javascript.rhino.jstype.JSTypeNative;
import java.util.ArrayDeque;
import java.util.EnumSet;
import java.util.function.Supplier;
import org.jspecify.nullness.Nullable;

/**
Expand Down
76 changes: 30 additions & 46 deletions src/com/google/javascript/jscomp/FunctionArgumentInjector.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import static com.google.common.base.Preconditions.checkState;

import com.google.common.base.Predicate;
import com.google.common.base.Supplier;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.javascript.jscomp.NodeUtil.Visitor;
Expand All @@ -30,10 +29,11 @@
import java.util.LinkedHashSet;
import java.util.Map;
import java.util.Set;
import java.util.function.Supplier;

/**
* A nifty set of functions to deal with the issues of replacing function
* parameters with a set of call argument expressions.
* A nifty set of functions to deal with the issues of replacing function parameters with a set of
* call argument expressions.
*/
class FunctionArgumentInjector {

Expand Down Expand Up @@ -174,11 +174,8 @@ ImmutableMap<String, Node> getFunctionCallParameterMap(
return argMap.buildOrThrow();
}

/**
* Parameter names will be name unique when at a later time.
*/
private static String getUniqueAnonymousParameterName(
Supplier<String> safeNameIdSupplier) {
/** Parameter names will be name unique when at a later time. */
private static String getUniqueAnonymousParameterName(Supplier<String> safeNameIdSupplier) {
return "JSCompiler_inline_anon_param_" + safeNameIdSupplier.get();
}

Expand All @@ -202,16 +199,12 @@ Set<String> findModifiedParameters(Node fnNode) {
}

/**
* Check for uses of the named value that imply a pass-by-value
* parameter is expected. This is used to prevent cases like:
* Check for uses of the named value that imply a pass-by-value parameter is expected. This is
* used to prevent cases like:
*
* function (x) {
* x=2;
* return x;
* }
* <p>function (x) { x=2; return x; }
*
* We don't want "undefined" to be substituted for "x", and get
* undefined=2
* <p>We don't want "undefined" to be substituted for "x", and get undefined=2
*
* @param n The node in question.
* @param names The set of names to check.
Expand Down Expand Up @@ -242,12 +235,11 @@ private static Set<String> findModifiedParameters(
}

/**
* This is similar to NodeUtil.isLValue except that object properties and
* array member modification aren't important ("o" in "o.a = 2" is still "o"
* after assignment, where in as "o = x", "o" is now "x").
* This is similar to NodeUtil.isLValue except that object properties and array member
* modification aren't important ("o" in "o.a = 2" is still "o" after assignment, where in as "o =
* x", "o" is now "x").
*
* This also looks for the redefinition of a name.
* function (x) {var x;}
* <p>This also looks for the redefinition of a name. function (x) {var x;}
*
* @param n The NAME node in question.
*/
Expand Down Expand Up @@ -282,14 +274,15 @@ void maybeAddTempsForCallArguments(
int argCount = argMap.size();
// We limit the "trivial" bodies to those where there is a single expression or
// return, the expression is
boolean isTrivialBody = (!block.hasChildren()
|| (block.hasOneChild() && !bodyMayHaveConditionalCode(block.getLastChild())));
boolean hasMinimalParameters = NodeUtil.isUndefined(argMap.get(THIS_MARKER))
&& argCount <= 2; // this + one parameter
boolean isTrivialBody =
(!block.hasChildren()
|| (block.hasOneChild() && !bodyMayHaveConditionalCode(block.getLastChild())));
boolean hasMinimalParameters =
NodeUtil.isUndefined(argMap.get(THIS_MARKER)) && argCount <= 2; // this + one parameter

// Get the list of parameters that may need temporaries due to side-effects.
ImmutableSet<String> namesAfterSideEffects = findParametersReferencedAfterSideEffect(
argMap.keySet(), block);
ImmutableSet<String> namesAfterSideEffects =
findParametersReferencedAfterSideEffect(argMap.keySet(), block);

// Check for arguments that are evaluated more than once.
for (Map.Entry<String, Node> entry : argMap.entrySet()) {
Expand All @@ -304,7 +297,8 @@ void maybeAddTempsForCallArguments(
boolean argSideEffects = compiler.getAstAnalyzer().mayHaveSideEffects(cArg);
if (!argSideEffects && references == 0) {
safe = true;
} else if (isTrivialBody && hasMinimalParameters
} else if (isTrivialBody
&& hasMinimalParameters
&& references == 1
&& !(NodeUtil.canBeSideEffected(cArg) && namesAfterSideEffects.contains(argName))) {
// For functions with a trivial body, and where the parameter evaluation order
Expand All @@ -323,7 +317,7 @@ void maybeAddTempsForCallArguments(
// x( [] );
//
// The parameter in the call to foo should not become "[]".
safe = false;
safe = false;
} else if (argSideEffects) {
// Even if there are no references, we still need to evaluate the
// expression if it has side-effects.
Expand Down Expand Up @@ -409,12 +403,9 @@ private ImmutableSet<String> findParametersReferencedAfterSideEffect(
Set<String> locals = new LinkedHashSet<>(parameters);
gatherLocalNames(root, locals);

ReferencedAfterSideEffect collector = new ReferencedAfterSideEffect(
parameters, ImmutableSet.copyOf(locals));
NodeUtil.visitPostOrder(
root,
collector,
collector);
ReferencedAfterSideEffect collector =
new ReferencedAfterSideEffect(parameters, ImmutableSet.copyOf(locals));
NodeUtil.visitPostOrder(root, collector, collector);
return collector.getResults();
}

Expand Down Expand Up @@ -513,9 +504,7 @@ private boolean hasNonLocalSideEffect(Node n) {
Token type = n.getToken();
// Note: Only care about changes to non-local names, specifically
// ignore VAR declaration assignments.
if (NodeUtil.isAssignmentOp(n)
|| type == Token.INC
|| type == Token.DEC) {
if (NodeUtil.isAssignmentOp(n) || type == Token.INC || type == Token.DEC) {
Node lhs = n.getFirstChild();
// Ignore changes to local names.
if (!isLocalName(lhs)) {
Expand Down Expand Up @@ -544,9 +533,7 @@ private boolean isLocalName(Node node) {
}
}

/**
* Gather any names declared in the local scope.
*/
/** Gather any names declared in the local scope. */
private static void gatherLocalNames(Node n, Set<String> names) {
if (n.isFunction()) {
if (NodeUtil.isFunctionDeclaration(n)) {
Expand All @@ -572,15 +559,13 @@ private static void gatherLocalNames(Node n, Set<String> names) {
}
}

/**
* Get a set of function parameter names.
*/
/** Get a set of function parameter names. */
private static ImmutableSet<String> getFunctionParameterSet(Node fnNode) {
ImmutableSet.Builder<String> builder = ImmutableSet.builder();
for (Node n = NodeUtil.getFunctionParameters(fnNode).getFirstChild();
n != null;
n = n.getNext()) {
if (n.isRest()){
if (n.isRest()) {
builder.add(REST_MARKER);
} else if (n.isDefaultValue() || n.isObjectPattern() || n.isArrayPattern()) {
throw new IllegalStateException("Not supported: " + n);
Expand All @@ -590,5 +575,4 @@ private static ImmutableSet<String> getFunctionParameterSet(Node fnNode) {
}
return builder.build();
}

}
11 changes: 8 additions & 3 deletions src/com/google/javascript/jscomp/FunctionInjector.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import com.google.common.base.Preconditions;
import com.google.common.base.Predicate;
import com.google.common.base.Predicates;
import com.google.common.base.Supplier;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
Expand All @@ -34,6 +33,7 @@
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.Set;
import java.util.function.Supplier;
import org.jspecify.nullness.Nullable;

/**
Expand All @@ -44,6 +44,7 @@ class FunctionInjector {

/** Sentinel value indicating that the key contains no functions. */
private static final Node NO_FUNCTIONS = new Node(Token.FUNCTION);

/** Sentinel value indicating that the key contains multiple distinct functions. */
private static final Node MULTIPLE_FUNCTIONS = new Node(Token.FUNCTION);

Expand Down Expand Up @@ -960,7 +961,9 @@ boolean inliningLowersCost(
isRemovable);
}

/** @return Whether inlining will lower cost. */
/**
* @return Whether inlining will lower cost.
*/
private static boolean doesLowerCost(
Node fnNode,
int callCost,
Expand Down Expand Up @@ -1014,7 +1017,9 @@ private static int estimateCallCost(Node fnNode, boolean referencesThis) {
return callCost;
}

/** @return The difference between the function definition cost and inline cost. */
/**
* @return The difference between the function definition cost and inline cost.
*/
private static int inlineCostDelta(Node fnNode, Set<String> namesToAlias, InliningMode mode) {
// The part of the function that is never inlined:
// "function xx(xx,xx){}" (15 + (param count * 3) -1;
Expand Down
Loading

0 comments on commit 6fd3617

Please sign in to comment.