bazel packages: simplify NativeInfo Only one of the 75 subclasses of NativeInfo uses its 'values' map. This change pushes the map down into the sole user (ToolchainInfo), saving two words in the NativeInfo object, plus an ImmutableSet of its field names. OutputGroupInfo now implements StructImpl directly, as it has no annotated fields and thus no need for NativeInfo. Also: - better doc comments - remove test of NativeInfo.values functionality. CcToolchainInfo test slightly augmented to compensate. - delete unused ToolchainInfo.create - make MapBackedSkylarkInfo private - delete 'implements Serializable' - delete getImplementationClasses PiperOrigin-RevId: 284628869
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/StarlarkAttrTransitionProviderTest.java b/src/test/java/com/google/devtools/build/lib/analysis/StarlarkAttrTransitionProviderTest.java index c0a66b6..8d3322a 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/StarlarkAttrTransitionProviderTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/StarlarkAttrTransitionProviderTest.java
@@ -26,7 +26,7 @@ import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.packages.Provider; -import com.google.devtools.build.lib.packages.SkylarkInfo.MapBackedSkylarkInfo; +import com.google.devtools.build.lib.packages.SkylarkInfo; import com.google.devtools.build.lib.packages.SkylarkProvider; import com.google.devtools.build.lib.packages.StructImpl; import com.google.devtools.build.lib.packages.util.BazelMockAndroidSupport; @@ -150,8 +150,8 @@ "my_rule(name = 'test', dep = ':dep')", "simple_rule(name = 'dep')"); - MapBackedSkylarkInfo splitAttr = - (MapBackedSkylarkInfo) + SkylarkInfo splitAttr = + (SkylarkInfo) getMyInfoFromTarget(getConfiguredTarget("//test/skylark:test")).getValue("split_attr"); assertThat(splitAttr.getValue("dep")).isNull();
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/platform/ToolchainInfoTest.java b/src/test/java/com/google/devtools/build/lib/analysis/platform/ToolchainInfoTest.java index d642994..8653d77 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/platform/ToolchainInfoTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/platform/ToolchainInfoTest.java
@@ -71,17 +71,14 @@ new EqualsTester() .addEqualityGroup( // Base case. - ToolchainInfo.create( - ImmutableMap.<String, Object>of("foo", "val1", "bar", "val2"), - Location.BUILTIN), - ToolchainInfo.create( - ImmutableMap.<String, Object>of("foo", "val1", "bar", "val2"), - Location.BUILTIN)) + new ToolchainInfo( + ImmutableMap.<String, Object>of("foo", "val1", "bar", "val2"), Location.BUILTIN), + new ToolchainInfo( + ImmutableMap.<String, Object>of("foo", "val1", "bar", "val2"), Location.BUILTIN)) .addEqualityGroup( // Different data. - ToolchainInfo.create( - ImmutableMap.<String, Object>of("foo", "val1", "bar", "val3"), - Location.BUILTIN)) + new ToolchainInfo( + ImmutableMap.<String, Object>of("foo", "val1", "bar", "val3"), Location.BUILTIN)) .testEquals(); } }
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java index 1e3cf1d..3062a51 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java
@@ -59,6 +59,7 @@ " sysroot = provider.sysroot,", " cpu = provider.cpu,", " ar_executable = provider.ar_executable,", + " ar_executable_call = provider.ar_executable(),", " use_pic_for_dynamic_libraries = provider.needs_pic_for_dynamic_libraries(", " feature_configuration = feature_configuration,", " ),", @@ -81,7 +82,12 @@ Label.parseAbsolute("//test:rule.bzl", ImmutableMap.of()), "MyInfo"); StructImpl info = (StructImpl) ct.get(key); + // Observe: provider.ar_executable == provider.ar_executable(). + // That's because ToolchainInfo has an ar_executable field in its values map, + // and a @SkylarkCallable method of that name, and x.f() calls the method + // whereas x.f retrieves the field. This inconsistency is b/145735895. assertThat((String) info.getValue("ar_executable")).endsWith("/usr/bin/mock-ar"); + assertThat((String) info.getValue("ar_executable_call")).endsWith("/usr/bin/mock-ar"); assertThat(info.getValue("cpu")).isEqualTo("k8");
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java index 3ca022f..2513b33 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
@@ -20,6 +20,7 @@ import com.google.common.collect.ImmutableCollection; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.analysis.test.AnalysisFailure; import com.google.devtools.build.lib.analysis.test.AnalysisFailureInfo; import com.google.devtools.build.lib.cmdline.Label; @@ -2080,43 +2081,60 @@ .testExpression("foo(23, 5, 0)", 18); } + // This class extends NativeInfo (which provides @SkylarkCallable-annotated fields) + // with additional fields from a map. @SkylarkModule(name = "SkylarkClassObjectWithSkylarkCallables", doc = "") - static final class SkylarkClassObjectWithSkylarkCallables extends NativeInfo { - private static final NativeProvider<SkylarkClassObjectWithSkylarkCallables> CONSTRUCTOR = + private static final class SkylarkClassObjectWithSkylarkCallables extends NativeInfo { + + static final NativeProvider<SkylarkClassObjectWithSkylarkCallables> CONSTRUCTOR = new NativeProvider<SkylarkClassObjectWithSkylarkCallables>( SkylarkClassObjectWithSkylarkCallables.class, "struct_with_skylark_callables") {}; + final Map<String, Object> fields = + ImmutableMap.of( + "values_only_field", + "fromValues", + "values_only_method", + new BuiltinFunction(FunctionSignature.of()) { + @Override + public String getName() { + return "values_only_method"; + } + + public String invoke() { + return "fromValues"; + } + }, + "collision_field", + "fromValues", + "collision_method", + new BuiltinFunction(FunctionSignature.of()) { + @Override + public String getName() { + return "collision_method"; + } + + public String invoke() { + return "fromValues"; + } + }); + SkylarkClassObjectWithSkylarkCallables() { - super( - CONSTRUCTOR, - ImmutableMap.of( - "values_only_field", - "fromValues", - "values_only_method", - new BuiltinFunction(FunctionSignature.of()) { - @Override - public String getName() { - return "values_only_method"; - } + super(CONSTRUCTOR, Location.BUILTIN); + } - public String invoke() { - return "fromValues"; - } - }, - "collision_field", - "fromValues", - "collision_method", - new BuiltinFunction(FunctionSignature.of()) { - @Override - public String getName() { - return "collision_method"; - } + @Override + public Object getValue(String name) throws EvalException { + Object x = fields.get(name); + return x != null ? x : super.getValue(name); + } - public String invoke() { - return "fromValues"; - } - }), - Location.BUILTIN); + @Override + public ImmutableCollection<String> getFieldNames() { + return ImmutableSet.<String>builder() + .addAll(fields.keySet()) + .addAll(super.getFieldNames()) + .build(); } @SkylarkCallable(name = "callable_only_field", documented = false, structField = true)