Unify Skylark and BUILD lists
Use SkylarkList everywhere rather than either List or GlobList.
Keep a GlobList underneath a MutableList, where applicable.
--
MOS_MIGRATED_REVID=105864035
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java
index d4d8f6e..c34ad70 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java
@@ -16,7 +16,6 @@
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
-import com.google.common.collect.Lists;
import com.google.devtools.build.lib.syntax.ClassObject.SkylarkClassObject;
import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
import com.google.devtools.build.lib.syntax.SkylarkList.Tuple;
@@ -258,47 +257,18 @@
return (String) lval + (String) rval;
}
- // list + list, tuple + tuple (list + tuple, tuple + list => error)
- if (lval instanceof List<?> && rval instanceof List<?>) {
- List<?> llist = (List<?>) lval;
- List<?> rlist = (List<?>) rval;
- if (EvalUtils.isTuple(llist) != EvalUtils.isTuple(rlist)) {
- throw new EvalException(getLocation(), "can only concatenate "
- + EvalUtils.getDataTypeName(llist) + " (not \"" + EvalUtils.getDataTypeName(rlist)
- + "\") to " + EvalUtils.getDataTypeName(llist));
- }
- if (llist instanceof GlobList<?> || rlist instanceof GlobList<?>) {
- return GlobList.concat(llist, rlist);
- } else {
- List<Object> result = Lists.newArrayListWithCapacity(llist.size() + rlist.size());
- result.addAll(llist);
- result.addAll(rlist);
- return EvalUtils.makeSequence(result, EvalUtils.isTuple(llist));
- }
- }
-
if (lval instanceof SelectorValue || rval instanceof SelectorValue
|| lval instanceof SelectorList
|| rval instanceof SelectorList) {
return SelectorList.concat(getLocation(), lval, rval);
}
- if ((lval instanceof SkylarkList || lval instanceof List<?>)
- && ((rval instanceof SkylarkList || rval instanceof List<?>))) {
- SkylarkList left = (SkylarkList) SkylarkType.convertToSkylark(lval, env);
- SkylarkList right = (SkylarkList) SkylarkType.convertToSkylark(rval, env);
- boolean isImmutable = left.isTuple();
- if (isImmutable != right.isTuple()) {
- throw new EvalException(getLocation(), "can only concatenate "
- + EvalUtils.getDataTypeName(left) + " (not \"" + EvalUtils.getDataTypeName(right)
- + "\") to " + EvalUtils.getDataTypeName(left));
- }
- Iterable<Object> concatenated = Iterables.concat(left, right);
- if (isImmutable) {
- return Tuple.copyOf(concatenated);
- } else {
- return new MutableList(concatenated, env);
- }
+ if ((lval instanceof Tuple) && (rval instanceof Tuple)) {
+ return Tuple.copyOf(Iterables.concat((Tuple) lval, (Tuple) rval));
+ }
+
+ if ((lval instanceof MutableList) && (rval instanceof MutableList)) {
+ return MutableList.concat((MutableList) lval, (MutableList) rval, env);
}
if (lval instanceof Map<?, ?> && rval instanceof Map<?, ?>) {
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BuiltinFunction.java b/src/main/java/com/google/devtools/build/lib/syntax/BuiltinFunction.java
index 9b5b95f..fffad90 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/BuiltinFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/BuiltinFunction.java
@@ -17,7 +17,6 @@
import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.profiler.Profiler;
import com.google.devtools.build.lib.profiler.ProfilerTask;
-import com.google.devtools.build.lib.syntax.SkylarkSignatureProcessor.HackHackEitherList;
import com.google.devtools.build.lib.syntax.SkylarkType.SkylarkFunctionType;
import java.lang.reflect.InvocationTargetException;
@@ -281,9 +280,6 @@
if (returnType != null) {
Class<?> type = returnType;
- if (type == HackHackEitherList.class) {
- type = Object.class;
- }
Class<?> methodReturnType = invokeMethod.getReturnType();
Preconditions.checkArgument(type == methodReturnType,
"signature for function %s says it returns %s but its invoke method returns %s",
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java
index 82ec917..03bc6ff 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java
@@ -398,17 +398,6 @@
return isSkylark;
}
- /**
- * Is the caller of the current function executing Skylark code?
- * @return true if this is skylark was enabled when this code was read.
- */
- // TODO(bazel-team): Delete this function.
- // This function is currently used by MethodLibrary to modify behavior of lists
- // depending on the Skylark-ness of the code; lists should be unified between the two modes.
- boolean isCallerSkylark() {
- return continuation.isSkylark;
- }
-
@Override
public Mutability mutability() {
// the mutability of the environment is that of its dynamic frame.
@@ -808,11 +797,6 @@
}
Object value = ext.get(nameInLoadedFile);
- // TODO(bazel-team): Unify data structures between Skylark and BUILD,
- // and stop doing the conversions below:
- if (!isSkylark) {
- value = SkylarkType.convertFromSkylark(value);
- }
try {
update(symbol.getName(), value);
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 c1382ca..cbe5564 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
@@ -43,12 +43,6 @@
*/
public final class FuncallExpression extends Expression {
- private static enum ArgConversion {
- FROM_SKYLARK,
- TO_SKYLARK,
- NO_CONVERSION
- }
-
/**
* A value class to store Methods with their corresponding SkylarkCallable annotations.
* This is needed because the annotation is sometimes in a superclass.
@@ -334,6 +328,7 @@
+ Printer.listString(ImmutableList.copyOf(args), "(", ", ", ")", null));
}
}
+ // TODO(bazel-team): get rid of this, by having everyone use the Skylark data structures
result = SkylarkType.convertToSkylark(result, method, env);
if (result != null && !EvalUtils.isSkylarkAcceptable(result.getClass())) {
throw new EvalException(loc, Printer.format(
@@ -444,9 +439,8 @@
@SuppressWarnings("unchecked")
private void evalArguments(ImmutableList.Builder<Object> posargs, Map<String, Object> kwargs,
- Environment env, BaseFunction function)
+ Environment env)
throws EvalException, InterruptedException {
- ArgConversion conversion = getArgConversion(function);
ImmutableList.Builder<String> duplicates = new ImmutableList.Builder<>();
// Iterate over the arguments. We assume all positional arguments come before any keyword
// or star arguments, because the argument list was already validated by
@@ -454,14 +448,6 @@
// 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);
- } else if (conversion == ArgConversion.TO_SKYLARK) {
- // We try to auto convert the type if we can.
- value = SkylarkType.convertToSkylark(value, env);
- // 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.isStar()) { // expand the starArg
@@ -514,10 +500,8 @@
// Add self as an implicit parameter in front.
posargs.add(objValue);
}
- evalArguments(posargs, kwargs, env, function);
- return convertFromSkylark(
- function.call(posargs.build(), ImmutableMap.<String, Object>copyOf(kwargs), this, env),
- env);
+ evalArguments(posargs, kwargs, env);
+ return function.call(posargs.build(), ImmutableMap.<String, Object>copyOf(kwargs), this, env);
} else if (objValue instanceof ClassObject) {
Object fieldValue = ((ClassObject) objValue).getValue(func.getName());
if (fieldValue == null) {
@@ -529,15 +513,13 @@
getLocation(), String.format("struct field '%s' is not a function", func.getName()));
}
function = (BaseFunction) fieldValue;
- evalArguments(posargs, kwargs, env, function);
- return convertFromSkylark(
- function.call(posargs.build(), ImmutableMap.<String, Object>copyOf(kwargs), this, env),
- env);
+ evalArguments(posargs, kwargs, env);
+ return function.call(posargs.build(), ImmutableMap.<String, Object>copyOf(kwargs), this, env);
} else if (env.isSkylark()) {
// 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);
+ evalArguments(posargs, kwargs, env);
Class<?> objClass;
Object obj;
if (objValue instanceof Class<?>) {
@@ -579,38 +561,14 @@
Map<String, Object> kwargs = new HashMap<>();
if ((funcValue instanceof BaseFunction)) {
BaseFunction function = (BaseFunction) funcValue;
- evalArguments(posargs, kwargs, env, function);
- return convertFromSkylark(
- function.call(posargs.build(), ImmutableMap.<String, Object>copyOf(kwargs), this, env),
- env);
+ evalArguments(posargs, kwargs, env);
+ return function.call(posargs.build(), ImmutableMap.<String, Object>copyOf(kwargs), this, env);
} else {
throw new EvalException(
getLocation(), "'" + EvalUtils.getDataTypeName(funcValue) + "' object is not callable");
}
}
- protected Object convertFromSkylark(Object returnValue, Environment env) throws EvalException {
- EvalUtils.checkNotNull(this, returnValue);
- if (!env.isSkylark()) {
- // The call happens in the BUILD language. Note that accessing "BUILD language" functions in
- // Skylark should never happen.
- return SkylarkType.convertFromSkylark(returnValue);
- }
- return returnValue;
- }
-
- private ArgConversion getArgConversion(BaseFunction function) {
- if (function == null) {
- // It means we try to call a Java function.
- return ArgConversion.FROM_SKYLARK;
- }
- // If we call a UserDefinedFunction we call into Skylark. If we call from Skylark
- // the argument conversion is invariant, but if we call from the BUILD language
- // we might need an auto conversion.
- return function instanceof UserDefinedFunction
- ? ArgConversion.TO_SKYLARK : ArgConversion.NO_CONVERSION;
- }
-
/**
* Returns the value of the argument 'name' (or null if there is none).
* This function is used to associate debugging information to rules created by skylark "macros".
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ListComprehension.java b/src/main/java/com/google/devtools/build/lib/syntax/ListComprehension.java
index bfb777e..567fdaf 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ListComprehension.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ListComprehension.java
@@ -59,7 +59,7 @@
@Override
public Object getResult(Environment env) throws EvalException {
- return env.isSkylark() ? new MutableList(result, env) : result;
+ return new MutableList(result, env);
}
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ListLiteral.java b/src/main/java/com/google/devtools/build/lib/syntax/ListLiteral.java
index cf620bd..13c4d01 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ListLiteral.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ListLiteral.java
@@ -88,11 +88,7 @@
}
result.add(expr.eval(env));
}
- if (env.isSkylark()) {
- return isTuple() ? Tuple.copyOf(result) : new MutableList(result, env);
- } else {
- return EvalUtils.makeSequence(result, isTuple());
- }
+ return isTuple() ? Tuple.copyOf(result) : new MutableList(result, env);
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java b/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
index e8e8767..62cb8df 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
@@ -14,8 +14,8 @@
package com.google.devtools.build.lib.syntax;
-import com.google.common.base.Joiner;
import com.google.common.base.CharMatcher;
+import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
@@ -27,12 +27,9 @@
import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
import com.google.devtools.build.lib.syntax.SkylarkList.Tuple;
import com.google.devtools.build.lib.syntax.SkylarkSignature.Param;
-import com.google.devtools.build.lib.syntax.SkylarkSignatureProcessor.HackHackEitherList;
import com.google.devtools.build.lib.syntax.Type.ConversionException;
import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.LinkedList;
@@ -52,16 +49,6 @@
private MethodLibrary() {}
- // TODO(bazel-team):
- // the Build language and Skylark currently have different list types:
- // the Build language uses plain java List (usually ArrayList) which is mutable and accepts
- // any argument, whereas Skylark uses SkylarkList which is immutable and accepts only
- // arguments of the same kind. Some methods below use HackHackEitherList as a magic marker
- // to indicate that either kind of lists is used depending on the evaluation context.
- // It might be a good idea to either have separate methods for the two languages where it matters,
- // or to unify the two languages so they use the same data structure (which might require
- // updating all existing clients).
-
// Convert string index in the same way Python does.
// If index is negative, starts from the end.
// If index is outside bounds, it is restricted to the valid range.
@@ -114,11 +101,10 @@
+ "<pre class=\"language-python\">\"|\".join([\"a\", \"b\", \"c\"]) == \"a|b|c\"</pre>",
mandatoryPositionals = {
@Param(name = "self", type = String.class, doc = "This string, a separator."),
- @Param(name = "elements", type = HackHackEitherList.class, doc = "The objects to join.")})
+ @Param(name = "elements", type = SkylarkList.class, doc = "The objects to join.")})
private static BuiltinFunction join = new BuiltinFunction("join") {
- public String invoke(String self, Object elements) throws ConversionException {
- List<?> seq = Type.OBJECT_LIST.convert(elements, "'join.elements' argument");
- return Joiner.on(self).join(seq);
+ public String invoke(String self, SkylarkList elements) throws ConversionException {
+ return Joiner.on(self).join(elements);
}
};
@@ -281,7 +267,7 @@
};
@SkylarkSignature(name = "split", objectType = StringModule.class,
- returnType = HackHackEitherList.class,
+ returnType = MutableList.class,
doc = "Returns a list of all the words in the string, using <code>sep</code> "
+ "as the separator, optionally limiting the number of splits to <code>maxsplit</code>.",
mandatoryPositionals = {
@@ -293,18 +279,18 @@
useEnvironment = true,
useLocation = true)
private static BuiltinFunction split = new BuiltinFunction("split") {
- public Object invoke(String self, String sep, Object maxSplitO, Location loc,
+ public MutableList invoke(String self, String sep, Object maxSplitO, Location loc,
Environment env) throws ConversionException, EvalException {
int maxSplit = Type.INTEGER.convertOptional(
maxSplitO, "'split' argument of 'split'", /*label*/null, -2);
// + 1 because the last result is the remainder, and default of -2 so that after +1 it's -1
String[] ss = Pattern.compile(sep, Pattern.LITERAL).split(self, maxSplit + 1);
- return convert(Arrays.<String>asList(ss), env);
+ return MutableList.of(env, (Object[]) ss);
}
};
@SkylarkSignature(name = "rsplit", objectType = StringModule.class,
- returnType = HackHackEitherList.class,
+ returnType = MutableList.class,
doc = "Returns a list of all the words in the string, using <code>sep</code> "
+ "as the separator, optionally limiting the number of splits to <code>maxsplit</code>. "
+ "Except for splitting from the right, this method behaves like split().",
@@ -318,19 +304,18 @@
useLocation = true)
private static BuiltinFunction rsplit = new BuiltinFunction("rsplit") {
@SuppressWarnings("unused")
- public Object invoke(String self, String sep, Object maxSplitO, Location loc, Environment env)
+ public MutableList invoke(
+ String self, String sep, Object maxSplitO, Location loc, Environment env)
throws ConversionException, EvalException {
int maxSplit =
Type.INTEGER.convertOptional(maxSplitO, "'split' argument of 'split'", null, -1);
List<String> result;
try {
- result = stringRSplit(self, sep, maxSplit);
+ return stringRSplit(self, sep, maxSplit, env);
} catch (IllegalArgumentException ex) {
throw new EvalException(loc, ex);
}
-
- return convert(result, env);
}
};
@@ -346,7 +331,8 @@
* @return A list of words
* @throws IllegalArgumentException
*/
- private static List<String> stringRSplit(String input, String separator, int maxSplits)
+ private static MutableList stringRSplit(
+ String input, String separator, int maxSplits, Environment env)
throws IllegalArgumentException {
if (separator.isEmpty()) {
throw new IllegalArgumentException("Empty separator");
@@ -378,11 +364,11 @@
result.addFirst(input.substring(0, remainingLength));
}
- return result;
+ return new MutableList(result, env);
}
@SkylarkSignature(name = "partition", objectType = StringModule.class,
- returnType = HackHackEitherList.class,
+ returnType = MutableList.class,
doc = "Splits the input string at the first occurrence of the separator "
+ "<code>sep</code> and returns the resulting partition as a three-element "
+ "list of the form [substring_before, separator, substring_after].",
@@ -395,14 +381,14 @@
useLocation = true)
private static BuiltinFunction partition = new BuiltinFunction("partition") {
@SuppressWarnings("unused")
- public Object invoke(String self, String sep, Location loc, Environment env)
+ public MutableList invoke(String self, String sep, Location loc, Environment env)
throws EvalException {
return partitionWrapper(self, sep, true, env, loc);
}
};
@SkylarkSignature(name = "rpartition", objectType = StringModule.class,
- returnType = HackHackEitherList.class,
+ returnType = MutableList.class,
doc = "Splits the input string at the last occurrence of the separator "
+ "<code>sep</code> and returns the resulting partition as a three-element "
+ "list of the form [substring_before, separator, substring_after].",
@@ -415,7 +401,7 @@
useLocation = true)
private static BuiltinFunction rpartition = new BuiltinFunction("rpartition") {
@SuppressWarnings("unused")
- public Object invoke(String self, String sep, Location loc, Environment env)
+ public MutableList invoke(String self, String sep, Location loc, Environment env)
throws EvalException {
return partitionWrapper(self, sep, false, env, loc);
}
@@ -433,10 +419,10 @@
* @param loc The location that is used for potential exceptions
* @return A list with three elements
*/
- private static Object partitionWrapper(String self, String separator, boolean forward,
+ private static MutableList partitionWrapper(String self, String separator, boolean forward,
Environment env, Location loc) throws EvalException {
try {
- return convert(stringPartition(self, separator, forward), env);
+ return new MutableList(stringPartition(self, separator, forward), env);
} catch (IllegalArgumentException ex) {
throw new EvalException(loc, ex);
}
@@ -445,7 +431,7 @@
/**
* Splits the input string at the {first|last} occurrence of the given separator
* and returns the resulting partition as a three-tuple of Strings, contained
- * in a {@code List}.
+ * in a {@code MutableList}.
*
* <p>If the input string does not contain the separator, the tuple will
* consist of the original input string and two empty strings.
@@ -742,16 +728,16 @@
@Param(name = "self", type = String.class, doc = "This string."),
},
extraPositionals = {
- @Param(name = "args", type = HackHackEitherList.class, defaultValue = "[]",
+ @Param(name = "args", type = SkylarkList.class, defaultValue = "()",
doc = "List of arguments"),
},
extraKeywords = {@Param(name = "kwargs", doc = "Dictionary of arguments")},
useLocation = true)
private static BuiltinFunction format = new BuiltinFunction("format") {
@SuppressWarnings("unused")
- public String invoke(String self, Object args, Map<String, Object> kwargs, Location loc)
+ public String invoke(String self, SkylarkList args, Map<String, Object> kwargs, Location loc)
throws ConversionException, EvalException {
- return new FormatParser(loc).format(self, Type.OBJECT_LIST.convert(args, "format"), kwargs);
+ return new FormatParser(loc).format(self, args.getList(), kwargs);
}
};
@@ -790,20 +776,6 @@
}
};
- @SkylarkSignature(name = "$slice", objectType = List.class,
- documented = false,
- mandatoryPositionals = {
- @Param(name = "self", type = List.class, doc = "This list or tuple."),
- @Param(name = "start", type = Integer.class, doc = "start position of the slice."),
- @Param(name = "end", type = Integer.class, doc = "end position of the slice.")},
- doc = "x[<code>start</code>:<code>end</code>] returns a slice or a list slice.")
- private static BuiltinFunction listSlice = new BuiltinFunction("$slice") {
- public Object invoke(List<Object> self, Integer left, Integer right)
- throws EvalException, ConversionException {
- return sliceList(self, left, right);
- }
- };
-
@SkylarkSignature(name = "$slice", objectType = MutableList.class, returnType = MutableList.class,
documented = false,
mandatoryPositionals = {
@@ -813,11 +785,11 @@
doc = "x[<code>start</code>:<code>end</code>] returns a slice or a list slice.",
useEnvironment = true)
private static BuiltinFunction mutableListSlice = new BuiltinFunction("$slice") {
- public MutableList invoke(MutableList self, Integer left, Integer right,
- Environment env) throws EvalException, ConversionException {
- return new MutableList(sliceList(self.getList(), left, right), env);
- }
- };
+ public MutableList invoke(MutableList self, Integer left, Integer right,
+ Environment env) throws EvalException, ConversionException {
+ return new MutableList(sliceList(self.getList(), left, right), env);
+ }
+ };
@SkylarkSignature(name = "$slice", objectType = Tuple.class, returnType = Tuple.class,
documented = false,
@@ -846,7 +818,7 @@
// supported list methods
@SkylarkSignature(
name = "sorted",
- returnType = HackHackEitherList.class,
+ returnType = MutableList.class,
doc =
"Sort a collection. Elements are sorted first by their type, "
+ "then by their value (in ascending order).",
@@ -856,46 +828,19 @@
)
private static BuiltinFunction sorted =
new BuiltinFunction("sorted") {
- public Object invoke(Object self, Location loc, Environment env)
+ public MutableList invoke(Object self, Location loc, Environment env)
throws EvalException, ConversionException {
try {
- Collection<?> col =
- Ordering.from(EvalUtils.SKYLARK_COMPARATOR)
- .sortedCopy(EvalUtils.toCollection(self, loc));
- return convert(col, env);
+ return new MutableList(
+ Ordering.from(EvalUtils.SKYLARK_COMPARATOR).sortedCopy(
+ EvalUtils.toCollection(self, loc)),
+ env);
} catch (EvalUtils.ComparisonException e) {
throw new EvalException(loc, e);
}
}
};
- // This function has a SkylarkSignature but is only used by the Build language, not Skylark.
- @SkylarkSignature(
- name = "append",
- objectType = List.class,
- returnType = Runtime.NoneType.class,
- documented = false,
- doc = "Adds an item to the end of the list.",
- mandatoryPositionals = {
- // we use List rather than SkylarkList because this is actually for use *outside* Skylark
- @Param(name = "self", type = List.class, doc = "This list."),
- @Param(name = "item", type = Object.class, doc = "Item to add at the end.")
- },
- useLocation = true,
- useEnvironment = true)
- private static BuiltinFunction append =
- new BuiltinFunction("append") {
- public Runtime.NoneType invoke(List<Object> self, Object item,
- Location loc, Environment env) throws EvalException, ConversionException {
- if (EvalUtils.isTuple(self)) {
- throw new EvalException(loc,
- "function 'append' is not defined on object of type 'Tuple'");
- }
- self.add(item);
- return Runtime.NONE;
- }
- };
-
@SkylarkSignature(
name = "append",
objectType = MutableList.class,
@@ -908,7 +853,7 @@
},
useLocation = true,
useEnvironment = true)
- private static BuiltinFunction skylarkAppend =
+ private static BuiltinFunction append =
new BuiltinFunction("append") {
public Runtime.NoneType invoke(MutableList self, Object item,
Location loc, Environment env) throws EvalException, ConversionException {
@@ -917,32 +862,6 @@
}
};
- // This function has a SkylarkSignature but is only used by the Build language, not Skylark.
- @SkylarkSignature(
- name = "extend",
- objectType = List.class,
- returnType = Runtime.NoneType.class,
- documented = false,
- doc = "Adds all items to the end of the list.",
- mandatoryPositionals = {
- // we use List rather than SkylarkList because this is actually for use *outside* Skylark
- @Param(name = "self", type = List.class, doc = "This list."),
- @Param(name = "items", type = List.class, doc = "Items to add at the end.")},
- useLocation = true,
- useEnvironment = true)
- private static BuiltinFunction skylarkExtend =
- new BuiltinFunction("extend") {
- public Runtime.NoneType invoke(List<Object> self, List<Object> items,
- Location loc, Environment env) throws EvalException, ConversionException {
- if (EvalUtils.isTuple(self)) {
- throw new EvalException(loc,
- "function 'extend' is not defined on object of type 'Tuple'");
- }
- self.addAll(items);
- return Runtime.NONE;
- }
- };
-
@SkylarkSignature(
name = "extend",
objectType = MutableList.class,
@@ -970,7 +889,7 @@
@Param(name = "self", type = Map.class, doc = "This object."),
@Param(name = "key", type = Object.class, doc = "The index or key to access.")},
useLocation = true)
- private static BuiltinFunction indexOperator = new BuiltinFunction("$index") {
+ private static BuiltinFunction dictIndexOperator = new BuiltinFunction("$index") {
public Object invoke(Map<?, ?> self, Object key,
Location loc) throws EvalException, ConversionException {
if (!self.containsKey(key)) {
@@ -987,7 +906,7 @@
@Param(name = "self", type = MutableList.class, doc = "This list."),
@Param(name = "key", type = Object.class, doc = "The index or key to access.")},
useLocation = true)
- private static BuiltinFunction mutableListIndexOperator = new BuiltinFunction("$index") {
+ private static BuiltinFunction listIndexOperator = new BuiltinFunction("$index") {
public Object invoke(MutableList self, Object key,
Location loc) throws EvalException, ConversionException {
if (self.isEmpty()) {
@@ -1000,7 +919,7 @@
// tuple access operator
@SkylarkSignature(name = "$index", documented = false, objectType = Tuple.class,
- doc = "Returns the nth element of a list.",
+ doc = "Returns the nth element of a tuple.",
mandatoryPositionals = {
@Param(name = "self", type = Tuple.class, doc = "This tuple."),
@Param(name = "key", type = Object.class, doc = "The index or key to access.")},
@@ -1009,31 +928,13 @@
public Object invoke(Tuple self, Object key,
Location loc) throws EvalException, ConversionException {
if (self.isEmpty()) {
- throw new EvalException(loc, "Tuple is empty");
+ throw new EvalException(loc, "tuple is empty");
}
int index = getListIndex(key, self.size(), loc);
return self.getList().get(index);
}
};
- // list access operator
- @SkylarkSignature(name = "$index", documented = false, objectType = List.class,
- doc = "Returns the nth element of a list.",
- mandatoryPositionals = {
- @Param(name = "self", type = List.class, doc = "This object."),
- @Param(name = "key", type = Object.class, doc = "The index or key to access.")},
- useLocation = true)
- private static BuiltinFunction listIndexOperator = new BuiltinFunction("$index") {
- public Object invoke(List<?> self, Object key,
- Location loc) throws EvalException, ConversionException {
- if (self.isEmpty()) {
- throw new EvalException(loc, "List is empty");
- }
- int index = getListIndex(key, self.size(), loc);
- return self.get(index);
- }
- };
-
@SkylarkSignature(name = "$index", documented = false, objectType = String.class,
doc = "Returns the nth element of a string.",
mandatoryPositionals = {
@@ -1049,58 +950,57 @@
};
@SkylarkSignature(name = "values", objectType = Map.class,
- returnType = HackHackEitherList.class,
+ returnType = MutableList.class,
doc = "Returns the list of values. Dictionaries are always sorted by their keys:"
+ "<pre class=\"language-python\">"
+ "{2: \"a\", 4: \"b\", 1: \"c\"}.values() == [\"c\", \"a\", \"b\"]</pre>\n",
mandatoryPositionals = {@Param(name = "self", type = Map.class, doc = "This dict.")},
- useLocation = true, useEnvironment = true)
+ useEnvironment = true)
private static BuiltinFunction values = new BuiltinFunction("values") {
- public Object invoke(Map<?, ?> self,
- Location loc, Environment env) throws EvalException, ConversionException {
+ public MutableList invoke(Map<?, ?> self,
+ Environment env) throws EvalException, ConversionException {
// Use a TreeMap to ensure consistent ordering.
Map<?, ?> dict = new TreeMap<>(self);
- return convert(dict.values(), env);
+ return new MutableList(dict.values(), env);
}
};
@SkylarkSignature(name = "items", objectType = Map.class,
- returnType = HackHackEitherList.class,
+ returnType = MutableList.class,
doc = "Returns the list of key-value tuples. Dictionaries are always sorted by their keys:"
+ "<pre class=\"language-python\">"
+ "{2: \"a\", 4: \"b\", 1: \"c\"}.items() == [(1, \"c\"), (2, \"a\"), (4, \"b\")]"
+ "</pre>\n",
mandatoryPositionals = {
@Param(name = "self", type = Map.class, doc = "This dict.")},
- useLocation = true, useEnvironment = true)
+ useEnvironment = true)
private static BuiltinFunction items = new BuiltinFunction("items") {
- public Object invoke(Map<?, ?> self,
- Location loc, Environment env) throws EvalException, ConversionException {
+ public MutableList invoke(Map<?, ?> self,
+ Environment env) throws EvalException, ConversionException {
// Use a TreeMap to ensure consistent ordering.
Map<?, ?> dict = new TreeMap<>(self);
List<Object> list = Lists.newArrayListWithCapacity(dict.size());
for (Map.Entry<?, ?> entries : dict.entrySet()) {
- List<?> item = ImmutableList.of(entries.getKey(), entries.getValue());
- list.add(env.isCallerSkylark() ? Tuple.copyOf(item) : item);
+ list.add(Tuple.of(entries.getKey(), entries.getValue()));
}
- return convert(list, env);
+ return new MutableList(list, env);
}
};
@SkylarkSignature(name = "keys", objectType = Map.class,
- returnType = HackHackEitherList.class,
+ returnType = MutableList.class,
doc = "Returns the list of keys. Dictionaries are always sorted by their keys:"
+ "<pre class=\"language-python\">{2: \"a\", 4: \"b\", 1: \"c\"}.keys() == [1, 2, 4]"
+ "</pre>\n",
mandatoryPositionals = {
@Param(name = "self", type = Map.class, doc = "This dict.")},
- useLocation = true, useEnvironment = true)
+ useEnvironment = true)
// Skylark will only call this on a dict; and
// allowed keys are all Comparable... if not mutually, it's OK to get a runtime exception.
private static BuiltinFunction keys = new BuiltinFunction("keys") {
- public Object invoke(Map<Comparable<?>, ?> dict,
- Location loc, Environment env) throws EvalException {
- return convert(Ordering.natural().sortedCopy(dict.keySet()), env);
+ public MutableList invoke(Map<Comparable<?>, ?> dict,
+ Environment env) throws EvalException {
+ return new MutableList(Ordering.natural().sortedCopy(dict.keySet()), env);
}
};
@@ -1123,17 +1023,6 @@
}
};
- // TODO(bazel-team): Use the same type for both Skylark and BUILD files.
- @SuppressWarnings("unchecked")
- private static Iterable<Object> convert(Collection<?> list, Environment env)
- throws EvalException {
- if (env.isCallerSkylark()) {
- return new MutableList(list, env);
- } else {
- return Lists.newArrayList(list);
- }
- }
-
// unary minus
@SkylarkSignature(name = "-", returnType = Integer.class,
documented = false,
@@ -1350,29 +1239,28 @@
}
};
- @SkylarkSignature(name = "enumerate", returnType = HackHackEitherList.class,
+ @SkylarkSignature(name = "enumerate", returnType = MutableList.class,
doc = "Returns a list of pairs (two-element tuples), with the index (int) and the item from"
+ " the input list.\n<pre class=\"language-python\">"
+ "enumerate([24, 21, 84]) == [(0, 24), (1, 21), (2, 84)]</pre>\n",
mandatoryPositionals = {
- @Param(name = "list", type = HackHackEitherList.class, doc = "input list")
+ @Param(name = "list", type = SkylarkList.class, doc = "input list")
},
- useLocation = true,
useEnvironment = true)
private static BuiltinFunction enumerate = new BuiltinFunction("enumerate") {
- public Object invoke(Object input, Location loc, Environment env)
+ public MutableList invoke(SkylarkList input, Environment env)
throws EvalException, ConversionException {
int count = 0;
List<SkylarkList> result = Lists.newArrayList();
- for (Object obj : Type.OBJECT_LIST.convert(input, "input")) {
+ for (Object obj : input) {
result.add(Tuple.of(count, obj));
count++;
}
- return convert(result, env);
+ return new MutableList(result, env);
}
};
- @SkylarkSignature(name = "range", returnType = HackHackEitherList.class,
+ @SkylarkSignature(name = "range", returnType = MutableList.class,
doc = "Creates a list where items go from <code>start</code> to <code>stop</code>, using a "
+ "<code>step</code> increment. If a single argument is provided, items will "
+ "range from 0 to that element."
@@ -1393,8 +1281,8 @@
useLocation = true,
useEnvironment = true)
private static final BuiltinFunction range = new BuiltinFunction("range") {
- public Object invoke(Integer startOrStop, Object stopOrNone, Integer step, Location loc,
- Environment env)
+ public MutableList invoke(Integer startOrStop, Object stopOrNone, Integer step,
+ Location loc, Environment env)
throws EvalException, ConversionException {
int start;
int stop;
@@ -1420,7 +1308,7 @@
start += step;
}
}
- return convert(result, env);
+ return new MutableList(result, env);
}
};
@@ -1499,13 +1387,13 @@
}
};
- @SkylarkSignature(name = "dir", returnType = SkylarkList.class,
+ @SkylarkSignature(name = "dir", returnType = MutableList.class,
doc = "Returns a list strings: the names of the attributes and "
+ "methods of the parameter object.",
mandatoryPositionals = {@Param(name = "x", doc = "The object to check.")},
useLocation = true, useEnvironment = true)
private static final BuiltinFunction dir = new BuiltinFunction("dir") {
- public SkylarkList invoke(Object object,
+ public MutableList invoke(Object object,
Location loc, Environment env) throws EvalException, ConversionException {
// Order the fields alphabetically.
Set<String> fields = new TreeSet<>();
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkList.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkList.java
index 6f597cf..67b7db6 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkList.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkList.java
@@ -14,8 +14,8 @@
package com.google.devtools.build.lib.syntax;
-import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.syntax.Mutability.Freezable;
@@ -47,6 +47,15 @@
public abstract ImmutableList<Object> getImmutableList();
/**
+ * Returns a List object with the current underlying contents of this SkylarkList.
+ * This object must not be modified, but may not be an ImmutableList.
+ * It may notably be a GlobList, where appropriate.
+ */
+ // TODO(bazel-team): move GlobList out of Skylark, into an extension,
+ // and maybe get rid of this method?
+ public abstract List<Object> getContents();
+
+ /**
* Returns true if this list is a tuple.
*/
public abstract boolean isTuple();
@@ -99,17 +108,61 @@
return getClass().hashCode() + 31 * getList().hashCode();
}
+ /**
+ * Cast a {@code List<?>} to a {@code List<T>} after checking its current contents.
+ * @param list the List to cast
+ * @param type the expected class of elements
+ * @param description a description of the argument being converted, or null, for debugging
+ */
@SuppressWarnings("unchecked")
- public <T> Iterable<T> to(Class<T> type) {
- for (Object value : getList()) {
- Preconditions.checkArgument(
- type.isInstance(value),
- Printer.formattable("list element %r is not of type %r", value, type));
+ public static <TYPE> List<TYPE> castList(
+ List<?> list, Class<TYPE> type, @Nullable String description)
+ throws EvalException {
+ for (Object value : list) {
+ if (!type.isInstance(value)) {
+ throw new EvalException(null,
+ Printer.format("Illegal argument: expected type %r %sbut got type %s instead",
+ type,
+ description == null ? "" : String.format("for '%s' element ", description),
+ EvalUtils.getDataTypeName(value)));
+ }
}
- return (Iterable<T>) this;
+ return (List<TYPE>) list;
}
/**
+ * Cast a SkylarkList to a {@code List<T>} after checking its current contents.
+ * Treat None as meaning the empty List.
+ * @param obj the Object to cast. null and None are treated as an empty list.
+ * @param type the expected class of elements
+ * @param description a description of the argument being converted, or null, for debugging
+ */
+ public static <TYPE> List<TYPE> castSkylarkListOrNoneToList(
+ Object obj, Class<TYPE> type, @Nullable String description)
+ throws EvalException {
+ if (EvalUtils.isNullOrNone(obj)) {
+ return ImmutableList.of();
+ }
+ if (obj instanceof SkylarkList) {
+ return ((SkylarkList) obj).getContents(type, description);
+ }
+ throw new EvalException(null,
+ Printer.format("Illegal argument: %s is not of expected type list or NoneType",
+ description == null ? Printer.repr(obj) : String.format("'%s'", description)));
+ }
+
+ /**
+ * Cast the SkylarkList object into a List of the given type.
+ * @param type the expected class of elements
+ * @param description a description of the argument being converted, or null, for debugging
+ */
+ public <TYPE> List<TYPE> getContents(Class<TYPE> type, @Nullable String description)
+ throws EvalException {
+ return castList(getContents(), type, description);
+ }
+
+
+ /**
* A class for mutable lists.
*/
@SkylarkModule(name = "list",
@@ -126,6 +179,12 @@
private final ArrayList<Object> contents = new ArrayList<>();
+ // Treat GlobList specially: external code depends on it.
+ // TODO(bazel-team): make data structures *and binary operators* extensible
+ // (via e.g. interface classes for each binary operator) so that GlobList
+ // can be implemented outside of the core of Skylark.
+ @Nullable private GlobList<?> globList;
+
private final Mutability mutability;
/**
@@ -137,6 +196,9 @@
MutableList(Iterable<?> contents, Mutability mutability) {
super();
addAll(contents);
+ if (contents instanceof GlobList<?>) {
+ globList = (GlobList<?>) contents;
+ }
this.mutability = mutability;
}
@@ -160,6 +222,16 @@
}
/**
+ * Builds a Skylark list (actually immutable) from a variable number of arguments.
+ * @param env an Environment from which to inherit Mutability, or null for immutable
+ * @param contents the contents of the list
+ * @return a Skylark list containing the specified arguments as elements.
+ */
+ public static MutableList of(Environment env, Object... contents) {
+ return new MutableList(ImmutableList.copyOf(contents), env);
+ }
+
+ /**
* Adds one element at the end of the MutableList.
* @param element the element to add
*/
@@ -183,6 +255,48 @@
} catch (MutabilityException ex) {
throw new EvalException(loc, ex);
}
+ globList = null; // If you're going to mutate it, invalidate the underlying GlobList.
+ }
+
+ @Nullable public GlobList<?> getGlobList() {
+ return globList;
+ }
+
+ /**
+ * @return the GlobList if there is one, otherwise an Immutable copy of the regular contents.
+ */
+ @Override
+ @SuppressWarnings("unchecked")
+ public List<Object> getContents() {
+ if (globList != null) {
+ return (List<Object>) (List<?>) globList;
+ }
+ return getImmutableList();
+ }
+
+ /**
+ * @return the GlobList if there is one, otherwise the regular contents.
+ */
+ private List<?> getContentsUnsafe() {
+ if (globList != null) {
+ return globList;
+ }
+ return contents;
+ }
+
+ /**
+ * Concatenate two MutableList
+ * @param left the start of the new list
+ * @param right the end of the new list
+ * @param env the Environment in which to create a new list
+ * @return a new MutableList
+ */
+ public static MutableList concat(MutableList left, MutableList right, Environment env) {
+ if (left.getGlobList() == null && right.getGlobList() == null) {
+ return new MutableList(Iterables.concat(left, right), env);
+ }
+ return new MutableList(GlobList.concat(
+ left.getContentsUnsafe(), right.getContentsUnsafe()), env);
}
/**
@@ -304,6 +418,11 @@
}
@Override
+ public List<Object> getContents() {
+ return contents;
+ }
+
+ @Override
public boolean isTuple() {
return true;
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignatureProcessor.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignatureProcessor.java
index 6b96dac..44f9783 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignatureProcessor.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignatureProcessor.java
@@ -115,18 +115,6 @@
}
/**
- * Fake class to use in SkylarkSignature annotations to indicate that either List or SkylarkList
- * may be used, depending on whether the Build language or Skylark is being evaluated.
- */
- // TODO(bazel-team): either make SkylarkList a subclass of List (mutable or immutable throwing
- // runtime exceptions), or have the Build language use immutable SkylarkList, but either way,
- // do away with this hack.
- public static class HackHackEitherList {
- private HackHackEitherList() { }
- }
-
-
- /**
* Configures the parameter of this Skylark function using the annotation.
*/
// TODO(bazel-team): Maybe have the annotation be a string representing the
@@ -146,18 +134,8 @@
return new Parameter.Star<>(null);
}
if (param.type() != Object.class) {
- if (param.type() == HackHackEitherList.class) {
- // NB: a List in the annotation actually indicates either a List or a SkylarkList
- // and we trust the BuiltinFunction to do the enforcement.
- // For automatic document generation purpose, we lie and just say it's a list;
- // hopefully user code should never be exposed to the java List case
- officialType = SkylarkType.of(SkylarkList.class, param.generic1());
- enforcedType = SkylarkType.Union.of(
- SkylarkType.of(List.class),
- SkylarkType.of(SkylarkList.class, param.generic1()));
- Preconditions.checkArgument(enforcedType instanceof SkylarkType.Union);
- } else if (param.generic1() != Object.class) {
- // Otherwise, we will enforce the proper parametric type for Skylark list and set objects
+ if (param.generic1() != Object.class) {
+ // Enforce the proper parametric type for Skylark list and set objects
officialType = SkylarkType.of(param.type(), param.generic1());
enforcedType = officialType;
} else {
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkType.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkType.java
index f79b129..190dfb1 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkType.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkType.java
@@ -644,34 +644,6 @@
}
}
- /** Cast a SkylarkList object into an Iterable of the given type. Treat null as empty List */
- public static <TYPE> Iterable<TYPE> castList(Object obj, final Class<TYPE> type) {
- if (obj == null) {
- return ImmutableList.of();
- }
- return ((SkylarkList) obj).to(type);
- }
-
- /** Cast a List or SkylarkList object into an Iterable of the given type. null means empty List */
- public static <TYPE> Iterable<TYPE> castList(
- Object obj, final Class<TYPE> type, final String what) throws EvalException {
- if (obj == null) {
- return ImmutableList.of();
- }
- List<TYPE> results = new ArrayList<>();
- for (Object object : com.google.devtools.build.lib.syntax.Type.LIST.convert(obj, what)) {
- try {
- results.add(type.cast(object));
- } catch (ClassCastException e) {
- throw new EvalException(null, String.format(
- "Illegal argument: expected %s type for '%s' but got %s instead",
- EvalUtils.getDataTypeNameFromClass(type), what,
- EvalUtils.getDataTypeName(object)));
- }
- }
- return results;
- }
-
/**
* Cast a Map object into an Iterable of Map entries of the given key, value types.
* @param obj the Map object, where null designates an empty map
@@ -749,17 +721,4 @@
}
return object;
}
-
- /**
- * Converts object from a Skylark-compatible wrapper type to its original type.
- */
- public static Object convertFromSkylark(Object value) {
- if (value instanceof MutableList) {
- return new ArrayList<>(((MutableList) value).getList());
- } else if (value instanceof Tuple) {
- return ((Tuple) value).getImmutableList();
- } else {
- return value;
- }
- }
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Type.java b/src/main/java/com/google/devtools/build/lib/syntax/Type.java
index e876781..a3d348c 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Type.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Type.java
@@ -19,6 +19,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
+import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
import com.google.devtools.build.lib.util.LoggingUtil;
import com.google.devtools.build.lib.util.StringCanonicalizer;
@@ -558,11 +559,20 @@
}
++index;
}
+ // We preserve GlobList-s so they can make it to attributes;
+ // some external code relies on attributes preserving this information.
+ // TODO(bazel-team): somehow make Skylark extensible enough that
+ // GlobList support can be wholly moved out of Skylark into an extension.
if (x instanceof GlobList<?>) {
return new GlobList<>(((GlobList<?>) x).getCriteria(), result);
- } else {
- return result;
}
+ if (x instanceof MutableList) {
+ GlobList<?> globList = ((MutableList) x).getGlobList();
+ if (globList != null) {
+ return new GlobList<>(globList.getCriteria(), result);
+ }
+ }
+ return result;
}
@Override
