diff options
Diffstat (limited to 'src/main/java/com/google/devtools/build')
9 files changed, 130 insertions, 58 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSet.java b/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSet.java index 39265a6875..9f5b9d97c5 100644 --- a/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSet.java +++ b/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSet.java @@ -151,7 +151,7 @@ public final class NestedSet<E> implements Iterable<E> { } /** - * Returns true if the set is empty. + * Returns true if the set is empty. Runs in O(1) time (i.e. does not flatten the set). */ public boolean isEmpty() { return children == EMPTY_CHILDREN; diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkFileType.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkFileType.java index 6b31fd8761..b077b013cf 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkFileType.java +++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkFileType.java @@ -19,6 +19,8 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable; import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory; +import com.google.devtools.build.lib.syntax.EvalException; +import com.google.devtools.build.lib.syntax.EvalUtils; import com.google.devtools.build.lib.util.FileType; import com.google.devtools.build.lib.util.FileTypeSet; @@ -53,8 +55,16 @@ public class SkylarkFileType { + "<a href=\"File.html\"><code>File</code></a>s that match the FileType. The parameter " + "must be a <a href=\"set.html\"><code>set</code></a> or a " + "<a href=\"list.html\"><code>list</code></a>.") - public List<Artifact> filter(Iterable<Artifact> files) { - return ImmutableList.copyOf(FileType.filter(files, fileType)); + // toIterablesStrict() will ensure the parameter is a SkylarkNestedSet or a java Iterable + // (including SkylarkList). If it fails, the error location information will be inserted by the + // Skylark interface framework. If there's a dynamic type error on a non-Artifact element, the + // error will also be handled by the Skylark interface framework. + @SuppressWarnings("unchecked") + public List<Artifact> filter(Object filesUnchecked) throws EvalException { + return ImmutableList.copyOf( + FileType.filter( + (Iterable<Artifact>) EvalUtils.toIterableStrict(filesUnchecked, null), + fileType)); } @VisibleForTesting diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java index f200cf5406..c58334d450 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java @@ -58,6 +58,7 @@ import com.google.devtools.build.lib.vfs.PathFragment; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.UUID; @@ -650,30 +651,43 @@ public class SkylarkRuleImplementationFunctions { }; + /** + * Ensures the given {@link Map} has keys that have {@link Label} type and values that have either + * {@link Iterable} or {@link SkylarkNestedSet} type, and raises {@link EvalException} otherwise. + * Returns a corresponding map where any sets are replaced by iterables. + */ // TODO(bazel-team): find a better way to typecheck this argument. @SuppressWarnings("unchecked") private static Map<Label, Iterable<Artifact>> checkLabelDict( Map<?, ?> labelDict, Location loc) throws EvalException { + Map<Label, Iterable<Artifact>> convertedMap = new HashMap<Label, Iterable<Artifact>>(); for (Map.Entry<?, ?> entry : labelDict.entrySet()) { Object key = entry.getKey(); if (!(key instanceof Label)) { throw new EvalException( loc, Printer.format("invalid key %r in 'label_dict'", key)); } + ImmutableList.Builder<Artifact> files = ImmutableList.builder(); Object val = entry.getValue(); - if (!(val instanceof Iterable)) { + Iterable<?> valIter; + try { + valIter = EvalUtils.toIterableStrict(val, loc); + } catch (EvalException ex) { + // EvalException is thrown only if the type is wrong. throw new EvalException( - loc, Printer.format("invalid value %r in 'label_dict'", val)); + loc, Printer.format("invalid value %r in 'label_dict': " + ex, val)); } - for (Object file : (Iterable) val) { + for (Object file : valIter) { if (!(file instanceof Artifact)) { throw new EvalException( loc, Printer.format("invalid value %r in 'label_dict'", val)); } + files.add((Artifact) file); } + convertedMap.put((Label) key, files.build()); } - return (Map<Label, Iterable<Artifact>>) labelDict; + return convertedMap; } /** suffix of script to be used in case the command is too long to fit on a single line */ @@ -682,12 +696,11 @@ public class SkylarkRuleImplementationFunctions { @SkylarkSignature( name = "resolve_command", doc = - "Experimental." - + "Returns a tuple (inputs, command, input_manifests) of the list of resolved inputs, " - + "the argv list for the resolved command, and " - + "the dict mapping locations to runfiles required to run the command, " - + "all of them suitable for passing as the same-named arguments of the ctx.action " - + "method.", + "<i>(Experimental)</i> " + + "Returns a tuple <code>(inputs, command, input_manifests)</code> of the list of " + + "resolved inputs, the argv list for the resolved command, and the dict mapping " + + "locations to runfiles required to run the command, all of them suitable for passing " + + "as the same-named arguments of the <code>ctx.action</code> method.", objectType = SkylarkRuleContext.class, returnType = Tuple.class, parameters = { diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProviderSkylarkConverters.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProviderSkylarkConverters.java index 7e3b443119..ee8a37c35f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProviderSkylarkConverters.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProviderSkylarkConverters.java @@ -94,7 +94,7 @@ public class ObjcProviderSkylarkConverters { @Override public Iterable<?> valueForJava(Key<?> javaKey, Object skylarkValue) { validateTypes(skylarkValue, javaKey.getType(), javaKey.getSkylarkKeyName()); - return (SkylarkNestedSet) skylarkValue; + return ((SkylarkNestedSet) skylarkValue).toCollection(); } } @@ -118,7 +118,7 @@ public class ObjcProviderSkylarkConverters { public Iterable<?> valueForJava(Key<?> javaKey, Object skylarkValue) { validateTypes(skylarkValue, String.class, javaKey.getSkylarkKeyName()); NestedSetBuilder<PathFragment> result = NestedSetBuilder.stableOrder(); - for (String path : (Iterable<String>) skylarkValue) { + for (String path : ((SkylarkNestedSet) skylarkValue).toCollection(String.class)) { result.add(new PathFragment(path)); } return result.build(); @@ -145,7 +145,7 @@ public class ObjcProviderSkylarkConverters { public Iterable<?> valueForJava(Key<?> javaKey, Object skylarkValue) { validateTypes(skylarkValue, String.class, javaKey.getSkylarkKeyName()); NestedSetBuilder<SdkFramework> result = NestedSetBuilder.stableOrder(); - for (String path : (Iterable<String>) skylarkValue) { + for (String path : ((SkylarkNestedSet) skylarkValue).toCollection(String.class)) { result.add(new SdkFramework(path)); } return result.build(); @@ -178,7 +178,8 @@ public class ObjcProviderSkylarkConverters { public Iterable<?> valueForJava(Key<?> javaKey, Object skylarkValue) { validateTypes(skylarkValue, SkylarkClassObject.class, javaKey.getSkylarkKeyName()); NestedSetBuilder<BundleableFile> result = NestedSetBuilder.stableOrder(); - for (SkylarkClassObject struct : (Iterable<SkylarkClassObject>) skylarkValue) { + for (SkylarkClassObject struct : + ((SkylarkNestedSet) skylarkValue).toCollection(SkylarkClassObject.class)) { Artifact artifact; String path; try { diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java b/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java index 799f850c8a..284483616f 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java @@ -145,7 +145,7 @@ public class BazelLibrary { + "the input depset as well as all additional elements.", parameters = { @Param(name = "input", type = SkylarkNestedSet.class, doc = "The input depset."), - @Param(name = "new_elements", type = Iterable.class, doc = "The elements to be added.") + @Param(name = "new_elements", type = Object.class, doc = "The elements to be added.") }, useLocation = true ) @@ -153,8 +153,11 @@ public class BazelLibrary { new BuiltinFunction("union") { @SuppressWarnings("unused") public SkylarkNestedSet invoke( - SkylarkNestedSet input, Iterable<Object> newElements, Location loc) + SkylarkNestedSet input, Object newElements, Location loc) throws EvalException { + // newElements' type is Object because of the polymorphism on unioning two + // SkylarkNestedSets versus a set and another kind of iterable. + // Can't use EvalUtils#toIterable since that would discard this information. return new SkylarkNestedSet(input, newElements, loc); } }; diff --git a/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java b/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java index 83d2b9f3a4..b3d7bc0ad6 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java @@ -353,6 +353,9 @@ public final class EvalUtils { // This is not as efficient as special casing String in for and dict and list comprehension // statements. However this is a more unified way. return split((String) o); + } else if (o instanceof SkylarkNestedSet) { + // TODO(bazel-team): Add a deprecation warning: don't implicitly flatten depsets. + return ((SkylarkNestedSet) o).toCollection(); } else if (o instanceof Iterable) { return (Iterable<?>) o; } else if (o instanceof Map) { @@ -363,6 +366,35 @@ public final class EvalUtils { } } + /** + * Given an {@link Iterable}, returns it as-is. Given a {@link SkylarkNestedSet}, returns its + * contents as an iterable. Throws {@link EvalException} for any other value. + * + * <p>This is a kludge for the change that made {@code SkylarkNestedSet} not implement {@code + * Iterable}. It is different from {@link #toIterable} in its behavior for strings and other types + * that are not strictly Java-iterable. + * + * @throws EvalException if {@code o} is not an iterable or set + * @deprecated avoid writing APIs that implicitly treat depsets as iterables. It encourages + * unnecessary flattening of depsets. + * + * <p>TODO(bazel-team): Remove this if/when implicit iteration over {@code SkylarkNestedSet} is no + * longer supported. + */ + @Deprecated + public static Iterable<?> toIterableStrict(Object o, Location loc) throws EvalException { + if (o instanceof Iterable) { + return (Iterable<?>) o; + } else if (o instanceof SkylarkNestedSet) { + // TODO(bazel-team): Add a deprecation warning: don't implicitly flatten depsets. + return ((SkylarkNestedSet) o).toCollection(); + } else { + throw new EvalException(loc, + "expected Iterable or depset, but got '" + getDataTypeName(o) + "' (strings and maps " + + "are not allowed here)"); + } + } + public static void lock(Object object, Location loc) { if (object instanceof SkylarkMutable) { ((SkylarkMutable) object).lock(loc); @@ -392,7 +424,10 @@ public final class EvalUtils { } else if (arg instanceof Map) { return ((Map<?, ?>) arg).size(); } else if (arg instanceof SkylarkList) { - return ((SkylarkList) arg).size(); + return ((SkylarkList<?>) arg).size(); + } else if (arg instanceof SkylarkNestedSet) { + // TODO(bazel-team): Add a deprecation warning: don't implicitly flatten depsets. + return ((SkylarkNestedSet) arg).toCollection().size(); } else if (arg instanceof Iterable) { // Iterables.size() checks if arg is a Collection so it's efficient in that sense. return Iterables.size((Iterable<?>) arg); 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 47ca735a2a..3a0a176d28 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 @@ -1024,23 +1024,16 @@ public class MethodLibrary { */ private static Object findExtreme(SkylarkList<?> args, Ordering<Object> maxOrdering, Location loc) throws EvalException { - // Args can either be a list of elements or a list whose first element is a non-empty iterable - // of elements. + // Args can either be a list of items to compare, or a singleton list whose element is an + // iterable of items to compare. In either case, there must be at least one item to compare. try { - return maxOrdering.max(getIterable(args, loc)); + Iterable<?> items = (args.size() == 1) ? EvalUtils.toIterable(args.get(0), loc) : args; + return maxOrdering.max(items); } catch (NoSuchElementException ex) { - throw new EvalException(loc, "expected at least one argument"); + throw new EvalException(loc, "expected at least one item"); } } - /** - * This method returns the first element of the list, if that particular element is an - * Iterable<?>. Otherwise, it will return the entire list. - */ - private static Iterable<?> getIterable(SkylarkList<?> list, Location loc) throws EvalException { - return (list.size() == 1) ? EvalUtils.toIterable(list.get(0), loc) : list; - } - @SkylarkSignature( name = "all", returnType = Boolean.class, diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkNestedSet.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkNestedSet.java index 530efc0994..3edfb99b31 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkNestedSet.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkNestedSet.java @@ -27,7 +27,6 @@ import com.google.devtools.build.lib.syntax.SkylarkList.MutableList; import com.google.devtools.build.lib.util.Preconditions; import java.util.ArrayList; import java.util.Collection; -import java.util.Iterator; import java.util.List; import javax.annotation.Nullable; @@ -101,7 +100,7 @@ import javax.annotation.Nullable; + "nested depsets." ) @Immutable -public final class SkylarkNestedSet implements Iterable<Object>, SkylarkValue, SkylarkQueryable { +public final class SkylarkNestedSet implements SkylarkValue, SkylarkQueryable { private final SkylarkType contentType; private final NestedSet<?> set; @@ -151,7 +150,7 @@ public final class SkylarkNestedSet implements Iterable<Object>, SkylarkValue, S throw new EvalException( loc, String.format( - "cannot add value of type '%s' to a depset", EvalUtils.getDataTypeName(item))); + "cannot union value of type '%s' to a depset", EvalUtils.getDataTypeName(item))); } this.contentType = Preconditions.checkNotNull(contentType, "type cannot be null"); @@ -252,10 +251,23 @@ public final class SkylarkNestedSet implements Iterable<Object>, SkylarkValue, S } } + private void checkHasContentType(Class<?> type) { + // Empty sets should be SkylarkType.TOP anyway. + if (!set.isEmpty()) { + Preconditions.checkArgument( + contentType.canBeCastTo(type), + "Expected a depset of '%s' but got a depset of '%s'", + EvalUtils.getDataTypeNameFromClass(type), contentType); + } + } + /** * Returns the embedded {@link NestedSet}, while asserting that its elements all have the given * type. * + * <p>If you do not specifically need the {@code NestedSet} and you are going to flatten it + * anyway, prefer {@link #toCollection} to make your intent clear. + * * @param type a {@link Class} representing the expected type of the contents * @return the {@code NestedSet}, with the appropriate generic type * @throws IllegalArgumentException if the type does not accurately describe all elements @@ -263,17 +275,32 @@ public final class SkylarkNestedSet implements Iterable<Object>, SkylarkValue, S // The precondition ensures generic type safety. @SuppressWarnings("unchecked") public <T> NestedSet<T> getSet(Class<T> type) { - // Empty sets don't need have to have a type since they don't have items - if (set.isEmpty()) { - return (NestedSet<T>) set; - } - Preconditions.checkArgument( - contentType.canBeCastTo(type), - "Expected a depset of '%s' but got a depset of '%s'", - EvalUtils.getDataTypeNameFromClass(type), contentType); + checkHasContentType(type); return (NestedSet<T>) set; } + /** + * Returns the contents of the set as a {@link Collection}. + */ + public Collection<Object> toCollection() { + // Do not remove <Object>: workaround for Java 7 type inference. + return ImmutableList.<Object>copyOf(set.toCollection()); + } + + /** + * Returns the contents of the set as a {@link Collection}, asserting that the set type is + * compatible with {@code T}. + * + * @param type a {@link Class} representing the expected type of the contents + * @throws IllegalArgumentException if the type does not accurately describe all elements + */ + // The precondition ensures generic type safety. + @SuppressWarnings("unchecked") + public <T> Collection<T> toCollection(Class<T> type) { + checkHasContentType(type); + return (Collection<T>) toCollection(); + } + @SkylarkCallable( name = "to_list", doc = "Returns a frozen list of the elements, without duplicates, in the depset's traversal " @@ -282,18 +309,6 @@ public final class SkylarkNestedSet implements Iterable<Object>, SkylarkValue, S return new MutableList<Object>(set, null); } - // For some reason this cast is unsafe in Java - @SuppressWarnings("unchecked") - @Override - public Iterator<Object> iterator() { - return (Iterator<Object>) set.iterator(); - } - - public Collection<Object> toCollection() { - // Do not remove <Object>: workaround for Java 7 type inference. - return ImmutableList.<Object>copyOf(set.toCollection()); - } - public boolean isEmpty() { return set.isEmpty(); } @@ -319,7 +334,7 @@ public final class SkylarkNestedSet implements Iterable<Object>, SkylarkValue, S @Override public void write(Appendable buffer, char quotationMark) { Printer.append(buffer, "set("); - Printer.printList(buffer, this, "[", ", ", "]", null, quotationMark); + Printer.printList(buffer, set, "[", ", ", "]", null, quotationMark); Order order = getOrder(); if (order != Order.STABLE_ORDER) { Printer.append(buffer, ", order = \"" + order.getSkylarkName() + "\""); 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 e2cc62fcbb..dbbfc79c42 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 @@ -530,11 +530,13 @@ public abstract class Type<T> { @Override public List<ElemT> convert(Object x, Object what, Object context) throws ConversionException { - if (!(x instanceof Iterable<?>)) { + Iterable<?> iterable; + try { + iterable = EvalUtils.toIterableStrict(x, null); + } catch (EvalException ex) { throw new ConversionException(this, x, what); } int index = 0; - Iterable<?> iterable = (Iterable<?>) x; List<ElemT> result = new ArrayList<>(Iterables.size(iterable)); ListConversionContext conversionContext = new ListConversionContext(what); for (Object elem : iterable) { |