LateBoundDefault: enforce access to a single fragment (or none).

Currently, there is no way to enforce that LateBoundDefaults only access
the fragments that they declare. This means that LateBoundDefaults can
fail to declare fragments at all, or declare the wrong ones, and still
have no troubles.

But when trimming, these fragments must be declared, because otherwise
they will not necessarily be available.

This change refactors LateBoundDefault to declare a single fragment type,
not a set. All existing LateBoundDefaults use sets with a single element
or no elements at all for their set of fragment classes, so this does not
limit anything being done currently.

To account for LateBoundDefaults which do not use configuration at all,
typically those which only want to access the configured attribute map,
it is possible for Void to be the fragment class which is requested.

To account for LateBoundDefaults which need to access methods of the
BuildConfiguration instance itself, it is possible for BuildConfiguration
to be the fragment class which is requested; however, this is unsafe, so
it is only a temporary state until a way to do this without also giving
access to all of the fragments can be added.

Drive-by refactoring: LateBoundDefaults' values are now typed. All actual
production LateBoundDefaults were Label or List<Label> typed, through the
LateBoundLabel and LateBoundLabelList subclasses. These subclasses have
been removed, and LateBoundDefault has two type parameters, one for the
type of its input, and one for the type of its output.

RELNOTES: None.
PiperOrigin-RevId: 169242278

3 files changed, 79 insertions, 86 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java
index f52d66f209..8a8e815ad2 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java
@@ -40,11 +40,9 @@ import com.google.devtools.build.lib.analysis.constraints.EnvironmentRule;
 import com.google.devtools.build.lib.analysis.test.TestConfiguration;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.packages.Attribute;
-import com.google.devtools.build.lib.packages.Attribute.LateBoundLabel;
-import com.google.devtools.build.lib.packages.Attribute.LateBoundLabelList;
+import com.google.devtools.build.lib.packages.Attribute.LateBoundDefault;
 import com.google.devtools.build.lib.packages.Attribute.Transition;
 import com.google.devtools.build.lib.packages.AttributeMap;
-import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.packages.RuleClass.Builder;
 import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
@@ -73,35 +71,32 @@ public class BaseRuleClasses {
         }
       };
 
+  // TODO(b/65746853): provide a way to do this without passing the entire configuration
   /**
    * Implementation for the :action_listener attribute.
+   *
+   * <p>action_listeners are special rules; they tell the build system to add extra_actions to
+   * existing rules. As such they need an edge to every ConfiguredTarget with the limitation that
+   * they only run on the target configuration and should not operate on action_listeners and
+   * extra_actions themselves (to avoid cycles).
    */
   @VisibleForTesting
-  static final LateBoundLabelList<BuildConfiguration> ACTION_LISTENER =
-      new LateBoundLabelList<BuildConfiguration>() {
-    @Override
-    public List<Label> resolve(Rule rule, AttributeMap attributes,
-        BuildConfiguration configuration) {
-      // action_listeners are special rules; they tell the build system to add extra_actions to
-      // existing rules. As such they need an edge to every ConfiguredTarget with the limitation
-      // that they only run on the target configuration and should not operate on action_listeners
-      // and extra_actions themselves (to avoid cycles).
-      return configuration.getActionListeners();
-    }
-  };
+  static final LateBoundDefault<?, List<Label>> ACTION_LISTENER =
+      LateBoundDefault.fromTargetConfiguration(
+          BuildConfiguration.class,
+          ImmutableList.of(),
+          (rule, attributes, configuration) -> configuration.getActionListeners());
 
