Use Object for what instead of String
The string is only used for error messaging on failure. Instead of constructing
lots of strings allow passing in a generic Object whos toString is used for the
error message. This allows us to bypass a lot of garbage string generation, at
th expense of a little indirection.
Chose Object over some specialized type, since we use inline strings frequently
as well.
--
MOS_MIGRATED_REVID=133741724
diff --git a/src/main/java/com/google/devtools/build/lib/packages/BuildType.java b/src/main/java/com/google/devtools/build/lib/packages/BuildType.java
index 3a8c69c..825efb5 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/BuildType.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/BuildType.java
@@ -28,7 +28,6 @@
import com.google.devtools.build.lib.syntax.Type.ConversionException;
import com.google.devtools.build.lib.syntax.Type.DictType;
import com.google.devtools.build.lib.syntax.Type.ListType;
-
import java.util.Collection;
import java.util.Collections;
import java.util.LinkedHashMap;
@@ -36,7 +35,6 @@
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
-
import javax.annotation.Nullable;
/**
@@ -87,7 +85,7 @@
}
@Override
- public DistributionType convert(Object x, String what, Object context) {
+ public DistributionType convert(Object x, Object what, Object context) {
throw new UnsupportedOperationException();
}
@@ -158,7 +156,7 @@
* <p>The caller is responsible for casting the returned value appropriately.
*/
public static <T> Object selectableConvert(
- Type type, Object x, String what, @Nullable Label context)
+ Type type, Object x, Object what, @Nullable Label context)
throws ConversionException {
if (x instanceof com.google.devtools.build.lib.syntax.SelectorList) {
return new SelectorList<T>(
@@ -177,7 +175,7 @@
}
@Override
- public FilesetEntry convert(Object x, String what, Object context)
+ public FilesetEntry convert(Object x, Object what, Object context)
throws ConversionException {
if (!(x instanceof FilesetEntry)) {
throw new ConversionException(this, x, what);
@@ -233,7 +231,7 @@
}
@Override
- public Label convert(Object x, String what, Object context)
+ public Label convert(Object x, Object what, Object context)
throws ConversionException {
if (x instanceof Label) {
return (Label) x;
@@ -259,7 +257,7 @@
}
@Override
- public License convert(Object x, String what, Object context) throws ConversionException {
+ public License convert(Object x, Object what, Object context) throws ConversionException {
try {
List<String> licenseStrings = STRING_LIST.convert(x, what);
return License.parseLicense(licenseStrings);
@@ -303,7 +301,7 @@
}
@Override
- public Set<DistributionType> convert(Object x, String what, Object context)
+ public Set<DistributionType> convert(Object x, Object what, Object context)
throws ConversionException {
try {
List<String> distribStrings = STRING_LIST.convert(x, what);
@@ -366,7 +364,7 @@
}
@Override
- public Label convert(Object x, String what, Object context)
+ public Label convert(Object x, Object what, Object context)
throws ConversionException {
String value;
@@ -401,7 +399,7 @@
private final List<Selector<T>> elements;
@VisibleForTesting
- SelectorList(List<Object> x, String what, @Nullable Label context,
+ SelectorList(List<Object> x, Object what, @Nullable Label context,
Type<T> originalType) throws ConversionException {
if (x.size() > 1 && originalType.concat(ImmutableList.<T>of()) == null) {
throw new ConversionException(
@@ -411,7 +409,7 @@
ImmutableList.Builder<Selector<T>> builder = ImmutableList.builder();
for (Object elem : x) {
if (elem instanceof SelectorValue) {
- builder.add(new Selector<T>(((SelectorValue) elem).getDictionary(), what,
+ builder.add(new Selector<>(((SelectorValue) elem).getDictionary(), what,
context, originalType, ((SelectorValue) elem).getNoMatchError()));
} else {
T directValue = originalType.convert(elem, what, context);
@@ -482,7 +480,7 @@
/**
* Creates a new Selector using the default error message when no conditions match.
*/
- Selector(ImmutableMap<?, ?> x, String what, @Nullable Label context, Type<T> originalType)
+ Selector(ImmutableMap<?, ?> x, Object what, @Nullable Label context, Type<T> originalType)
throws ConversionException {
this(x, what, context, originalType, "");
}
@@ -490,7 +488,7 @@
/**
* Creates a new Selector with a custom error message for when no conditions match.
*/
- Selector(ImmutableMap<?, ?> x, String what, @Nullable Label context, Type<T> originalType,
+ Selector(ImmutableMap<?, ?> x, Object what, @Nullable Label context, Type<T> originalType,
String noMatchError) throws ConversionException {
this.originalType = originalType;
LinkedHashMap<Label, T> result = new LinkedHashMap<>();
@@ -633,7 +631,7 @@
// Like BooleanType, this must handle integers as well.
@Override
- public TriState convert(Object x, String what, Object context)
+ public TriState convert(Object x, Object what, Object context)
throws ConversionException {
if (x instanceof TriState) {
return (TriState) x;
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 cba10a7..53aabfa 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
@@ -1727,9 +1727,11 @@
*/
private static Object convertFromBuildLangType(Rule rule, Attribute attr, Object buildLangValue)
throws ConversionException {
- String what = "attribute '" + attr.getName() + "' in '" + rule.getRuleClass() + "' rule";
- Object converted =
- BuildType.selectableConvert(attr.getType(), buildLangValue, what, rule.getLabel());
+ Object converted = BuildType.selectableConvert(
+ attr.getType(),
+ buildLangValue,
+ new AttributeConversionContext(attr.getName(), rule.getRuleClass()),
+ rule.getLabel());
if ((converted instanceof SelectorList<?>) && !attr.isConfigurable()) {
throw new ConversionException(
@@ -1749,6 +1751,28 @@
}
/**
+ * Provides a {@link #toString()} description of the attribute being converted for
+ * {@link BuildType#selectableConvert}. This is preferred over a raw string to avoid uselessly
+ * constructing strings which are never used. A separate class instead of inline to avoid
+ * accidental memory leaks.
+ */
+ private static class AttributeConversionContext {
+ private final String attrName;
+ private final String ruleClass;
+
+ AttributeConversionContext(String attrName, String ruleClass) {
+ this.attrName = attrName;
+ this.ruleClass = ruleClass;
+ }
+
+ @Override
+ public String toString() {
+ return "attribute '" + attrName + "' in '" + ruleClass + "' rule";
+ }
+ }
+
+
+ /**
* Verifies that the rule has a valid value for the attribute according to its allowed values.
*
* <p>If the value for the given attribute on the given rule is invalid, an error will be recorded
