From a60c6a4b76d6bdf49c3307eb6cb81f9796aaa359 Mon Sep 17 00:00:00 2001 From: brandjon Date: Thu, 21 Dec 2017 05:53:21 -0800 Subject: Refactor SkylarkProvider constructors and add tests A new constructor is exposed for building an already-exported SkylarkProvider. The existing constructor no longer takes a name argument (since it was almost entirely ignored). The contract around the name arg for BaseFunction has been refined: it is null if the subclass provides its own naming mechanism. RELNOTES: None PiperOrigin-RevId: 179804491 --- .../devtools/build/lib/packages/Provider.java | 17 ++- .../build/lib/packages/SkylarkProvider.java | 160 ++++++++++++++++----- 2 files changed, 139 insertions(+), 38 deletions(-) (limited to 'src/main/java/com/google/devtools/build/lib/packages') diff --git a/src/main/java/com/google/devtools/build/lib/packages/Provider.java b/src/main/java/com/google/devtools/build/lib/packages/Provider.java index b497b76131..2616a209e0 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Provider.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Provider.java @@ -31,7 +31,9 @@ import javax.annotation.Nullable; * {@link SkylarkProvider}. * *

{@link Provider} serves both as "type identifier" for declared provider instances and as a - * function that can be called to construct a provider. + * function that can be called to construct a provider. To the Skylark user, there are "providers" + * and "provider instances"; the former is a Java instance of this class, and the latter is a Java + * instance of {@link Info}. * *