-  /**
-   * Implementation for the :run_under attribute.
-   */
-  public static final LateBoundLabel<BuildConfiguration> RUN_UNDER =
-      new LateBoundLabel<BuildConfiguration>() {
-        @Override
-        public Label resolve(Rule rule, AttributeMap attributes,
-            BuildConfiguration configuration) {
-          RunUnder runUnder = configuration.getRunUnder();
-          return runUnder == null ? null : runUnder.getLabel();
-        }
-      };
+  // TODO(b/65746853): provide a way to do this without passing the entire configuration
+  /** Implementation for the :run_under attribute. */
+  public static final LateBoundDefault<?, Label> RUN_UNDER =
+      LateBoundDefault.fromTargetConfiguration(
+          BuildConfiguration.class,
+          null,
+          (rule, attributes, configuration) -> {
+            RunUnder runUnder = configuration.getRunUnder();
+            return runUnder != null ? runUnder.getLabel() : null;
+          });
 
   /**
    * A base rule for all test rules.
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java
index 4997a895f8..855c985486 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java
@@ -13,6 +13,7 @@
 // limitations under the License.
 package com.google.devtools.build.lib.analysis;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
@@ -367,9 +368,7 @@ public abstract class DependencyResolver {
         continue;
       }
 
-      @SuppressWarnings("unchecked")
-      LateBoundDefault<BuildConfiguration> lateBoundDefault =
-        (LateBoundDefault<BuildConfiguration>) attribute.getLateBoundDefault();
+      LateBoundDefault<?, ?> lateBoundDefault = attribute.getLateBoundDefault();
 
       Collection<BuildOptions> splitOptions =
           getSplitOptions(depResolver.rule, attribute, ruleConfig);
@@ -452,20 +451,8 @@ public abstract class DependencyResolver {
       throws EvalException, InterruptedException {
     Preconditions.checkArgument(attribute.isLateBound());
 
-    @SuppressWarnings("unchecked")
-    LateBoundDefault<BuildConfiguration> lateBoundDefault =
-      (LateBoundDefault<BuildConfiguration>) attribute.getLateBoundDefault();
-
-    // TODO(bazel-team): This might be too expensive - can we cache this somehow?
-    if (!lateBoundDefault.getRequiredConfigurationFragments().isEmpty()) {
-      if (!config.hasAllFragments(lateBoundDefault.getRequiredConfigurationFragments())) {
-        return ImmutableList.<Label>of();
-      }
-    }
-
-    // TODO(bazel-team): We should check if the implementation tries to access an undeclared
-    // fragment.
-    Object actualValue = lateBoundDefault.resolve(rule, attributeMap, config);
+    Object actualValue =
+        resolveLateBoundDefault(attribute.getLateBoundDefault(), rule, attributeMap, config);
     if (EvalUtils.isNullOrNone(actualValue)) {
       return ImmutableList.<Label>of();
     }
@@ -495,6 +482,29 @@ public abstract class DependencyResolver {
     }
   }
 
+  @VisibleForTesting(/* used to test LateBoundDefaults' default values */ )
+  public static <FragmentT, ValueT> ValueT resolveLateBoundDefault(
+      LateBoundDefault<FragmentT, ValueT> lateBoundDefault,
+      Rule rule,
+      AttributeMap attributeMap,
+      BuildConfiguration config) {
+    Class<FragmentT> fragmentClass = lateBoundDefault.getFragmentClass();
+    // TODO(b/65746853): remove this when nothing uses it anymore
+    if (BuildConfiguration.class.equals(fragmentClass)) {
+      return lateBoundDefault.resolve(rule, attributeMap, fragmentClass.cast(config));
+    }
+    if (Void.class.equals(fragmentClass)) {
+      return lateBoundDefault.resolve(rule, attributeMap, null);
+    }
+    FragmentT fragment =
+        fragmentClass.cast(
+            config.getFragment((Class<? extends BuildConfiguration.Fragment>) fragmentClass));
+    if (fragment == null) {
+      return null;
+    }
+    return lateBoundDefault.resolve(rule, attributeMap, fragment);
+  }
+
   /**
    * Adds new dependencies to the given rule under the given attribute name
    *
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/DefaultsPackage.java b/src/main/java/com/google/devtools/build/lib/analysis/config/DefaultsPackage.java
index 8ddadf130f..07d8c755b6 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/DefaultsPackage.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/DefaultsPackage.java
@@ -21,7 +21,6 @@ import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy;
 import com.google.devtools.build.lib.util.Preconditions;
-
 import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
@@ -29,52 +28,41 @@ import java.util.Set;
 /**
  * A helper class to compute and inject a defaults package into the package cache.
  *
- * <p>The <code>//tools/defaults</code> package provides a mechanism let tool locations be
- * specified over the commandline, without requiring any special support in the rule code.
- * As such, it can be used in genrule <code>$(location)</code> substitutions.
+ * <p>The <code>//tools/defaults</code> package provides a mechanism let tool locations be specified
+ * over the commandline, without requiring any special support in the rule code. As such, it can be
+ * used in genrule <code>$(location)</code> substitutions.
  *
  * <p>It works as follows:
- * <ul>
- *
- *  <li> SomeLanguage.createCompileAction will refer to a host-configured target for the
- *  compiler by looking for
- *  <code>env.getHostPrerequisiteArtifact("$somelanguage_compiler")</code>.
- *
- *  <li> the attribute <code>$somelanguage_compiler</code> is defined in the
- *  {@link RuleDefinition} subclass for that language.
- *
- *  <li> if the attribute cannot be set on the command-line, its value may be a normal label.
- *
- *  <li> if the attribute can be set on the command-line, its value will be
- *  <code>//tools/defaults:somelanguage_compiler</code>.
- *
- *  <li> in the latter case, the {@link BuildConfiguration.Fragment} subclass will define the
- *  option (with an existing target, eg. <code>//third_party/somelanguage:compiler</code>), and
- *  return the name in its implementation of {@link FragmentOptions#getDefaultsLabels}.
- *
- *  <li> On startup, the rule is wired up with  <code>//tools/defaults:somelanguage_compiler</code>.
- *
- *  <li> On starting a build, the <code>//tools/defaults</code> package is synthesized, using
- *  the values as specified on the command-line. The contents of
- *  <code>tools/defaults/BUILD</code> is ignored.
- *
- *  <li> Hence, changes in the command line values for tools are now handled exactly as if they
- *  were changes in a BUILD file.
- *
- *  <li> The file <code>tools/defaults/BUILD</code> must exist, so we create a package in that
- *  location.
- *
- *  <li> The code in {@link DefaultsPackage} can dump the synthesized package as a BUILD file,
- * so external tooling does not need to understand the intricacies of handling command-line
- * options.
  *
+ * <ul>
+ *   <li>SomeLanguage.createCompileAction will refer to a host-configured target for the compiler by
+ *       looking for <code>env.getHostPrerequisiteArtifact("$somelanguage_compiler")</code>.
+ *   <li>the attribute <code>$somelanguage_compiler</code> is defined in the {@link RuleDefinition}
+ *       subclass for that language.
+ *   <li>if the attribute cannot be set on the command-line, its value may be a normal label.
+ *   <li>if the attribute can be set on the command-line, its value will be <code>
+ *       //tools/defaults:somelanguage_compiler</code>.
+ *   <li>in the latter case, the {@link BuildConfiguration.Fragment} subclass will define the option
+ *       (with an existing target, eg. <code>//third_party/somelanguage:compiler</code>), and return
+ *       the name in its implementation of {@link FragmentOptions#getDefaultsLabels}.
+ *   <li>On startup, the rule is wired up with <code>//tools/defaults:somelanguage_compiler</code>.
+ *   <li>On starting a build, the <code>//tools/defaults</code> package is synthesized, using the
+ *       values as specified on the command-line. The contents of <code>tools/defaults/BUILD</code>
+ *       is ignored.
+ *   <li>Hence, changes in the command line values for tools are now handled exactly as if they were
+ *       changes in a BUILD file.
+ *   <li>The file <code>tools/defaults/BUILD</code> must exist, so we create a package in that
+ *       location.
+ *   <li>The code in {@link DefaultsPackage} can dump the synthesized package as a BUILD file, so
+ *       external tooling does not need to understand the intricacies of handling command-line
+ *       options.
  * </ul>
  *
- * <p>For built-in rules (as opposed to genrules), late-bound labels provide an alternative
- * method of depending on command-line values. These work by declaring attribute default values
- * to be {@link LateBoundLabel} instances, whose <code>resolve(Rule rule, AttributeMap attributes,
- * T configuration)</code> method will have access to {@link BuildConfiguration}, which in turn
- * may depend on command line flag values.
+ * <p>For built-in rules (as opposed to genrules), late-bound labels provide an alternative method
+ * of depending on command-line values. These work by declaring attribute default values to be
+ * {@link LateBoundDefault} instances, whose <code>resolve(Rule rule, AttributeMap attributes,
+ * FragmentT configuration)</code> method will have access to a {@link BuildConfiguration.Fragment},
+ * which in turn may depend on command line flag values.
  */
 public final class DefaultsPackage {