diff options
author | 2015-03-02 19:52:39 +0000 | |
---|---|---|
committer | 2015-03-05 14:16:49 +0000 | |
commit | c673a82bea3457cf51de51df73759ac947feadcb (patch) | |
tree | f892986eaaa810c387b1e58c28bf6326737df217 /src/main/java/com/google/devtools/build | |
parent | edf7bdb0e4e62c75a8ac889ebfc01519af1c20e4 (diff) |
Minor cleanups in Skylark
--
MOS_MIGRATED_REVID=87535290
Diffstat (limited to 'src/main/java/com/google/devtools/build')
6 files changed, 36 insertions, 46 deletions
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 fe0ae3cc9f..43dc269257 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 @@ -21,6 +21,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; +import com.google.common.collect.Ordering; import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.Location; @@ -46,10 +47,8 @@ import com.google.devtools.build.lib.syntax.SkylarkNestedSet; 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; import java.util.List; import java.util.Map; import java.util.Set; @@ -113,18 +112,10 @@ public class MethodLibrary { ImmutableList.of("this", "elements"), 2, false) { @Override public Object call(Object[] args, FuncallExpression ast) throws ConversionException { - String thiz = Type.STRING.convert(args[0], "'join' operand"); + String thisString = Type.STRING.convert(args[0], "'join' operand"); List<?> seq = Type.OBJECT_LIST.convert(args[1], "'join' argument"); - StringBuilder sb = new StringBuilder(); - for (Iterator<?> i = seq.iterator(); i.hasNext();) { - sb.append(i.next().toString()); - if (i.hasNext()) { - sb.append(thiz); - } - } - return sb.toString(); - } - }; + return Joiner.on(thisString).join(seq); + }}; @SkylarkBuiltin(name = "lower", objectType = StringModule.class, returnType = String.class, doc = "Returns the lower case version of this string.") @@ -295,7 +286,7 @@ public class MethodLibrary { end = Type.INTEGER.convert(args[3], "'count' argument"); } String str = getPythonSubstring(thiz, start, end); - if (sub.equals("")) { + if (sub.isEmpty()) { return str.length() + 1; } int count = 0; @@ -591,7 +582,7 @@ public class MethodLibrary { @Override public Object call(List<Object> args, Map<String, Object> kwargs, FuncallExpression ast, Environment env) throws EvalException, InterruptedException { - if (args.size() > 0) { + if (!args.isEmpty()) { throw new EvalException(ast.getLocation(), "struct only supports keyword arguments"); } return new SkylarkClassObject(kwargs, ast.getLocation()); @@ -836,7 +827,7 @@ public class MethodLibrary { @Override public Object call(Object[] args, FuncallExpression ast) throws EvalException { // There is no 'type' type in Skylark, so we return a string with the type name. - return EvalUtils.getDataTypeName(args[0]); + return EvalUtils.getDataTypeName(args[0], false); } }; @@ -888,12 +879,10 @@ public class MethodLibrary { count = 1; } if (kwargs.size() > count) { - kwargs = new HashMap<String, Object>(kwargs); + kwargs = new HashMap<>(kwargs); kwargs.remove("sep"); - List<String> bad = new ArrayList<String>(kwargs.keySet()); - java.util.Collections.sort(bad); - throw new EvalException(ast.getLocation(), - "unexpected keywords: '" + bad + "'"); + List<String> bad = Ordering.natural().sortedCopy(kwargs.keySet()); + throw new EvalException(ast.getLocation(), "unexpected keywords: '" + bad + "'"); } String msg = Joiner.on(sep).join(Iterables.transform(args, new com.google.common.base.Function<Object, String>() { @@ -962,11 +951,7 @@ public class MethodLibrary { .put(count, SkylarkType.INT) .build(); - public static final List<Function> listFunctions = ImmutableList - .<Function>builder() - .add(append) - .add(extend) - .build(); + public static final List<Function> listFunctions = ImmutableList.of(append, extend); public static final Map<Function, SkylarkType> dictFunctions = ImmutableMap .<Function, SkylarkType>builder() diff --git a/src/main/java/com/google/devtools/build/lib/packages/Type.java b/src/main/java/com/google/devtools/build/lib/packages/Type.java index 48ba948090..fb6a04feb0 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Type.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Type.java @@ -27,6 +27,7 @@ import com.google.devtools.build.lib.syntax.FilesetEntry; import com.google.devtools.build.lib.syntax.GlobList; import com.google.devtools.build.lib.syntax.Label; import com.google.devtools.build.lib.syntax.SelectorValue; +import com.google.devtools.build.lib.syntax.SkylarkList; import com.google.devtools.build.lib.util.LoggingUtil; import com.google.devtools.build.lib.util.StringCanonicalizer; @@ -444,9 +445,7 @@ public abstract class Type<T> { throw new IllegalStateException(msg); } String tag = (Boolean) value ? name : "no" + name; - return new ImmutableSet.Builder<String>() - .add(tag) - .build(); + return ImmutableSet.of(tag); } } @@ -540,9 +539,7 @@ public abstract class Type<T> { String msg = "Illegal tag conversion from null on Attribute " + name + "."; throw new IllegalStateException(msg); } - return new ImmutableSet.Builder<String>() - .add((String) value) - .build(); + return ImmutableSet.of((String) value); } } @@ -888,7 +885,7 @@ public abstract class Type<T> { /** * A list is representable as a tag set as the contents of itself expressed - * as Strings. So a List<String> is effectively converted to a Set<String>. + * as Strings. So a {@code List<String>} is effectively converted to a {@code Set<String>}. */ @Override public Set<String> toTagSet(Object items, String name) { @@ -919,7 +916,9 @@ public abstract class Type<T> { @SuppressWarnings("unchecked") public List<Object> convert(Object x, String what, Label currentRule) throws ConversionException { - if (x instanceof List) { + if (x instanceof SkylarkList) { + return ((SkylarkList) x).toList(); + } else if (x instanceof List) { return (List<Object>) x; } else if (x instanceof Iterable) { return ImmutableList.copyOf((Iterable<?>) x); @@ -945,7 +944,7 @@ public abstract class Type<T> { /** * Special Type that represents a selector expression for configurable attributes. Holds a - * mapping of <Label, T> entries, where keys are configurability patterns and values are + * mapping of {@code <Label, T>} entries, where keys are configurability patterns and values are * objects of the attribute's native Type. */ public static final class Selector<T> { diff --git a/src/main/java/com/google/devtools/build/lib/syntax/DictionaryLiteral.java b/src/main/java/com/google/devtools/build/lib/syntax/DictionaryLiteral.java index 953840b3f1..0b926f6ae1 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/DictionaryLiteral.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/DictionaryLiteral.java @@ -83,7 +83,7 @@ public class DictionaryLiteral extends Expression { @Override public String toString() { - StringBuffer sb = new StringBuffer(); + StringBuilder sb = new StringBuilder(); sb.append("{"); String sep = ""; for (DictionaryEntryLiteral e : entries) { @@ -116,6 +116,6 @@ public class DictionaryLiteral extends Expression { } type = type.infer(nextType, "dict literal", entry.getLocation(), getLocation()); } - return SkylarkType.of(Map.class, type.getType()); + return SkylarkType.of(SkylarkType.MAP, type); } } 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 index c317c7d0a3..8b0f5b0751 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/FunctionSignature.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/FunctionSignature.java @@ -191,7 +191,7 @@ public abstract class FunctionSignature implements Serializable { || defaultValues.size() == shape.getOptionals()); Preconditions.checkArgument(types == null || types.size() == shape.getArguments()); - return new AutoValueFunctionSignatureWithValues<V, T>(signature, defaultValues, types); + return new AutoValueFunctionSignatureWithValues<>(signature, defaultValues, types); } public static <V, T> WithValues<V, T> create(FunctionSignature signature, @@ -500,7 +500,12 @@ public abstract class FunctionSignature implements Serializable { // 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()) + // which importantly for future plans will define .equals() and .hashCode() (also toString()). + // Then, intern Shape, name list, FunctionSignature and WithDefaults, so that + // comparison can be done with == and more memory and caches can be shared between functions. + // Later, this can lead to further optimizations of function call by using tables matching + // a FunctionSignature and a CallerSignature. (struct access can be similarly sped up + // by interning the list of names.) private static class AutoValueFunctionSignatureShape extends Shape { private int mandatoryPositionals; private int optionalPositionals; 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 b8fb203351..97a4bbd015 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 @@ -148,9 +148,9 @@ class Parser { this.eventHandler = eventHandler; this.parsePython = parsePython; this.tokens = lexer.getTokens().iterator(); - this.comments = new ArrayList<Comment>(); + this.comments = new ArrayList<>(); this.locator = locator; - this.includedFiles = new ArrayList<Path>(); + this.includedFiles = new ArrayList<>(); this.includedFiles.add(lexer.getFilename()); nextToken(); } @@ -762,8 +762,7 @@ class Parser { "null element in list in AST at %s:%s", token.left, token.right); switch (token.kind) { case RBRACKET: { // singleton List - ListLiteral literal = - ListLiteral.makeList(Collections.singletonList(expression)); + ListLiteral literal = ListLiteral.makeList(Collections.singletonList(expression)); setLocation(literal, start, token.right); nextToken(); return literal; 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 c30824f1af..4e25b771b0 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 @@ -312,7 +312,7 @@ public abstract class SkylarkList implements Iterable<Object> { /** * @param elements the contents of the list * @param contentType a Java class for the contents of the list - * @return a Skylark list containing elements without a type check. + * @return a Skylark list containing elements without a type check * Only use if you already know for sure all elements are of the specified type. */ @SuppressWarnings("unchecked") @@ -323,11 +323,12 @@ public abstract class SkylarkList implements Iterable<Object> { /** * @param elements the contents of the list * @param contentType a SkylarkType for the contents of the list - * @return a Skylark list without a type check and without creating an immutable copy. + * @return a Skylark list without a type check and without creating an immutable copy * Therefore the iterable containing elements must be immutable * (which is not checked here so callers must be extra careful). * This way it's possibly to create a SkylarkList without requesting the original iterator. * This can be useful for nested set - list conversions. + * Only use if you already know for sure all elements are of the specified type. */ @SuppressWarnings("unchecked") public static SkylarkList lazyList(Iterable<?> elements, SkylarkType contentType) { @@ -337,11 +338,12 @@ public abstract class SkylarkList implements Iterable<Object> { /** * @param elements the contents of the list * @param contentType a Java class for the contents of the list - * @return a Skylark list without a type check and without creating an immutable copy. + * @return a Skylark list without a type check and without creating an immutable copy * Therefore the iterable containing elements must be immutable * (which is not checked here so callers must be extra careful). * This way it's possibly to create a SkylarkList without requesting the original iterator. * This can be useful for nested set - list conversions. + * Only use if you already know for sure all elements are of the specified type. */ @SuppressWarnings("unchecked") public static SkylarkList lazyList(Iterable<?> elements, Class<?> contentType) { |