Introduce first class function signatures; make the parser use them.
This is the first meaty cl in a series to refactor the Skylark function call protocol.
1- We introduce a first-class notion of FunctionSignature, that supports positional and named-only arguments, mandatory and optional, default values, type-checking, *stararg and **kwarg;
2- To keep things clean, we distinguish two different kinds of Argument's: Argument.Passed that appears in function calls, and Parameter, that appears in function definitions.
3- We refactor the Parser so it uses this infrastructure, and make minimal changes to MixedModeFunction so that it works with it (but don't actually implement *starparam and **kwparam yet).
4- As we modify FuncallExpression, we ensure that the args and kwargs arguments it passes to the underlying function are immutable, as a prerequisite to upcoming implementation of *starparam and **kwparam as being provided directly from a skylark list or dict.
Further changes under review will take advantage of this FunctionSignature to redo all our function call protocol, to be used uniformly for both UserDefinedFunction's and builtin function. The result will be a simpler inheritance model, with better type-checking, builtin functions that are both simpler and better documented, and many redundant competing functionality-limited codepaths being merged and replaced by something better.
NB: The changes to MixedModeFunction, SkylarkFunction and MethodLibrary are temporary hacks to be done away with in an upcoming CL. The rest is the actual changes.
--
MOS_MIGRATED_REVID=86704072
diff --git a/src/main/java/com/google/devtools/build/lib/packages/MethodLibrary.java b/src/main/java/com/google/devtools/build/lib/packages/MethodLibrary.java
index 05914af..fe0ae3c 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/MethodLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/MethodLibrary.java
@@ -46,6 +46,7 @@
import com.google.devtools.build.lib.syntax.SkylarkType;
import com.google.devtools.build.lib.syntax.SkylarkType.SkylarkFunctionType;
+import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.Iterator;
@@ -881,12 +882,18 @@
public Object call(List<Object> args, Map<String, Object> kwargs, FuncallExpression ast,
Environment env) throws EvalException, InterruptedException {
String sep = " ";
+ int count = 0;
if (kwargs.containsKey("sep")) {
- sep = cast(kwargs.remove("sep"), String.class, "sep", ast.getLocation());
+ sep = cast(kwargs.get("sep"), String.class, "sep", ast.getLocation());
+ count = 1;
}
- if (kwargs.size() > 0) {
+ if (kwargs.size() > count) {
+ kwargs = new HashMap<String, Object>(kwargs);
+ kwargs.remove("sep");
+ List<String> bad = new ArrayList<String>(kwargs.keySet());
+ java.util.Collections.sort(bad);
throw new EvalException(ast.getLocation(),
- "unexpected keywords: '" + kwargs.keySet() + "'");
+ "unexpected keywords: '" + bad + "'");
}
String msg = Joiner.on(sep).join(Iterables.transform(args,
new com.google.common.base.Function<Object, String>() {
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
index f8f442f..b7da520 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
@@ -33,7 +33,6 @@
import com.google.devtools.build.lib.syntax.Environment;
import com.google.devtools.build.lib.syntax.FuncallExpression;
import com.google.devtools.build.lib.syntax.GlobList;
-import com.google.devtools.build.lib.syntax.Ident;
import com.google.devtools.build.lib.syntax.Label;
import com.google.devtools.build.lib.syntax.Label.SyntaxException;
import com.google.devtools.build.lib.syntax.SkylarkEnvironment;
@@ -1069,7 +1068,7 @@
}
/**
- * Helper function for {@link RuleFactory#createRule}.
+ * Helper function for {@link RuleFactory#createAndAddRule}.
*/
Rule createRuleWithLabel(Package.AbstractBuilder<?, ?> pkgBuilder, Label ruleLabel,
Map<String, Object> attributeValues, EventHandler eventHandler, FuncallExpression ast,
@@ -1180,10 +1179,9 @@
// Save the location of each non-default attribute definition:
if (ast != null) {
- for (Argument arg : ast.getArguments()) {
- Ident keyword = arg.getName();
- if (keyword != null) {
- String name = keyword.getName();
+ for (Argument.Passed arg : ast.getArguments()) {
+ if (arg.isKeyword()) {
+ String name = arg.getName();
Integer attrIndex = getAttributeIndex(name);
if (attrIndex != null) {
rule.setAttributeLocation(attrIndex, arg.getValue().getLocation());
@@ -1363,12 +1361,12 @@
/**
* Returns the default value for the specified rule attribute.
*
- * For most rule attributes, the default value is either explicitly specified
+ * <p>For most rule attributes, the default value is either explicitly specified
* in the attribute, or implicitly based on the type of the attribute, except
* for some special cases (e.g. "licenses", "distribs") where it comes from
* some other source, such as state in the package.
*
- * Precondition: {@code !attr.hasComputedDefault()}. (Computed defaults are
+ * <p>Precondition: {@code !attr.hasComputedDefault()}. (Computed defaults are
* evaluated in second pass.)
*/
private static Object getAttributeNoncomputedDefaultValue(Attribute attr,
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Argument.java b/src/main/java/com/google/devtools/build/lib/syntax/Argument.java
index 0706dee..4c16adb 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Argument.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Argument.java
@@ -13,106 +13,167 @@
// limitations under the License.
package com.google.devtools.build.lib.syntax;
+import com.google.common.base.Preconditions;
+
+import java.util.List;
+
+import javax.annotation.Nullable;
+
/**
- * Syntax node for a function argument. This can be a key/value pair such as
- * appears as a keyword argument to a function call or just an expression that
- * is used as a positional argument. It also can be used for function definitions
- * to identify the name (and optionally the default value) of the argument.
+ * Syntax node for a function argument.
+ *
+ * <p>Argument is a base class for arguments passed in a call (@see Argument.Passed)
+ * or defined as part of a function definition (@see Parameter).
+ * It is notably used by some {@link Parser} and printer functions.
*/
-public final class Argument extends ASTNode {
+public abstract class Argument extends ASTNode {
- private final Ident name;
-
- private final Expression value;
-
- private final boolean kwargs;
-
- /**
- * Create a new argument.
- * At call site: name is optional, value is mandatory. kwargs is true for ** arguments.
- * At definition site: name is mandatory, (default) value is optional.
- */
- public Argument(Ident name, Expression value, boolean kwargs) {
- this.name = name;
- this.value = value;
- this.kwargs = kwargs;
+ public boolean isStar() {
+ return false;
}
- public Argument(Ident name, Expression value) {
- this.name = name;
- this.value = value;
- this.kwargs = false;
+ public boolean isStarStar() {
+ return false;
}
/**
- * Creates an Argument with null as name. It can be used as positional arguments
- * of function calls.
+ * Argument.Passed is the class of arguments passed in a function call
+ * (as opposed to being used in a definition -- @see Parameter for that).
+ * Argument.Passed is usually what we mean when informally say "argument".
+ *
+ * <p>An Argument.Passed can be Positional, Keyword, Star, or StarStar.
*/
- public Argument(Expression value) {
- this(null, value);
+ public abstract static class Passed extends Argument {
+ /** the value to be passed by this argument */
+ protected final Expression value;
+
+ private Passed(Expression value) {
+ this.value = Preconditions.checkNotNull(value);
+ }
+
+ public boolean isPositional() {
+ return false;
+ }
+ public boolean isKeyword() {
+ return false;
+ }
+ @Nullable public String getName() { // only for keyword arguments
+ return null;
+ }
+ public Expression getValue() {
+ return value;
+ }
+ }
+
+ /** positional argument: Expression */
+ public static class Positional extends Passed {
+
+ public Positional(Expression value) {
+ super(value);
+ }
+
+ @Override public boolean isPositional() {
+ return true;
+ }
+ @Override
+ public String toString() {
+ return String.valueOf(value);
+ }
+ }
+
+ /** keyword argument: K = Expression */
+ public static class Keyword extends Passed {
+
+ final String name;
+
+ public Keyword(String name, Expression value) {
+ super(value);
+ this.name = name;
+ }
+
+ @Override public String getName() {
+ return name;
+ }
+ @Override public boolean isKeyword() {
+ return true;
+ }
+ @Override
+ public String toString() {
+ return name + " = " + String.valueOf(value);
+ }
+ }
+
+ /** positional rest (starred) argument: *Expression */
+ public static class Star extends Passed {
+
+ public Star(Expression value) {
+ super(value);
+ }
+
+ @Override public boolean isStar() {
+ return true;
+ }
+ @Override
+ public String toString() {
+ return "*" + String.valueOf(value);
+ }
+ }
+
+ /** keyword rest (star_starred) parameter: **Expression */
+ public static class StarStar extends Passed {
+
+ public StarStar(Expression value) {
+ super(value);
+ }
+
+ @Override public boolean isStarStar() {
+ return true;
+ }
+ @Override
+ public String toString() {
+ return "**" + String.valueOf(value);
+ }
+ }
+
+ /** Some arguments failed to satisfy python call convention strictures */
+ protected static class ArgumentException extends Exception {
+ /** construct an ArgumentException from a message only */
+ public ArgumentException(String message) {
+ super(message);
+ }
}
/**
- * Creates an Argument with null as value. It can be used as a mandatory keyword argument
- * of a function definition.
+ * Validate that the list of Argument's, whether gathered by the Parser or from annotations,
+ * satisfies the requirements of the Python calling conventions: all Positional's first,
+ * at most one Star, at most one StarStar, at the end only.
*/
- public Argument(Ident name) {
- this(name, null);
- }
-
- /**
- * Returns the name of this keyword argument or null if this argument is
- * positional.
- */
- public Ident getName() {
- return name;
- }
-
- /**
- * Returns the String value of the Ident of this argument. Shortcut for arg.getName().getName().
- */
- public String getArgName() {
- return name.getName();
- }
-
- /**
- * Returns the syntax of this argument expression.
- */
- public Expression getValue() {
- return value;
- }
-
- /**
- * Returns true if this argument is positional.
- */
- public boolean isPositional() {
- return name == null && !kwargs;
- }
-
- /**
- * Returns true if this argument is a keyword argument.
- */
- public boolean isNamed() {
- return name != null;
- }
-
- /**
- * Returns true if this argument is a **kwargs argument.
- */
- public boolean isKwargs() {
- return kwargs;
- }
-
- /**
- * Returns true if this argument has value.
- */
- public boolean hasValue() {
- return value != null;
- }
-
- @Override
- public String toString() {
- return isNamed() ? name + "=" + value : String.valueOf(value);
+ public static void validateFuncallArguments(List<Passed> arguments)
+ throws ArgumentException {
+ boolean hasNamed = false;
+ boolean hasStar = false;
+ boolean hasKwArg = false;
+ for (Passed arg : arguments) {
+ if (hasKwArg) {
+ throw new ArgumentException("argument after **kwargs");
+ }
+ if (arg.isPositional()) {
+ if (hasNamed) {
+ throw new ArgumentException("non-keyword arg after keyword arg");
+ } else if (arg.isStar()) {
+ throw new ArgumentException("only named arguments may follow *expression");
+ }
+ } else if (arg.isKeyword()) {
+ hasNamed = true;
+ } else if (arg.isStar()) {
+ if (hasStar) {
+ throw new ArgumentException("more than one *stararg");
+ }
+ hasStar = true;
+ } else {
+ hasKwArg = true;
+ }
+ }
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
index 38f4a7d..3ef5973 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
@@ -14,7 +14,7 @@
package com.google.devtools.build.lib.syntax;
-import com.google.common.base.Preconditions;
+import com.google.common.base.Joiner;
import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
@@ -31,7 +31,6 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
-import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ExecutionException;
@@ -141,7 +140,8 @@
}
if (keepLooking) {
if (classObj.getSuperclass() != null) {
- SkylarkCallable annotation = getAnnotationFromParentClass(classObj.getSuperclass(), method);
+ SkylarkCallable annotation =
+ getAnnotationFromParentClass(classObj.getSuperclass(), method);
if (annotation != null) {
return annotation;
}
@@ -170,7 +170,7 @@
private final Ident func;
- private final List<Argument> args;
+ private final List<Argument.Passed> args;
private final int numPositionalArgs;
@@ -182,13 +182,10 @@
* evaluated, so functions and variables share a common namespace.
*/
public FuncallExpression(Expression obj, Ident func,
- List<Argument> args) {
- for (Argument arg : args) {
- Preconditions.checkArgument(arg.hasValue());
- }
+ List<Argument.Passed> args) {
this.obj = obj;
this.func = func;
- this.args = args;
+ this.args = args; // we assume the parser validated it with Argument#validateFuncallArguments()
this.numPositionalArgs = countPositionalArguments();
}
@@ -199,7 +196,7 @@
* arbitrary expressions. In any case, the "func" expression is always
* evaluated, so functions and variables share a common namespace.
*/
- public FuncallExpression(Ident func, List<Argument> args) {
+ public FuncallExpression(Ident func, List<Argument.Passed> args) {
this(null, func, args);
}
@@ -208,7 +205,7 @@
*/
private int countPositionalArguments() {
int num = 0;
- for (Argument arg : args) {
+ for (Argument.Passed arg : args) {
if (arg.isPositional()) {
num++;
}
@@ -236,7 +233,7 @@
* positional and the remaining ones are keyword args, where n =
* getNumPositionalArguments().
*/
- public List<Argument> getArguments() {
+ public List<Argument.Passed> getArguments() {
return Collections.unmodifiableList(args);
}
@@ -393,21 +390,20 @@
}
/**
- * Add one argument to the keyword map, raising an exception when names conflict.
+ * Add one argument to the keyword map, registering a duplicate in case of conflict.
*/
- private void addKeywordArg(Map<String, Object> kwargs, String name, Object value)
- throws EvalException {
+ private void addKeywordArg(Map<String, Object> kwargs, String name,
+ Object value, ImmutableList.Builder<String> duplicates) {
if (kwargs.put(name, value) != null) {
- throw new EvalException(getLocation(),
- "duplicate keyword '" + name + "' in call to '" + func + "'");
+ duplicates.add(name);
}
}
/**
- * Add multiple arguments to the keyword map (**kwargs).
+ * Add multiple arguments to the keyword map (**kwargs), registering duplicates
*/
- private void addKeywordArgs(Map<String, Object> kwargs, Object items)
- throws EvalException {
+ private void addKeywordArgs(Map<String, Object> kwargs,
+ Object items, ImmutableList.Builder<String> duplicates) throws EvalException {
if (!(items instanceof Map<?, ?>)) {
throw new EvalException(getLocation(),
"Argument after ** must be a dictionary, not " + EvalUtils.getDataTypeName(items));
@@ -417,15 +413,21 @@
throw new EvalException(getLocation(),
"Keywords must be strings, not " + EvalUtils.getDataTypeName(entry.getKey()));
}
- addKeywordArg(kwargs, (String) entry.getKey(), entry.getValue());
+ addKeywordArg(kwargs, (String) entry.getKey(), entry.getValue(), duplicates);
}
}
- private void evalArguments(List<Object> posargs, Map<String, Object> kwargs,
+ @SuppressWarnings("unchecked")
+ private void evalArguments(ImmutableList.Builder<Object> posargs, Map<String, Object> kwargs,
Environment env, Function function)
- throws EvalException, InterruptedException {
+ throws EvalException, InterruptedException {
ArgConversion conversion = getArgConversion(function);
- for (Argument arg : args) {
+ ImmutableList.Builder<String> duplicates = new ImmutableList.Builder<String>();
+ // Iterate over the arguments. We assume all positional arguments come before any keyword
+ // or star arguments, because the argument list was already validated by
+ // Argument#validateFuncallArguments, as called by the Parser,
+ // which should be the only place that build FuncallExpression-s.
+ for (Argument.Passed arg : args) {
Object value = arg.getValue().eval(env);
if (conversion == ArgConversion.FROM_SKYLARK) {
value = SkylarkType.convertFromSkylark(value);
@@ -434,26 +436,25 @@
value = SkylarkType.convertToSkylark(value, getLocation());
// We call into Skylark so we need to be sure that the caller uses the appropriate types.
SkylarkType.checkTypeAllowedInSkylark(value, getLocation());
- }
+ } // else NO_CONVERSION
if (arg.isPositional()) {
posargs.add(value);
- } else if (arg.isKwargs()) { // expand the kwargs
- addKeywordArgs(kwargs, value);
+ } else if (arg.isStar()) { // expand the starArg
+ if (value instanceof Iterable) {
+ posargs.addAll((Iterable<Object>) value);
+ }
+ } else if (arg.isStarStar()) { // expand the kwargs
+ addKeywordArgs(kwargs, value, duplicates);
} else {
- addKeywordArg(kwargs, arg.getArgName(), value);
+ addKeywordArg(kwargs, arg.getName(), value, duplicates);
}
}
- if (function instanceof UserDefinedFunction) {
- // Adding the default values for a UserDefinedFunction if needed.
- UserDefinedFunction func = (UserDefinedFunction) function;
- if (args.size() < func.getArgs().size()) {
- for (Map.Entry<String, Object> entry : func.getDefaultValues().entrySet()) {
- String key = entry.getKey();
- if (func.getArgIndex(key) >= numPositionalArgs && !kwargs.containsKey(key)) {
- kwargs.put(key, entry.getValue());
- }
- }
- }
+ List<String> dups = duplicates.build();
+ if (!dups.isEmpty()) {
+ throw new EvalException(getLocation(),
+ "duplicate keyword" + (dups.size() > 1 ? "s" : "") + " '"
+ + Joiner.on("', '").join(dups)
+ + "' in call to " + func);
}
}
@@ -464,10 +465,12 @@
@Override
Object eval(Environment env) throws EvalException, InterruptedException {
- List<Object> posargs = new ArrayList<>();
- Map<String, Object> kwargs = new LinkedHashMap<>();
+ ImmutableList.Builder<Object> posargs = new ImmutableList.Builder<Object>();
+ // We copy this into an ImmutableMap in the end, but we can't use an ImmutableMap.Builder, or
+ // we'd still have to have a HashMap on the side for the sake of properly handling duplicates.
+ Map<String, Object> kwargs = new HashMap<>();
- if (obj != null) {
+ if (obj != null) { // obj.func(...)
Object objValue = obj.eval(env);
// Strings, lists and dictionaries (maps) have functions that we want to use in MethodLibrary.
// For other classes, we can call the Java methods.
@@ -475,43 +478,47 @@
env.getFunction(EvalUtils.getSkylarkType(objValue.getClass()), func.getName());
if (function != null) {
if (!isNamespace(objValue.getClass())) {
+ // Add self as an implicit parameter in front.
posargs.add(objValue);
}
evalArguments(posargs, kwargs, env, function);
- return EvalUtils.checkNotNull(this, function.call(posargs, kwargs, this, env));
+ return EvalUtils.checkNotNull(this,
+ function.call(posargs.build(), ImmutableMap.<String, Object>copyOf(kwargs), this, env));
} else if (env.isSkylarkEnabled()) {
-
- // When calling a Java method, the name is not in the Environment, so
- // evaluating 'func' would fail. For arguments we don't need to consider the default
- // arguments since the Java function doesn't have any.
-
+ // Only allow native Java calls when using Skylark
+ // When calling a Java method, the name is not in the Environment,
+ // so evaluating 'func' would fail.
evalArguments(posargs, kwargs, env, null);
if (!kwargs.isEmpty()) {
throw new EvalException(func.getLocation(),
- "Keyword arguments are not allowed when calling a java method");
+ String.format("Keyword arguments are not allowed when calling a java method"
+ + "\nwhile calling method '%s' on object %s of type %s",
+ func.getName(), objValue, EvalUtils.getDataTypeName(objValue)));
}
if (objValue instanceof Class<?>) {
// Static Java method call
- return invokeJavaMethod(null, (Class<?>) objValue, func.getName(), posargs);
+ return invokeJavaMethod(null, (Class<?>) objValue, func.getName(), posargs.build());
} else {
- return invokeJavaMethod(objValue, objValue.getClass(), func.getName(), posargs);
+ return invokeJavaMethod(objValue, objValue.getClass(), func.getName(), posargs.build());
}
} else {
throw new EvalException(getLocation(), String.format(
"function '%s' is not defined on '%s'", func.getName(),
EvalUtils.getDataTypeName(objValue)));
}
+ } else { // func(...)
+ Object funcValue = func.eval(env);
+ if ((funcValue instanceof Function)) {
+ Function function = (Function) funcValue;
+ evalArguments(posargs, kwargs, env, function);
+ return EvalUtils.checkNotNull(this,
+ function.call(posargs.build(), ImmutableMap.<String, Object>copyOf(kwargs), this, env));
+ } else {
+ throw new EvalException(getLocation(),
+ "'" + EvalUtils.getDataTypeName(funcValue)
+ + "' object is not callable");
+ }
}
-
- Object funcValue = func.eval(env);
- if (!(funcValue instanceof Function)) {
- throw new EvalException(getLocation(),
- "'" + EvalUtils.getDataTypeName(funcValue)
- + "' object is not callable");
- }
- Function function = (Function) funcValue;
- evalArguments(posargs, kwargs, env, function);
- return EvalUtils.checkNotNull(this, function.call(posargs, kwargs, this, env));
}
private ArgConversion getArgConversion(Function function) {
@@ -533,7 +540,7 @@
@Override
SkylarkType validate(ValidationEnvironment env) throws EvalException {
- for (Argument arg : args) {
+ for (Argument.Passed arg : args) {
arg.getValue().validate(env);
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FunctionDefStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/FunctionDefStatement.java
index 29ed579..f06829a 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FunctionDefStatement.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FunctionDefStatement.java
@@ -15,10 +15,12 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.syntax.SkylarkType.SkylarkFunctionType;
+import java.util.ArrayList;
import java.util.Collection;
+import java.util.List;
+import java.util.Map;
/**
* Syntax node for a function definition.
@@ -26,29 +28,39 @@
public class FunctionDefStatement extends Statement {
private final Ident ident;
- private final ImmutableList<Argument> args;
+ private final FunctionSignature.WithValues<Expression, Expression> args;
private final ImmutableList<Statement> statements;
- public FunctionDefStatement(Ident ident, Collection<Argument> args,
+ public FunctionDefStatement(Ident ident,
+ FunctionSignature.WithValues<Expression, Expression> args,
Collection<Statement> statements) {
- for (Argument arg : args) {
- Preconditions.checkArgument(arg.isNamed());
- }
+
+ // TODO(bazel-team): lift the following limitation from {@link MixedModeFunction}
+ FunctionSignature.Shape shape = args.getSignature().getShape();
+ Preconditions.checkArgument(!shape.hasKwArg() && !shape.hasStarArg()
+ && shape.getNamedOnly() == 0, "no star, star-star or named-only parameters (for now)");
+
this.ident = ident;
- this.args = ImmutableList.copyOf(args);
+ this.args = args;
this.statements = ImmutableList.copyOf(statements);
}
@Override
void exec(Environment env) throws EvalException, InterruptedException {
- ImmutableMap.Builder<String, Object> defaultValues = ImmutableMap.builder();
- for (Argument arg : args) {
- if (arg.hasValue()) {
- defaultValues.put(arg.getArgName(), arg.getValue().eval(env));
+ List<Expression> defaultExpressions = args.getDefaultValues();
+ ArrayList<Object> defaultValues = null;
+ ArrayList<SkylarkType> types = null;
+
+ if (defaultExpressions != null) {
+ defaultValues = new ArrayList<Object>(defaultExpressions.size());
+ for (Expression expr : defaultExpressions) {
+ defaultValues.add(expr.eval(env));
}
}
env.update(ident.getName(), new UserDefinedFunction(
- ident, args, defaultValues.build(), statements, (SkylarkEnvironment) env));
+ ident, FunctionSignature.WithValues.<Object, SkylarkType>create(
+ args.getSignature(), defaultValues, types),
+ statements, (SkylarkEnvironment) env));
}
@Override
@@ -64,7 +76,7 @@
return statements;
}
- public ImmutableList<Argument> getArgs() {
+ public FunctionSignature.WithValues<Expression, Expression> getArgs() {
return args;
}
@@ -74,18 +86,45 @@
}
@Override
- void validate(ValidationEnvironment env) throws EvalException {
+ void validate(final ValidationEnvironment env) throws EvalException {
SkylarkFunctionType type = SkylarkFunctionType.of(ident.getName());
ValidationEnvironment localEnv = new ValidationEnvironment(env, type);
- for (Argument i : args) {
+ FunctionSignature sig = args.getSignature();
+ FunctionSignature.Shape shape = sig.getShape();
+ ImmutableList<String> names = sig.getNames();
+ List<Expression> defaultExpressions = args.getDefaultValues();
+
+ int positionals = shape.getPositionals();
+ int mandatoryPositionals = shape.getMandatoryPositionals();
+ int namedOnly = shape.getNamedOnly();
+ int mandatoryNamedOnly = shape.getMandatoryNamedOnly();
+ boolean starArg = shape.hasStarArg();
+ boolean hasStar = starArg || (namedOnly > 0);
+ boolean kwArg = shape.hasKwArg();
+ int named = positionals + namedOnly;
+ int args = named + (starArg ? 1 : 0) + (kwArg ? 1 : 0);
+ int startOptionals = mandatoryPositionals;
+ int endOptionals = named - mandatoryNamedOnly;
+ int iStarArg = named;
+ int iKwArg = args - 1;
+
+ int j = 0; // index for the defaultExpressions
+ for (int i = 0; i < args; i++) {
+ String name = names.get(i);
SkylarkType argType = SkylarkType.UNKNOWN;
- if (i.hasValue()) {
- argType = i.getValue().validate(env);
- if (argType.equals(SkylarkType.NONE)) {
- argType = SkylarkType.UNKNOWN;
+ if (hasStar && i == iStarArg) {
+ argType = SkylarkType.of(SkylarkList.class, Object.class);
+ } else if (kwArg && i == iKwArg) {
+ argType = SkylarkType.of(Map.class, Object.class);
+ } else {
+ if (startOptionals <= i && i < endOptionals) {
+ argType = defaultExpressions.get(j++).validate(env);
+ if (argType.equals(SkylarkType.NONE)) {
+ argType = SkylarkType.UNKNOWN;
+ }
}
}
- localEnv.update(i.getArgName(), argType, getLocation());
+ localEnv.update(name, argType, getLocation());
}
for (Statement stmts : statements) {
stmts.validate(localEnv);
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FunctionSignature.java b/src/main/java/com/google/devtools/build/lib/syntax/FunctionSignature.java
new file mode 100644
index 0000000..c317c7d
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FunctionSignature.java
@@ -0,0 +1,560 @@
+// Copyright 2014 Google Inc. All rights reserved.
+//
+// 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.devtools.build.lib.syntax;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
+
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+import javax.annotation.Nullable;
+
+/**
+ * Function Signatures for BUILD language (same as Python)
+ *
+ * <p>Skylark's function signatures are just like Python3's.
+ * A function may have 6 kinds of arguments:
+ * positional mandatory, positional optional, positional rest (aka *star argument),
+ * key-only mandatory, key-only optional, key rest (aka **star_star argument).
+ * A caller may specify all arguments but the *star and **star_star arguments by name,
+ * and thus all mandatory and optional arguments are named arguments.
+ *
+ * <p>To enable various optimizations in the argument processing routine,
+ * we sort arguments according the following constraints, enabling corresponding optimizations:
+ * <ol>
+ * <li>the positional mandatories come just before the positional optionals,
+ * so they can be filled in one go.
+ * <li>the optionals are grouped together, so we can iterate over them in one go.
+ * <li>positionals come first, so it's easy to prepend extra positional arguments such as "self"
+ * to an argument list, and we optimize for the common case of no key-only mandatory parameters.
+ * key-only parameters are thus grouped together.
+ * positional mandatory and key-only mandatory parameters are separate,
+ * but there no loop over a contiguous chunk of them, anyway.
+ * <li>the named are all grouped together, with star and star_star rest arguments coming last.
+ * </ol>
+ *
+ * <p>Parameters are thus sorted in the following obvious order:
+ * positional mandatory arguments (if any), positional optional arguments (if any),
+ * key-only optional arguments (if any), key-only mandatory arguments (if any),
+ * then star argument (if any), then star_star argument (if any).
+ */
+public abstract class FunctionSignature implements Serializable {
+
+ /**
+ * The shape of a FunctionSignature, without names
+ */
+ public abstract static class Shape implements Serializable {
+
+ /** Create a function signature */
+ public static Shape create(
+ int mandatoryPositionals,
+ int optionalPositionals,
+ int mandatoryNamedOnly,
+ int optionalNamedOnly,
+ boolean starArg,
+ boolean kwArg) {
+ Preconditions.checkArgument(
+ 0 <= mandatoryPositionals && 0 <= optionalPositionals
+ && 0 <= mandatoryNamedOnly && 0 <= optionalNamedOnly);
+ return new AutoValueFunctionSignatureShape(
+ mandatoryPositionals, optionalPositionals,
+ mandatoryNamedOnly, optionalNamedOnly, starArg, kwArg);
+ }
+
+ // These abstract getters specify the actual argument count fields to be defined by AutoValue.
+ /** number of mandatory positional arguments */
+ public abstract int getMandatoryPositionals();
+
+ /** number of optional positional arguments */
+ public abstract int getOptionalPositionals();
+
+ /** number of mandatory named-only arguments. */
+ public abstract int getMandatoryNamedOnly();
+
+ /** number of optional named-only arguments */
+ public abstract int getOptionalNamedOnly();
+
+ /** indicator for presence of a star argument for extra positional arguments */
+ public abstract boolean hasStarArg();
+
+ /** indicator for presence of a star-star argument for extra keyword arguments */
+ public abstract boolean hasKwArg();
+
+
+ // The are computed argument counts
+ /** number of optional positional arguments. */
+ public int getPositionals() {
+ return getMandatoryPositionals() + getOptionalPositionals();
+ }
+
+ /** number of optional named-only arguments. */
+ public int getNamedOnly() {
+ return getMandatoryNamedOnly() + getOptionalNamedOnly();
+ }
+
+ /** number of optional arguments. */
+ public int getOptionals() {
+ return getOptionalPositionals() + getOptionalNamedOnly();
+ }
+
+ /** total number of arguments */
+ public int getArguments() {
+ return getPositionals() + getNamedOnly() + (hasStarArg() ? 1 : 0) + (hasKwArg() ? 1 : 0);
+ }
+ }
+
+ /**
+ * Signatures proper.
+ *
+ * <p>A signature is a Shape and an ImmutableList of argument variable names
+ * NB: we assume these lists are short, so we may do linear scans.
+ */
+ public static FunctionSignature create(Shape shape, ImmutableList<String> names) {
+ Preconditions.checkArgument(names.size() == shape.getArguments());
+ return new AutoValueFunctionSignature(shape, names);
+ }
+
+ // Field definition (details filled in by AutoValue)
+ /** The shape */
+ public abstract Shape getShape();
+
+ /** The names */
+ public abstract ImmutableList<String> getNames();
+
+ /** append a representation of this signature to a string buffer. */
+ public StringBuffer toStringBuffer(StringBuffer sb) {
+ return WithValues.<Object, SkylarkType>create(this).toStringBuffer(sb);
+ }
+
+ @Override
+ public String toString() {
+ StringBuffer sb = new StringBuffer();
+ toStringBuffer(sb);
+ return sb.toString();
+ }
+
+
+ /**
+ * FunctionSignature.WithValues: also specifies a List of default values and types.
+ *
+ * <p>The lists can be null, which is an optimized path for specifying all null values.
+ *
+ * <p>Note that if some values can be null (for BuiltinFunction, not for UserDefinedFunction),
+ * you should use an ArrayList; otherwise, we recommend an ImmutableList.
+ *
+ * <p>V is the class of defaultValues and T is the class of types.
+ * When parsing a function definition at compile-time, they are <Expression, Expression>;
+ * when processing a @SkylarkBuiltin annotation at build-time, <Object, SkylarkType>.
+ */
+ public abstract static class WithValues<V, T> implements Serializable {
+
+ // The fields
+ /** The underlying signature with parameter shape and names */
+ public abstract FunctionSignature getSignature();
+
+ /** The default values (if any) as a List of one per optional parameter.
+ * We might have preferred ImmutableList, but we care about
+ * supporting null's for some BuiltinFunction's, and we don't spit on speed.
+ */
+ @Nullable public abstract List<V> getDefaultValues();
+
+ /** The parameter types (if specified) as a List of one per parameter, including * and **.
+ * We might have preferred ImmutableList, but we care about supporting null's
+ * so we can take shortcut for untyped values.
+ */
+ @Nullable public abstract List<T> getTypes();
+
+ /**
+ * Create a signature with (default and type) values.
+ * If you supply mutable List's, we trust that you won't modify them afterwards.
+ */
+ public static <V, T> WithValues<V, T> create(FunctionSignature signature,
+ @Nullable List<V> defaultValues, @Nullable List<T> types) {
+ Shape shape = signature.getShape();
+ Preconditions.checkArgument(defaultValues == null
+ || defaultValues.size() == shape.getOptionals());
+ Preconditions.checkArgument(types == null
+ || types.size() == shape.getArguments());
+ return new AutoValueFunctionSignatureWithValues<V, T>(signature, defaultValues, types);
+ }
+
+ public static <V, T> WithValues<V, T> create(FunctionSignature signature,
+ @Nullable List<V> defaultValues) {
+ return create(signature, defaultValues, null);
+ }
+
+ public static <V, T> WithValues<V, T> create(FunctionSignature signature,
+ @Nullable V[] defaultValues) {
+ return create(signature, Arrays.asList(defaultValues), null);
+ }
+
+ public static <V, T> WithValues<V, T> create(FunctionSignature signature) {
+ return create(signature, null, null);
+ }
+
+ /**
+ * Parse a list of Parameter into a FunctionSignature.
+ *
+ * <p>To be used both by the Parser and by the SkylarkBuiltin annotation processor.
+ */
+ public static <V, T> WithValues<V, T> of(Iterable<Parameter<V, T>> parameters)
+ throws SignatureException {
+ int mandatoryPositionals = 0;
+ int optionalPositionals = 0;
+ int mandatoryNamedOnly = 0;
+ int optionalNamedOnly = 0;
+ boolean hasStarStar = false;
+ boolean hasStar = false;
+ @Nullable String star = null;
+ @Nullable String starStar = null;
+ @Nullable T starType = null;
+ @Nullable T starStarType = null;
+ ArrayList<String> params = new ArrayList<>();
+ ArrayList<V> defaults = new ArrayList<>();
+ ArrayList<T> types = new ArrayList<>();
+ // mandatory named-only parameters are kept aside to be spliced after the optional ones.
+ ArrayList<String> mandatoryNamedOnlyParams = new ArrayList<>();
+ ArrayList<T> mandatoryNamedOnlyTypes = new ArrayList<>();
+ boolean defaultRequired = false; // true after mandatory positionals and before star.
+ Set<String> paramNameSet = new HashSet<>(); // set of names, to avoid duplicates
+
+ for (Parameter<V, T> param : parameters) {
+ if (hasStarStar) {
+ throw new SignatureException("illegal parameter after star-star parameter", param);
+ }
+ @Nullable String name = param.getName();
+ @Nullable T type = param.getType();
+ if (param.hasName()) {
+ if (paramNameSet.contains(name)) {
+ throw new SignatureException("duplicate parameter name in function definition", param);
+ }
+ paramNameSet.add(name);
+ }
+ if (param.isStarStar()) {
+ hasStarStar = true;
+ starStar = name;
+ starStarType = type;
+ } else if (param.isStar()) {
+ if (hasStar) {
+ throw new SignatureException(
+ "duplicate star parameter in function definition", param);
+ }
+ hasStar = true;
+ defaultRequired = false;
+ if (param.hasName()) {
+ star = name;
+ starType = type;
+ }
+ } else if (hasStar && param.isMandatory()) {
+ // mandatory named-only, added contiguously at the end, to simplify calls
+ mandatoryNamedOnlyParams.add(name);
+ mandatoryNamedOnlyTypes.add(type);
+ mandatoryNamedOnly++;
+ } else {
+ params.add(name);
+ types.add(type);
+ if (param.isMandatory()) {
+ if (defaultRequired) {
+ throw new SignatureException(
+ "a mandatory positional parameter must not follow an optional parameter",
+ param);
+ }
+ mandatoryPositionals++;
+ } else { // At this point, it's an optional parameter
+ defaults.add(param.getDefaultValue());
+ if (hasStar) {
+ // named-only optional
+ optionalNamedOnly++;
+ } else {
+ optionalPositionals++;
+ defaultRequired = true;
+ }
+ }
+ }
+ }
+ params.addAll(mandatoryNamedOnlyParams);
+ types.addAll(mandatoryNamedOnlyTypes);
+ if (star != null) {
+ params.add(star);
+ types.add(starType);
+ }
+ if (starStar != null) {
+ params.add(starStar);
+ types.add(starStarType);
+ }
+ return WithValues.<V, T>create(
+ FunctionSignature.create(
+ Shape.create(
+ mandatoryPositionals, optionalPositionals,
+ mandatoryNamedOnly, optionalNamedOnly,
+ star != null, starStar != null),
+ ImmutableList.<String>copyOf(params)),
+ FunctionSignature.<V>valueListOrNull(defaults),
+ FunctionSignature.<T>valueListOrNull(types));
+ }
+
+ /**
+ * Append a representation of this signature to a string buffer.
+ */
+ public StringBuffer toStringBuffer(final StringBuffer sb) {
+ FunctionSignature signature = getSignature();
+ Shape shape = signature.getShape();
+ final ImmutableList<String> names = signature.getNames();
+ @Nullable final List<V> defaultValues = getDefaultValues();
+ @Nullable final List<T> types = getTypes();
+
+ int mandatoryPositionals = shape.getMandatoryPositionals();
+ int optionalPositionals = shape.getOptionalPositionals();
+ int mandatoryNamedOnly = shape.getMandatoryNamedOnly();
+ int optionalNamedOnly = shape.getOptionalNamedOnly();
+ boolean starArg = shape.hasStarArg();
+ boolean kwArg = shape.hasKwArg();
+ int positionals = mandatoryPositionals + optionalPositionals;
+ int namedOnly = mandatoryNamedOnly + optionalNamedOnly;
+ int named = positionals + namedOnly;
+ int args = named + (starArg ? 1 : 0) + (kwArg ? 1 : 0);
+ int endOptionals = positionals + optionalNamedOnly;
+ boolean hasStar = starArg || (namedOnly > 0);
+ int iStarArg = named;
+ int iKwArg = args - 1;
+
+ class Show {
+ private boolean isMore = false;
+ private int j = 0;
+
+ public void comma() {
+ if (isMore) { sb.append(", "); }
+ isMore = true;
+ }
+ public void type(int i) {
+ if (types != null && types.get(i) != null) {
+ sb.append(" : ").append(types.get(i).toString());
+ }
+ }
+ public void mandatory(int i) {
+ comma();
+ sb.append(names.get(i));
+ type(i);
+ }
+ public void optional(int i) {
+ mandatory(i);
+ sb.append(" = ").append((defaultValues == null)
+ ? "null" : String.valueOf(defaultValues.get(j++)));
+ }
+ };
+ Show show = new Show();
+
+ int i = 0;
+ for (; i < mandatoryPositionals; i++) {
+ show.mandatory(i);
+ }
+ for (; i < positionals; i++) {
+ show.optional(i);
+ }
+ if (hasStar) {
+ show.comma();
+ sb.append("*");
+ if (starArg) {
+ sb.append(names.get(iStarArg));
+ }
+ }
+ for (; i < endOptionals; i++) {
+ show.optional(i);
+ }
+ for (; i < named; i++) {
+ show.mandatory(i);
+ }
+ if (kwArg) {
+ show.comma();
+ sb.append("**");
+ sb.append(names.get(iKwArg));
+ }
+
+ return sb;
+ }
+
+ @Override
+ public String toString() {
+ StringBuffer sb = new StringBuffer();
+ toStringBuffer(sb);
+ return sb.toString();
+ }
+ }
+
+ /** The given List, or null if all the list elements are null. */
+ @Nullable public static <E> List<E> valueListOrNull(List<E> list) {
+ if (list == null) {
+ return null;
+ }
+ for (E value : list) {
+ if (value != null) {
+ return list;
+ }
+ }
+ return null;
+ }
+
+ /**
+ * Constructs a function signature (with names) from signature description and names.
+ * This method covers the general case.
+ * The number of optional named-only parameters is deduced from the other arguments.
+ *
+ * @param numMandatoryPositionals an int for the number of mandatory positional parameters
+ * @param numOptionalPositionals an int for the number of optional positional parameters
+ * @param numMandatoryNamedOnly an int for the number of mandatory named-only parameters
+ * @param starArg a boolean for whether there is a starred parameter
+ * @param kwArg a boolean for whether there is a star-starred parameter
+ * @param names an Array of String for the parameter names
+ * @return a FunctionSignature
+ */
+ public static FunctionSignature of(int numMandatoryPositionals, int numOptionalPositionals,
+ int numMandatoryNamedOnly, boolean starArg, boolean kwArg, String... names) {
+ return create(Shape.create(
+ numMandatoryPositionals,
+ numOptionalPositionals,
+ numMandatoryNamedOnly,
+ names.length - (kwArg ? 1 : 0) - (starArg ? 1 : 0)
+ - numMandatoryPositionals - numOptionalPositionals - numMandatoryNamedOnly,
+ starArg, kwArg),
+ ImmutableList.<String>copyOf(names));
+ }
+
+ /**
+ * Constructs a function signature from mandatory positional argument names.
+ *
+ * @param names an Array of String for the positional parameter names
+ * @return a FunctionSignature
+ */
+ public static FunctionSignature of(String... names) {
+ return of(names.length, 0, 0, false, false, names);
+ }
+
+ /**
+ * Constructs a function signature from positional argument names.
+ *
+ * @param numMandatory an int for the number of mandatory positional parameters
+ * @param names an Array of String for the positional parameter names
+ * @return a FunctionSignature
+ */
+ public static FunctionSignature of(int numMandatory, String... names) {
+ return of(numMandatory, names.length - numMandatory, 0, false, false, names);
+ }
+
+ /**
+ * Constructs a function signature from mandatory named-only argument names.
+ *
+ * @param names an Array of String for the mandatory named-only parameter names
+ * @return a FunctionSignature
+ */
+ public static FunctionSignature namedOnly(String... names) {
+ return of(0, 0, names.length, false, false, names);
+ }
+
+ /**
+ * Constructs a function signature from named-only argument names.
+ *
+ * @param numMandatory an int for the number of mandatory named-only parameters
+ * @param names an Array of String for the named-only parameter names
+ * @return a FunctionSignature
+ */
+ public static FunctionSignature namedOnly(int numMandatory, String... names) {
+ return of(0, 0, numMandatory, false, false, names);
+ }
+
+ /** Invalid signature from Parser or from SkylarkBuiltin annotations */
+ protected static class SignatureException extends Exception {
+ @Nullable private final Parameter<?, ?> parameter;
+
+ /** SignatureException from a message and a Parameter */
+ public SignatureException(String message, @Nullable Parameter<?, ?> parameter) {
+ super(message);
+ this.parameter = parameter;
+ }
+
+ /** what parameter caused the exception, if identified? */
+ @Nullable public Parameter<?, ?> getParameter() {
+ return parameter;
+ }
+ }
+
+ /** A ready-made signature to allow only keyword arguments and put them in a kwarg parameter */
+ public static final FunctionSignature KWARGS =
+ FunctionSignature.of(0, 0, 0, false, true, "kwargs");
+
+
+ // Minimal boilerplate to get things running in absence of AutoValue
+ // TODO(bazel-team): actually migrate to AutoValue when possible,
+ // which importantly for future plans will define .equals() and .hashValue() (also toString())
+ private static class AutoValueFunctionSignatureShape extends Shape {
+ private int mandatoryPositionals;
+ private int optionalPositionals;
+ private int mandatoryNamedOnly;
+ private int optionalNamedOnly;
+ private boolean starArg;
+ private boolean kwArg;
+
+ @Override public int getMandatoryPositionals() { return mandatoryPositionals; }
+ @Override public int getOptionalPositionals() { return optionalPositionals; }
+ @Override public int getMandatoryNamedOnly() { return mandatoryNamedOnly; }
+ @Override public int getOptionalNamedOnly() { return optionalNamedOnly; }
+ @Override public boolean hasStarArg() { return starArg; }
+ @Override public boolean hasKwArg() { return kwArg; }
+
+ public AutoValueFunctionSignatureShape(
+ int mandatoryPositionals, int optionalPositionals,
+ int mandatoryNamedOnly, int optionalNamedOnly, boolean starArg, boolean kwArg) {
+ this.mandatoryPositionals = mandatoryPositionals;
+ this.optionalPositionals = optionalPositionals;
+ this.mandatoryNamedOnly = mandatoryNamedOnly;
+ this.optionalNamedOnly = optionalNamedOnly;
+ this.starArg = starArg;
+ this.kwArg = kwArg;
+ }
+ }
+
+ private static class AutoValueFunctionSignature extends FunctionSignature {
+ private Shape shape;
+ private ImmutableList<String> names;
+
+ @Override public Shape getShape() { return shape; }
+ @Override public ImmutableList<String> getNames() { return names; }
+
+ public AutoValueFunctionSignature(Shape shape, ImmutableList<String> names) {
+ this.shape = shape;
+ this.names = names;
+ }
+ }
+
+ private static class AutoValueFunctionSignatureWithValues<V, T> extends WithValues<V, T> {
+ private FunctionSignature signature;
+ private List<V> defaultValues;
+ private List<T> types;
+
+ @Override public FunctionSignature getSignature() { return signature; }
+ @Override public List<V> getDefaultValues() { return defaultValues; }
+ @Override public List<T> getTypes() { return types; }
+
+ public AutoValueFunctionSignatureWithValues(
+ FunctionSignature signature, List<V> defaultValues, List<T> types) {
+ this.signature = signature;
+ this.defaultValues = defaultValues;
+ this.types = types;
+ }
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Ident.java b/src/main/java/com/google/devtools/build/lib/syntax/Ident.java
index 86bd458..19d4483 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Ident.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Ident.java
@@ -14,6 +14,13 @@
package com.google.devtools.build.lib.syntax;
+// TODO(bazel-team): for extra performance:
+// (1) intern the strings, so we can use == to compare, and have .equals use the assumption.
+// Then have Argument and Parameter use Ident again instead of String as keys.
+// (2) Use Ident, not String, as keys in the Environment, which will be cleaner.
+// (3) For performance, avoid doing HashMap lookups at runtime, and compile local variable access
+// into array reference with a constant index. Variable lookups are currently a speed bottleneck,
+// as previously measured in an experiment.
/**
* Syntax node for an identifier.
*/
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Lexer.java b/src/main/java/com/google/devtools/build/lib/syntax/Lexer.java
index 3b64492..4c0c9b9 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Lexer.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Lexer.java
@@ -585,6 +585,8 @@
TokenKind tok = null;
if (c2 == '=') {
tok = EQUAL_TOKENS.get(c1);
+ } else if (c2 == '*' && c1 == '*') {
+ tok = TokenKind.STAR_STAR;
}
if (tok == null) {
return false;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/MixedModeFunction.java b/src/main/java/com/google/devtools/build/lib/syntax/MixedModeFunction.java
index 0427157..4116328 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/MixedModeFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/MixedModeFunction.java
@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.syntax;
+import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.events.Location;
@@ -31,6 +32,10 @@
// "Parameters" are formal parameters of a function definition.
// "Arguments" are actual parameters supplied at the call site.
+ // A function signature, including defaults and types
+ // never null after it is configured
+ protected FunctionSignature.WithValues<Object, SkylarkType> signature;
+
// Number of regular named parameters (excluding *p and **p) in the
// equivalent Python function definition).
private final List<String> parameters;
@@ -72,6 +77,36 @@
this.numMandatoryParameters = numMandatoryParameters;
this.onlyNamedArguments = onlyNamedArguments;
this.location = location;
+
+ // Fake a signature from the above
+ this.signature = FunctionSignature.WithValues.<Object, SkylarkType>create(
+ FunctionSignature.of(numMandatoryParameters, this.parameters.toArray(new String[0])));
+ }
+
+
+ /** Create a function using a signature with defaults */
+ public MixedModeFunction(String name,
+ FunctionSignature.WithValues<Object, SkylarkType> signature,
+ Location location) {
+ super(name);
+
+ // TODO(bazel-team): lift the following limitations, by actually implementing
+ // the full function call protocol.
+ FunctionSignature sig = signature.getSignature();
+ FunctionSignature.Shape shape = sig.getShape();
+ Preconditions.checkArgument(!shape.hasKwArg() && !shape.hasStarArg()
+ && shape.getNamedOnly() == 0, "no star, star-star or named-only parameters (for now)");
+
+ this.signature = signature;
+ this.parameters = ImmutableList.copyOf(sig.getNames());
+ this.numMandatoryParameters = shape.getMandatoryPositionals();
+ this.onlyNamedArguments = false;
+ this.location = location;
+ }
+
+ /** Create a function using a signature without defaults */
+ public MixedModeFunction(String name, FunctionSignature signature) {
+ this(name, FunctionSignature.WithValues.<Object, SkylarkType>create(signature), null);
}
@Override
@@ -124,14 +159,25 @@
}
}
- // third, defaults:
+ // third, check mandatory parameters:
for (int ii = 0; ii < numMandatoryParameters; ++ii) {
if (namedArguments[ii] == null) {
throw new EvalException(loc,
getSignature() + " received insufficient arguments");
}
}
- // (defaults are always null so nothing extra to do here.)
+
+ // fourth, fill in defaults from the signature, if any
+ List<Object> defaults = signature.getDefaultValues();
+ if (defaults != null) {
+ int jj = 0;
+ for (int ii = numMandatoryParameters; ii < numParams; ++ii) {
+ if (namedArguments[ii] == null) {
+ namedArguments[ii] = defaults.get(jj);
+ }
+ jj++;
+ }
+ }
try {
return call(namedArguments, ast, env);
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Parameter.java b/src/main/java/com/google/devtools/build/lib/syntax/Parameter.java
new file mode 100644
index 0000000..14267d8
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Parameter.java
@@ -0,0 +1,170 @@
+// Copyright 2014 Google Inc. All rights reserved.
+//
+// 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.devtools.build.lib.syntax;
+
+import javax.annotation.Nullable;
+
+/**
+ * Syntax node for a Parameter in a function (or lambda) definition; it's a subclass of Argument,
+ * and contrasts with the class Argument.Passed of arguments in a function call.
+ *
+ * <p>There are four concrete subclasses of Parameter: Mandatory, Optional, Star, StarStar.
+ *
+ * <p>See FunctionSignature for how a valid list of Parameter's is organized as a signature, e.g.
+ * def foo(mandatory, optional = e1, *args, mandatorynamedonly, optionalnamedonly = e2, **kw): ...
+ *
+ * <p>V is the class of a defaultValue (Expression at compile-time, Object at runtime),
+ * T is the class of a type (Expression at compile-time, SkylarkType at runtime).
+ */
+public abstract class Parameter<V, T> extends Argument {
+
+ @Nullable protected String name;
+ @Nullable protected final T type;
+
+ private Parameter(@Nullable String name, @Nullable T type) {
+ this.name = name;
+ this.type = type;
+ }
+ private Parameter(@Nullable String name) {
+ this.name = name;
+ this.type = null;
+ }
+
+ public boolean isMandatory() {
+ return false;
+ }
+ public boolean isOptional() {
+ return false;
+ }
+ public boolean isStar() {
+ return false;
+ }
+ public boolean isStarStar() {
+ return false;
+ }
+ @Nullable public String getName() {
+ return name;
+ }
+ public boolean hasName() {
+ return true;
+ }
+ @Nullable public T getType() {
+ return type;
+ }
+ @Nullable public V getDefaultValue() {
+ return null;
+ }
+
+ /** mandatory parameter (positional or key-only depending on position): Ident */
+ public static class Mandatory<V, T> extends Parameter<V, T> {
+
+ public Mandatory(String name) {
+ super(name);
+ }
+
+ public Mandatory(String name, @Nullable T type) {
+ super(name, type);
+ }
+
+ @Override public boolean isMandatory() {
+ return true;
+ }
+
+ @Override
+ public String toString() {
+ return name.toString();
+ }
+ }
+
+ /** optional parameter (positional or key-only depending on position): Ident = Value */
+ public static class Optional<V, T> extends Parameter<V, T> {
+ public final V defaultValue;
+
+ public Optional(String name, @Nullable V defaultValue) {
+ super(name);
+ this.defaultValue = defaultValue;
+ }
+
+ public Optional(String name, @Nullable T type, @Nullable V defaultValue) {
+ super(name, type);
+ this.defaultValue = defaultValue;
+ }
+
+ @Override @Nullable public V getDefaultValue() {
+ return defaultValue;
+ }
+
+ @Override public boolean isOptional() {
+ return true;
+ }
+
+ @Override
+ public String toString() {
+ return name.toString() + "=" + String.valueOf(defaultValue);
+ }
+ }
+
+ /** extra positionals parameter (star): *identifier */
+ public static class Star<V, T> extends Parameter<V, T> {
+ public Star(@Nullable String name, @Nullable T type) {
+ super(name, type);
+ }
+
+ public Star(@Nullable String name) {
+ super(name);
+ }
+
+ @Override
+ public boolean hasName() {
+ return name != null;
+ }
+
+ @Override public boolean isStar() {
+ return true;
+ }
+
+ @Override public String toString() {
+ if (name == null) {
+ return "*";
+ } else {
+ return "*" + name.toString();
+ }
+ }
+ }
+
+ /** extra keywords parameter (star_star): **identifier */
+ public static class StarStar<V, T> extends Parameter<V, T> {
+ public StarStar(String name, @Nullable T type) {
+ super(name, type);
+ }
+
+ public StarStar(String name) {
+ super(name);
+ }
+
+ @Override public boolean isStarStar() {
+ return true;
+ }
+
+ @Override
+ public String toString() {
+ return "**" + name.toString();
+ }
+ }
+
+ @Override
+ public void accept(SyntaxTreeVisitor visitor) {
+ visitor.visit(this);
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
index d86946a..93d6a06 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
@@ -16,6 +16,7 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
+import com.google.common.base.Supplier;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.events.Event;
@@ -30,11 +31,9 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.EnumSet;
-import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
-import java.util.Set;
/**
* Recursive descent parser for LL(2) BUILD language.
@@ -350,89 +349,92 @@
// create a funcall expression
private Expression makeFuncallExpression(Expression receiver, Ident function,
- List<Argument> args,
+ List<Argument.Passed> args,
int start, int end) {
if (function.getLocation() == null) {
function = setLocation(function, start, end);
}
- boolean seenKeywordArg = false;
- boolean seenKwargs = false;
- for (Argument arg : args) {
- if (arg.isPositional()) {
- if (seenKeywordArg || seenKwargs) {
- reportError(arg.getLocation(), "syntax error: non-keyword arg after keyword arg");
- return makeErrorExpression(start, end);
- }
- } else if (arg.isKwargs()) {
- if (seenKwargs) {
- reportError(arg.getLocation(), "there can be only one **kwargs argument");
- return makeErrorExpression(start, end);
- }
- seenKwargs = true;
- } else {
- seenKeywordArg = true;
- }
- }
-
return setLocation(new FuncallExpression(receiver, function, args), start, end);
}
// arg ::= IDENTIFIER '=' expr
// | expr
- private Argument parseFunctionCallArgument() {
- int start = token.left;
+ private Argument.Passed parseFuncallArgument() {
+ final int start = token.left;
+ // parse **expr
+ if (token.kind == TokenKind.STAR_STAR) {
+ nextToken();
+ Expression expr = parseExpression();
+ return setLocation(new Argument.StarStar(expr), start, expr);
+ }
+ // parse *expr
+ if (token.kind == TokenKind.STAR) {
+ nextToken();
+ Expression expr = parseExpression();
+ return setLocation(new Argument.Star(expr), start, expr);
+ }
+ // parse keyword = expr
if (token.kind == TokenKind.IDENTIFIER) {
Token identToken = token;
String name = (String) token.value;
- Ident ident = setLocation(new Ident(name), start, token.right);
nextToken();
if (token.kind == TokenKind.EQUALS) { // it's a named argument
nextToken();
Expression expr = parseExpression();
- return setLocation(new Argument(ident, expr), start, expr);
+ return setLocation(new Argument.Keyword(name, expr), start, expr);
} else { // oops, back up!
pushToken(identToken);
}
}
- // parse **expr
- if (token.kind == TokenKind.STAR) {
- expect(TokenKind.STAR);
- expect(TokenKind.STAR);
- Expression expr = parseExpression();
- return setLocation(new Argument(null, expr, true), start, expr);
- }
// parse a positional argument
Expression expr = parseExpression();
- return setLocation(new Argument(expr), start, expr);
+ return setLocation(new Argument.Positional(expr), start, expr);
}
// arg ::= IDENTIFIER '=' expr
// | IDENTIFIER
- private Argument parseFunctionDefArgument(boolean onlyOptional) {
+ private Parameter<Expression, Expression> parseFunctionParameter() {
+ // TODO(bazel-team): optionally support type annotations
int start = token.left;
- Ident ident = parseIdent();
- if (token.kind == TokenKind.EQUALS) { // there's a default value
+ if (token.kind == TokenKind.STAR_STAR) { // kwarg
nextToken();
- Expression expr = parseExpression();
- return setLocation(new Argument(ident, expr), start, expr);
- } else if (onlyOptional) {
- reportError(ident.getLocation(),
- "Optional arguments are only allowed at the end of the argument list.");
+ Ident ident = parseIdent();
+ return setLocation(new Parameter.StarStar<Expression, Expression>(
+ ident.getName()), start, ident);
+ } else if (token.kind == TokenKind.STAR) { // stararg
+ int end = token.right;
+ nextToken();
+ if (token.kind == TokenKind.IDENTIFIER) {
+ Ident ident = parseIdent();
+ return setLocation(new Parameter.Star<Expression, Expression>(ident.getName()),
+ start, ident);
+ } else {
+ return setLocation(new Parameter.Star<Expression, Expression>(null), start, end);
+ }
+ } else {
+ Ident ident = parseIdent();
+ if (token.kind == TokenKind.EQUALS) { // there's a default value
+ nextToken();
+ Expression expr = parseExpression();
+ return setLocation(new Parameter.Optional<Expression, Expression>(
+ ident.getName(), expr), start, expr);
+ } else {
+ return setLocation(new Parameter.Mandatory<Expression, Expression>(
+ ident.getName()), start, ident);
+ }
}
- return setLocation(new Argument(ident), start, ident);
}
// funcall_suffix ::= '(' arg_list? ')'
- private Expression parseFuncallSuffix(int start, Expression receiver,
- Ident function) {
- List<Argument> args = Collections.emptyList();
+ private Expression parseFuncallSuffix(int start, Expression receiver, Ident function) {
+ List<Argument.Passed> args = Collections.emptyList();
expect(TokenKind.LPAREN);
int end;
if (token.kind == TokenKind.RPAREN) {
end = token.right;
nextToken(); // RPAREN
} else {
- args = parseFunctionCallArguments(); // (includes optional trailing comma)
+ args = parseFuncallArguments(); // (includes optional trailing comma)
end = token.right;
expect(TokenKind.RPAREN);
}
@@ -458,22 +460,19 @@
}
// arg_list ::= ( (arg ',')* arg ','? )?
- private List<Argument> parseFunctionCallArguments() {
- List<Argument> args = new ArrayList<>();
- // terminating tokens for an arg list
- while (token.kind != TokenKind.RPAREN) {
- if (token.kind == TokenKind.EOF) {
- syntaxError(token);
- break;
- }
- args.add(parseFunctionCallArgument());
- if (token.kind == TokenKind.COMMA) {
- nextToken();
- } else {
- break;
- }
+ private List<Argument.Passed> parseFuncallArguments() {
+ List<Argument.Passed> arguments =
+ parseFunctionArguments(new Supplier<Argument.Passed>() {
+ @Override public Argument.Passed get() {
+ return parseFuncallArgument();
+ }
+ });
+ try {
+ Argument.validateFuncallArguments(arguments);
+ } catch (Argument.ArgumentException e) {
+ reportError(lexer.createLocation(token.left, token.right), e.getMessage());
}
- return args;
+ return arguments;
}
// expr_list ::= ( (expr ',')* expr ','? )?
@@ -517,9 +516,11 @@
private ExpressionStatement mocksubincludeExpression(
String labelName, String file, Location location) {
- List<Argument> args = new ArrayList<>();
- args.add(setLocation(new Argument(new StringLiteral(labelName, '"')), location));
- args.add(setLocation(new Argument(new StringLiteral(file, '"')), location));
+ List<Argument.Passed> args = new ArrayList<>();
+ args.add(setLocation(new Argument.Positional(
+ new StringLiteral(labelName, '"')), location));
+ args.add(setLocation(new Argument.Positional(
+ new StringLiteral(file, '"')), location));
Ident mockIdent = setLocation(new Ident("mocksubinclude"), location);
Expression funCall = new FuncallExpression(null, mockIdent, args);
return setLocation(new ExpressionStatement(funCall), location);
@@ -646,9 +647,9 @@
case MINUS: {
nextToken();
- List<Argument> args = new ArrayList<>();
+ List<Argument.Passed> args = new ArrayList<>();
Expression expr = parsePrimaryWithSuffix();
- args.add(setLocation(new Argument(expr), start, expr));
+ args.add(setLocation(new Argument.Positional(expr), start, expr));
return makeFuncallExpression(null, new Ident("-"), args,
start, token.right);
}
@@ -679,7 +680,7 @@
// substring_suffix ::= '[' expression? ':' expression? ']'
private Expression parseSubstringSuffix(int start, Expression receiver) {
- List<Argument> args = new ArrayList<>();
+ List<Argument.Passed> args = new ArrayList<>();
Expression startExpr;
Expression endExpr;
@@ -690,7 +691,7 @@
} else {
startExpr = parseExpression();
}
- args.add(setLocation(new Argument(startExpr), loc1, startExpr));
+ args.add(setLocation(new Argument.Positional(startExpr), loc1, startExpr));
// This is a dictionary access
if (token.kind == TokenKind.RBRACKET) {
expect(TokenKind.RBRACKET);
@@ -707,7 +708,7 @@
}
expect(TokenKind.RBRACKET);
- args.add(setLocation(new Argument(endExpr), loc2, endExpr));
+ args.add(setLocation(new Argument.Positional(endExpr), loc2, endExpr));
return makeFuncallExpression(receiver, new Ident("$substring"), args,
start, token.right);
}
@@ -1121,38 +1122,78 @@
expect(TokenKind.DEF);
Ident ident = parseIdent();
expect(TokenKind.LPAREN);
- // parsing the function arguments, at this point only identifiers
- // TODO(bazel-team): support proper arguments with default values and kwargs
- List<Argument> args = parseFunctionDefArguments();
+ FunctionSignature.WithValues<Expression, Expression> args = parseFunctionSignature();
expect(TokenKind.RPAREN);
expect(TokenKind.COLON);
List<Statement> block = parseSuite();
+ // TODO(bazel-team): lift this limitation
+ FunctionSignature.Shape shape = args.getSignature().getShape();
+ if (shape.hasKwArg() || shape.hasStarArg() || shape.getNamedOnly() > 0) {
+ reportError(lexer.createLocation(start, token.right),
+ "no star, star-star or named-only parameters (for now)");
+ return;
+ }
FunctionDefStatement stmt = new FunctionDefStatement(ident, args, block);
list.add(setLocation(stmt, start, token.right));
}
- private List<Argument> parseFunctionDefArguments() {
- List<Argument> args = new ArrayList<>();
- Set<String> argNames = new HashSet<>();
- boolean onlyOptional = false;
- while (token.kind != TokenKind.RPAREN) {
- Argument arg = parseFunctionDefArgument(onlyOptional);
- if (arg.hasValue()) {
- onlyOptional = true;
- }
- args.add(arg);
- if (argNames.contains(arg.getArgName())) {
+ private FunctionSignature.WithValues<Expression, Expression> parseFunctionSignature() {
+ List<Parameter<Expression, Expression>> parameters =
+ parseFunctionArguments(new Supplier<Parameter<Expression, Expression>>() {
+ @Override public Parameter<Expression, Expression> get() {
+ return parseFunctionParameter();
+ }
+ });
+ try {
+ return FunctionSignature.WithValues.<Expression, Expression>of(parameters);
+ } catch (FunctionSignature.SignatureException e) {
+ reportError(e.getParameter().getLocation(), e.getMessage());
+ // return bogus empty signature
+ return FunctionSignature.WithValues.<Expression, Expression>create(FunctionSignature.of());
+ }
+ }
+
+ /**
+ * Parse a list of Argument-s. The arguments can be of class Argument.Passed or Parameter,
+ * as returned by the Supplier parseArgument (that, taking no argument, must be closed over
+ * the mutable input data structures).
+ *
+ * <p>This parser does minimal validation: it ensures the proper python use of the comma (that
+ * can terminate before a star but not after) and the fact that a **kwarg must appear last.
+ * It does NOT validate further ordering constraints for a {@code List<Argument.Passed>}, such as
+ * all positional preceding keyword arguments in a call, nor does it check the more subtle
+ * constraints for Parameter-s. This validation must happen afterwards in an appropriate method.
+ */
+ private <V extends Argument> ImmutableList<V>
+ parseFunctionArguments(Supplier<V> parseArgument) {
+ boolean hasArg = false;
+ boolean hasStar = false;
+ boolean hasStarStar = false;
+ ArrayList<V> arguments = new ArrayList<>();
+
+ while (token.kind != TokenKind.RPAREN && token.kind != TokenKind.EOF) {
+ if (hasStarStar) {
reportError(lexer.createLocation(token.left, token.right),
- "duplicate argument name in function definition");
- }
- argNames.add(arg.getArgName());
- if (token.kind == TokenKind.COMMA) {
- nextToken();
- } else {
+ "unexpected tokens after kwarg");
break;
}
+ if (hasArg) {
+ expect(TokenKind.COMMA);
+ }
+ if (token.kind == TokenKind.RPAREN && !hasStar) {
+ // list can end with a COMMA if there is neither * nor **
+ break;
+ }
+ V arg = parseArgument.get();
+ hasArg = true;
+ if (arg.isStar()) {
+ hasStar = true;
+ } else if (arg.isStarStar()) {
+ hasStarStar = true;
+ }
+ arguments.add(arg);
}
- return args;
+ return ImmutableList.copyOf(arguments);
}
// suite ::= simple_stmt
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkFunction.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkFunction.java
index 8ab64cb..e6959e8 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkFunction.java
@@ -22,6 +22,7 @@
import com.google.devtools.build.lib.packages.Type.ConversionException;
import com.google.devtools.build.lib.syntax.EvalException.EvalExceptionWithJavaCause;
+import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
@@ -101,6 +102,7 @@
try {
ImmutableMap.Builder<String, Object> arguments = new ImmutableMap.Builder<>();
if (objectType != null && !FuncallExpression.isNamespace(objectType)) {
+ args = new ArrayList<Object>(args); // args immutable, get a mutable copy.
arguments.put("self", args.remove(0));
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SyntaxTreeVisitor.java b/src/main/java/com/google/devtools/build/lib/syntax/SyntaxTreeVisitor.java
index 5a95026..e53a7aa 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SyntaxTreeVisitor.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SyntaxTreeVisitor.java
@@ -37,13 +37,8 @@
}
// node specific visit methods
- public void visit(Argument node) {
- if (node.isNamed()) {
- visit(node.getName());
- }
- if (node.hasValue()) {
- visit(node.getValue());
- }
+ public void visit(Argument.Passed node) {
+ visit(node.getValue());
}
public void visit(BuildFileAST node) {
@@ -116,8 +111,11 @@
public void visit(FunctionDefStatement node) {
visit(node.getIdent());
- for (Argument arg : node.getArgs()) {
- visit(arg);
+ List<Expression> defaults = node.getArgs().getDefaultValues();
+ if (defaults != null) {
+ for (Expression expr : defaults) {
+ visit(expr);
+ }
}
for (Statement stmt : node.getStatements()) {
visit(stmt);
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/TokenKind.java b/src/main/java/com/google/devtools/build/lib/syntax/TokenKind.java
index f6dad9f..889fa66 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/TokenKind.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/TokenKind.java
@@ -65,6 +65,7 @@
RPAREN(")"),
SEMI(";"),
STAR("*"),
+ STAR_STAR("**"),
STRING("string"),
TRY("try");
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java b/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java
index cd909a9..58a5b4c 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java
@@ -13,10 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.syntax;
-import com.google.common.base.Function;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Lists;
import com.google.devtools.build.lib.events.Location;
/**
@@ -25,62 +22,20 @@
*/
public class UserDefinedFunction extends MixedModeFunction {
- private final ImmutableList<Argument> args;
- private final ImmutableMap<String, Integer> argIndexes;
- private final ImmutableMap<String, Object> defaultValues;
private final ImmutableList<Statement> statements;
private final SkylarkEnvironment definitionEnv;
- private static ImmutableList<String> argumentToStringList(ImmutableList<Argument> args) {
- Function<Argument, String> function = new Function<Argument, String>() {
- @Override
- public String apply(Argument id) {
- return id.getArgName();
- }
- };
- return ImmutableList.copyOf(Lists.transform(args, function));
- }
-
- private static int mandatoryArgNum(ImmutableList<Argument> args) {
- int mandatoryArgNum = 0;
- for (Argument arg : args) {
- if (!arg.hasValue()) {
- mandatoryArgNum++;
- }
- }
- return mandatoryArgNum;
- }
-
- UserDefinedFunction(Ident function, ImmutableList<Argument> args,
- ImmutableMap<String, Object> defaultValues,
+ protected UserDefinedFunction(Ident function,
+ FunctionSignature.WithValues<Object, SkylarkType> signature,
ImmutableList<Statement> statements, SkylarkEnvironment definitionEnv) {
- super(function.getName(), argumentToStringList(args), mandatoryArgNum(args), false,
- function.getLocation());
- this.args = args;
+ super(function.getName(), signature, function.getLocation());
+
this.statements = statements;
this.definitionEnv = definitionEnv;
- this.defaultValues = defaultValues;
-
- ImmutableMap.Builder<String, Integer> argIndexes = new ImmutableMap.Builder<> ();
- int i = 0;
- for (Argument arg : args) {
- if (!arg.isKwargs()) { // TODO(bazel-team): add varargs support?
- argIndexes.put(arg.getArgName(), i++);
- }
- }
- this.argIndexes = argIndexes.build();
}
- public ImmutableList<Argument> getArgs() {
- return args;
- }
-
- public Integer getArgIndex(String s) {
- return argIndexes.get(s);
- }
-
- ImmutableMap<String, Object> getDefaultValues() {
- return defaultValues;
+ public FunctionSignature.WithValues<Object, SkylarkType> getFunctionSignature() {
+ return signature;
}
ImmutableList<Statement> getStatements() {
@@ -91,16 +46,18 @@
return location;
}
+
@Override
- public Object call(Object[] namedArguments, FuncallExpression ast, Environment env)
+ public Object call(Object[] arguments, FuncallExpression ast, Environment env)
throws EvalException, InterruptedException {
SkylarkEnvironment functionEnv = SkylarkEnvironment.createEnvironmentForFunctionCalling(
env, definitionEnv, this);
+ ImmutableList<String> names = signature.getSignature().getNames();
// Registering the functions's arguments as variables in the local Environment
int i = 0;
- for (Object arg : namedArguments) {
- functionEnv.update(args.get(i++).getArgName(), arg);
+ for (String name : names) {
+ functionEnv.update(name, arguments[i++]);
}
try {