Info-related cleanups
- Reorder Info methods for consistency with ClassObject
- "StructConstructor" -> "StructProvider"
- Added javadoc
RELNOTES: None
PiperOrigin-RevId: 181469643
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleConfiguredTargetUtil.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleConfiguredTargetUtil.java
index b398073..a40bacb 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleConfiguredTargetUtil.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleConfiguredTargetUtil.java
@@ -268,7 +268,7 @@
SkylarkRuleContext context, RuleConfiguredTargetBuilder builder, Object target, Location loc)
throws EvalException {
- Info oldStyleProviders = NativeProvider.STRUCT.create(loc);
+ Info oldStyleProviders = NativeProvider.STRUCT.createEmpty(loc);
ArrayList<Info> declaredProviders = new ArrayList<>();
if (target instanceof Info) {
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Info.java b/src/main/java/com/google/devtools/build/lib/packages/Info.java
index b885d65..b3ed563 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Info.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Info.java
@@ -99,6 +99,8 @@
/**
* Preprocesses a map of field values to convert the field names and field values to
* Skylark-acceptable names and types.
+ *
+ * <p>This preserves the order of the map entries.
*/
protected static ImmutableMap<String, Object> copyValues(Map<String, Object> values) {
ImmutableMap.Builder<String, Object> builder = ImmutableMap.builder();
@@ -111,23 +113,6 @@
}
/**
- * Returns whether the given field name exists.
- *
- * <p>This conceptually extends the API for {@link ClassObject}.
- */
- public abstract boolean hasField(String name);
-
- /** Returns a value and try to cast it into specified type */
- public <T> T getValue(String key, Class<T> type) throws EvalException {
- Object obj = getValue(key);
- if (obj == null) {
- return null;
- }
- SkylarkType.checkType(obj, type, key);
- return type.cast(obj);
- }
-
- /**
* Returns the Skylark location where this provider instance was created.
*
* <p>Builtin provider instances may return {@link Location#BUILTIN}.
@@ -141,25 +126,44 @@
}
/**
- * Returns the fields of this struct.
+ * Returns whether the given field name exists.
*
- * Overrides {@link ClassObject#getFieldNames()}, but does not allow {@link EvalException} to
- * be thrown.
+ * <p>This conceptually extends the API for {@link ClassObject}.
*/
- @Override
- public abstract ImmutableCollection<String> getFieldNames();
+ public abstract boolean hasField(String name);
/**
- * Returns the value associated with the name field in this struct,
- * or null if the field does not exist.
+ * {@inheritDoc}
*
- * Overrides {@link ClassObject#getValue(String)}, but does not allow {@link EvalException} to
+ * <p>Overrides {@link ClassObject#getValue(String)}, but does not allow {@link EvalException} to
* be thrown.
*/
@Nullable
@Override
public abstract Object getValue(String name);
+ /**
+ * Returns the result of {@link #getValue(String)}, cast as the given type, throwing {@link
+ * EvalException} if the cast fails.
+ */
+ public <T> T getValue(String key, Class<T> type) throws EvalException {
+ Object obj = getValue(key);
+ if (obj == null) {
+ return null;
+ }
+ SkylarkType.checkType(obj, type, key);
+ return type.cast(obj);
+ }
+
+ /**
+ * {@inheritDoc}
+ *
+ * <p>Overrides {@link ClassObject#getFieldNames()}, but does not allow {@link EvalException} to
+ * be thrown.
+ */
+ @Override
+ public abstract ImmutableCollection<String> getFieldNames();
+
@Override
public String getErrorMessageForUnknownField(String name) {
String suffix = "Available attributes: "
diff --git a/src/main/java/com/google/devtools/build/lib/packages/NativeProvider.java b/src/main/java/com/google/devtools/build/lib/packages/NativeProvider.java
index 0f78e46..22a4eaa 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/NativeProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/NativeProvider.java
@@ -45,7 +45,7 @@
private final String errorMessageFormatForUnknownField;
/** "struct" function. */
- public static final StructConstructor STRUCT = new StructConstructor();
+ public static final StructProvider STRUCT = new StructProvider();
private final Class<V> valueClass;
@@ -61,17 +61,17 @@
* Skylark code.
*/
@Deprecated
- public static interface WithLegacySkylarkName {
+ public interface WithLegacySkylarkName {
String getSkylarkName();
}
/**
- * A constructor for default {@code struct}s.
+ * The provider for the built-in type {@code struct}.
*
- * <p>Singleton, instance is {@link #STRUCT}.
+ * <p>Its singleton instance is {@link #STRUCT}.
*/
- public static final class StructConstructor extends NativeProvider<Info> {
- private StructConstructor() {
+ public static final class StructProvider extends NativeProvider<Info> {
+ private StructProvider() {
super(Info.class, "struct");
}
@@ -82,11 +82,19 @@
return SkylarkInfo.fromMap(this, kwargs, loc);
}
- public Info create(Map<String, Object> values, String message) {
- return new SkylarkInfo.MapBackedSkylarkInfo(this, values, message);
+ /**
+ * Creates a struct with the he given field values and message format for unknown fields.
+ *
+ * <p>The custom message is useful for objects that have fields but aren't exactly used as
+ * providers, such as the {@code native} object, and the struct fields of {@code ctx} like
+ * {@code ctx.attr}.
+ * */
+ public Info create(Map<String, Object> values, String errorMessageFormatForUnknownField) {
+ return new SkylarkInfo.MapBackedSkylarkInfo(this, values, errorMessageFormatForUnknownField);
}
- public Info create(Location loc) {
+ /** Creates an empty struct with the given location. */
+ public Info createEmpty(Location loc) {
return SkylarkInfo.fromMap(this, ImmutableMap.of(), loc);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkInfo.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkInfo.java
index cc5d3ea..15e5b5e 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkInfo.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkInfo.java
@@ -11,6 +11,7 @@
// 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.packages;
import com.google.common.base.Joiner;
@@ -21,22 +22,28 @@
import com.google.common.collect.Sets;
import com.google.common.collect.Sets.SetView;
import com.google.devtools.build.lib.events.Location;
-import com.google.devtools.build.lib.packages.NativeProvider.StructConstructor;
+import com.google.devtools.build.lib.packages.NativeProvider.StructProvider;
import com.google.devtools.build.lib.syntax.Concatable;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.EvalUtils;
import java.util.Arrays;
import java.util.Map;
-/** Implementation of {@link Info} created from Skylark. */
+/**
+ * A provider instance of either 1) a Skylark-defined provider ({@link SkylarkInfo}), or 2) the
+ * built-in "struct" type ({@link NativeProvider#STRUCT}).
+ *
+ * <p>There are two concrete subclasses corresponding to two different implementation strategies.
+ * One is map-based and schemaless, the other has a fixed layout and is more memory-efficient.
+ */
public abstract class SkylarkInfo extends Info implements Concatable {
public SkylarkInfo(Provider provider, Location loc) {
super(provider, loc);
}
- public SkylarkInfo(StructConstructor provider, String message) {
- super(provider, message);
+ public SkylarkInfo(StructProvider provider, String errorMessageFormatForUnknownField) {
+ super(provider, errorMessageFormatForUnknownField);
}
@Override
@@ -46,7 +53,7 @@
@Override
public boolean isImmutable() {
- // If the provider is not yet exported the hash code of the object is subject to change
+ // If the provider is not yet exported, the hash code of the object is subject to change.
if (!getProvider().isExported()) {
return false;
}
@@ -58,15 +65,21 @@
return true;
}
- /** Return all the values stored in the object. */
+ /**
+ * Returns all the field values stored in the object, in the canonical order.
+ *
+ * <p>{@code protected} because this is only used for {@link #isImmutable}. It saves us having to
+ * get values one-by-one.
+ */
protected abstract Iterable<Object> getValues();
/**
- * {@link SkylarkInfo} implementation that stores its values in a map. This is mainly used for the
- * Skylark {@code struct()} constructor.
+ * A {@link SkylarkInfo} implementation that stores its values in a map. This is used for structs
+ * and for schemaless Skylark-defined providers.
*/
static final class MapBackedSkylarkInfo extends SkylarkInfo {
- protected final ImmutableMap<String, Object> values;
+
+ private final ImmutableMap<String, Object> values;
public MapBackedSkylarkInfo(Provider provider, Map<String, Object> kwargs, Location loc) {
super(provider, loc);
@@ -74,22 +87,24 @@
}
public MapBackedSkylarkInfo(
- StructConstructor provider, Map<String, Object> values, String message) {
- super(provider, message);
+ StructProvider provider,
+ Map<String, Object> values,
+ String errorMessageFormatForUnknownField) {
+ super(provider, errorMessageFormatForUnknownField);
this.values = copyValues(values);
}
@Override
- public Object getValue(String name) {
- return values.get(name);
- }
-
- @Override
public boolean hasField(String name) {
return values.containsKey(name);
}
@Override
+ public Object getValue(String name) {
+ return values.get(name);
+ }
+
+ @Override
public ImmutableCollection<String> getFieldNames() {
return values.keySet();
}
@@ -163,6 +178,7 @@
@Override
public Concatable concat(Concatable left, Concatable right, Location loc) throws EvalException {
+ // Casts are safe because SkylarkInfoConcatter is only used by SkylarkInfo.
SkylarkInfo lval = (SkylarkInfo) left;
SkylarkInfo rval = (SkylarkInfo) right;
Provider provider = lval.getProvider();
@@ -170,7 +186,7 @@
throw new EvalException(
loc,
String.format(
- "Cannot concat %s with %s",
+ "Cannot use '+' operator on instances of different providers (%s and %s)",
provider.getPrintableName(), rval.getProvider().getPrintableName()));
}
SetView<String> commonFields =
@@ -179,7 +195,8 @@
if (!commonFields.isEmpty()) {
throw new EvalException(
loc,
- "Cannot concat structs with common field(s): " + Joiner.on(",").join(commonFields));
+ "Cannot use '+' operator on provider instances with overlapping field(s): "
+ + Joiner.on(",").join(commonFields));
}
// Keep homogeneous compact concatenations compact.
if (lval instanceof CompactSkylarkInfo
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
index 4efa55a..bcd42e8 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
@@ -1059,7 +1059,7 @@
@Test
public void testStructConcatenationCommonFields() throws Exception {
- checkErrorContains("Cannot concat structs with common field(s): a",
+ checkErrorContains("Cannot use '+' operator on provider instances with overlapping field(s): a",
"x = struct(a = 1, b = 2)", "y = struct(c = 1, a = 2)", "z = x + y\n");
}
@@ -1247,7 +1247,7 @@
"data2 = provider()"
);
- checkEvalError("Cannot concat data1 with data2",
+ checkEvalError("Cannot use '+' operator on instances of different providers (data1 and data2)",
"d1 = data1(x = 1)",
"d2 = data2(y = 2)",
"d = d1 + d2"
@@ -1271,7 +1271,7 @@
public void declaredProvidersWithFieldsConcatError() throws Exception {
evalAndExport("data1 = provider(fields=['f1', 'f2'])", "data2 = provider(fields=['f3'])");
checkEvalError(
- "Cannot concat data1 with data2",
+ "Cannot use '+' operator on instances of different providers (data1 and data2)",
"d1 = data1(f1=1, f2=2)",
"d2 = data2(f3=3)",
"d = d1 + d2");
@@ -1281,7 +1281,7 @@
public void declaredProvidersWithOverlappingFieldsConcatError() throws Exception {
evalAndExport("data = provider(fields=['f1', 'f2'])");
checkEvalError(
- "Cannot concat structs with common field(s): f1",
+ "Cannot use '+' operator on provider instances with overlapping field(s): f1",
"d1 = data(f1 = 4)",
"d2 = data(f1 = 5)",
"d1 + d2");