Prefer to use {@link Key} as a serializable identifier of {@link Provider}. In particular, * {@link Key} should be used in all data structures exposed to Skyframe. @@ -62,8 +64,19 @@ import javax.annotation.Nullable; @Immutable public abstract class Provider extends BaseFunction { + /** + * Constructs a provider. + * + * @param name provider name; should be null iff the subclass overrides {@link #getName} + * @param signature the signature for calling this provider as a Skylark function (to construct an + * instance of the provider) + * @param location the location of this provider's Skylark definition. Use {@link + * Location#BUILTIN} if it is a native provider. + */ protected Provider( - String name, FunctionSignature.WithValues signature, Location location) { + @Nullable String name, + FunctionSignature.WithValues signature, + Location location) { super(name, signature, location); } diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkProvider.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkProvider.java index cdbf73edd1..1a94c9cf2b 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkProvider.java +++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkProvider.java @@ -28,66 +28,138 @@ import java.util.Objects; import javax.annotation.Nullable; /** - * Declared provider defined in Skylark. + * A provider defined in Skylark rather than in native code. * - *

This is a result of calling {@code provider()} function from Skylark ({@link + *

This is a result of calling the {@code provider()} function from Skylark ({@link * com.google.devtools.build.lib.analysis.skylark.SkylarkRuleClassFunctions#provider}). + * + *

{@code SkylarkProvider}s may be either schemaless or schemaful. Instances of schemaless + * providers can have any set of fields on them, whereas instances of schemaful providers may have + * only the fields that are named in the schema. Schemaful provider instances are more space + * efficient since they do not use maps; see {@link SkylarkInfo.CompactSkylarkInfo}. + * + *

Exporting a {@code SkylarkProvider} creates a key that is used to uniquely identify it. + * Usually a provider is exported by calling {@link #export}, but a test may wish to just create + * a pre-exported provider directly. Exported providers use only their key for {@link #equals} and + * {@link #hashCode}. */ public class SkylarkProvider extends Provider implements SkylarkExportable { - private static final FunctionSignature.WithValues SIGNATURE = + private static final FunctionSignature.WithValues SCHEMALESS_SIGNATURE = FunctionSignature.WithValues.create(FunctionSignature.KWARGS); + private static final String DEFAULT_ERROR_MESSAGE_FORMAT = "Object has no '%s' attribute."; + /** - * A map from provider fields to a continuous range of integers. This allows provider instances to - * store their values in an array rather than a map. {@code layout} will be null if the provider - * fields aren't known up front. + * A map from provider fields to a contiguous range of integers, as used in {@link + * SkylarkInfo.CompactSkylarkInfo}. The map entries are ordered by integer value. + * + *

This allows provider instances to store their values in an array rather than a map. {@code + * layout} will be null if the provider is schemaless, i.e., its fields aren't known up-front. */ @Nullable private final ImmutableMap layout; - @Nullable private SkylarkKey key; - @Nullable private String errorMessageFormatForInstances; + /** Null iff this provider has not yet been exported. */ + @Nullable + private SkylarkKey key; + + /** Error message format. Reassigned upon exporting. */ + private String errorMessageFormatForInstances; + + /** + * Creates an unexported {@link SkylarkProvider} with no schema. + * + *

The resulting object needs to be exported later (via {@link #export}). + * + * @param location the location of the Skylark definition for this provider (tests may use {@link + * Location#BUILTIN}) + */ + public static SkylarkProvider createUnexportedSchemaless(Location location) { + return new SkylarkProvider(/*key=*/ null, /*fields=*/ null, location); + } + + /** + * Creates an unexported {@link SkylarkProvider} with a schema. + * + *

The resulting object needs to be exported later (via {@link #export}). + * + * @param fields a list of allowed field names for instances of this provider, in some canonical + * order + * @param location the location of the Skylark definition for this provider (tests may use {@link + * Location#BUILTIN}) + */ + public static SkylarkProvider createUnexportedSchemaful( + Iterable fields, Location location) { + return new SkylarkProvider(/*key=*/ null, fields, location); + } - private static final String DEFAULT_ERROR_MESSAFE = "Object has no '%s' attribute."; + /** + * Creates an exported {@link SkylarkProvider} with no schema. + * + * @param key the key that identifies this provider + * @param location the location of the Skylark definition for this provider (tests may use {@link + * Location#BUILTIN}) + */ + public static SkylarkProvider createExportedSchemaless(SkylarkKey key, Location location) { + return new SkylarkProvider(key, /*fields=*/ null, location); + } /** - * Creates a Skylark-defined Declared Provider ({@link Info} constructor). + * Creates an exported {@link SkylarkProvider} with no schema. * - *

Needs to be exported later. + * @param key the key that identifies this provider + * @param fields a list of allowed field names for instances of this provider, in some canonical + * order + * @param location the location of the Skylark definition for this provider (tests may use {@link + * Location#BUILTIN}) */ - public SkylarkProvider(String name, - @Nullable Iterable fields, - Location location) { - this(name, buildSignature(fields), location); + public static SkylarkProvider createExportedSchemaful( + SkylarkKey key, Iterable fields, Location location) { + return new SkylarkProvider(key, fields, location); } + /** + * Constructs the provider. + * + *

If {@code key} is null, the provider is unexported. If {@code fields} is null, the provider + * is schemaless. + */ private SkylarkProvider( - String name, - FunctionSignature.WithValues signature, Location location) { - super(name, signature, location); - if (signature.getSignature().getShape().hasKwArg()) { - layout = null; + @Nullable SkylarkKey key, @Nullable Iterable fields, Location location) { + // We override getName() in order to use the name that is assigned when export() is called. + // Hence BaseFunction's constructor gets a null name. + super(/*name=*/ null, buildSignature(fields), location); + this.layout = buildLayout(fields); + this.key = key; // possibly null + this.errorMessageFormatForInstances = + key == null ? DEFAULT_ERROR_MESSAGE_FORMAT + : makeErrorMessageFormatForInstances(key.getExportedName()); + } + + + private static FunctionSignature.WithValues buildSignature( + @Nullable Iterable fields) { + if (fields == null) { + return SCHEMALESS_SIGNATURE; + } + return FunctionSignature.WithValues.create( + FunctionSignature.namedOnly(0, ImmutableList.copyOf(fields).toArray(new String[0]))); + } + + @Nullable + private static ImmutableMap buildLayout( + @Nullable Iterable fields) { + if (fields == null) { + return null; } else { ImmutableMap.Builder layoutBuilder = ImmutableMap.builder(); int i = 0; - for (String field : signature.getSignature().getNames()) { + for (String field : fields) { layoutBuilder.put(field, i++); } - layout = layoutBuilder.build(); + return layoutBuilder.build(); } - this.errorMessageFormatForInstances = DEFAULT_ERROR_MESSAFE; - } - - private static FunctionSignature.WithValues buildSignature( - @Nullable Iterable fields) { - if (fields == null) { - return SIGNATURE; - } - return - FunctionSignature.WithValues.create( - FunctionSignature.namedOnly(0, ImmutableList.copyOf(fields).toArray(new String[0])) - ); } @Override @@ -123,6 +195,19 @@ public class SkylarkProvider extends Provider implements SkylarkExportable { return getName(); } + /** + * Returns the list of fields used to define this provider, or null if the provider is schemaless. + * + *

Note: In the future, this method may be replaced by one that returns more detailed schema + * information (if/when the allowed schemas for structs become more complex). + */ + public @Nullable ImmutableList getFields() { + if (layout == null) { + return null; + } + return ImmutableList.copyOf(layout.keySet()); + } + @Override public String getErrorMessageFormatForInstances() { return errorMessageFormatForInstances; @@ -132,8 +217,11 @@ public class SkylarkProvider extends Provider implements SkylarkExportable { public void export(Label extensionLabel, String exportedName) { Preconditions.checkState(!isExported()); this.key = new SkylarkKey(extensionLabel, exportedName); - this.errorMessageFormatForInstances = - String.format("'%s' object has no attribute '%%s'", exportedName); + this.errorMessageFormatForInstances = makeErrorMessageFormatForInstances(exportedName); + } + + private static String makeErrorMessageFormatForInstances(String exportedName) { + return String.format("'%s' object has no attribute '%%s'", exportedName); } @Override -- cgit v1.2.3