Simplify split transition semantics: noops now return the input build options. This results in less special logic in the implementation and a simpler API. PiperOrigin-RevId: 197772283

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 dd34f19..a72b346 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

@@ -403,8 +403,15 @@
 
       LateBoundDefault<?, ?> lateBoundDefault = attribute.getLateBoundDefault();
 
-      Collection<BuildOptions> splitOptions = getSplitOptions(attributeMap, attribute, ruleConfig);
-      if (!splitOptions.isEmpty() && !ruleConfig.isHostConfiguration()) {
+      boolean hasSplitTransition = false;
+      List<BuildOptions> splitOptions = null;
+      if (attribute.hasSplitConfigurationTransition()) {
+        splitOptions =
+            attribute.getSplitTransition(attributeMap).checkedSplit(ruleConfig.getOptions());
+        hasSplitTransition = !SplitTransition.equals(ruleConfig.getOptions(), splitOptions);
+      }
+
+      if (hasSplitTransition && !ruleConfig.isHostConfiguration()) {
         // Late-bound attribute with a split transition:
         // Since we want to get the same results as TransitionResolver.evaluateTransition (but
         // skip it since we've already applied the split), we want to make sure this logic
@@ -449,24 +456,6 @@
   }
 
   /**
-   * Returns the BuildOptions if the rule's attribute triggers a split in this configuration, or
-   * the empty collection if the attribute does not trigger a split transition or if the split
-   * transition does not apply.
-   *
-   * <p>Even though the attribute may have a split, splits don't have to apply in every
-   * configuration (see {@link SplitTransition#split}).
-   */
-  private static Collection<BuildOptions> getSplitOptions(ConfiguredAttributeMapper attributeMap,
-      Attribute attribute,
-      BuildConfiguration ruleConfig) {
-    if (!attribute.hasSplitConfigurationTransition()) {
-      return ImmutableList.<BuildOptions>of();
-    }
-    SplitTransition transition = attribute.getSplitTransition(attributeMap);
-    return transition.split(ruleConfig.getOptions());
-  }
-
-  /**
    * Returns the label dependencies for the given late-bound attribute in this rule.
    *
    * @param rule the rule being evaluated

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
index adfeb58..fb17814 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java

@@ -790,10 +790,11 @@
     SplitTransition transition =
         attributeDefinition.getSplitTransition(
             ConfiguredAttributeMapper.of(rule, configConditions));
-    List<BuildOptions> splitOptions = transition.split(getConfiguration().getOptions());
+    BuildOptions fromOptions = getConfiguration().getOptions();
+    List<BuildOptions> splitOptions = transition.checkedSplit(fromOptions);
     List<ConfiguredTargetAndData> deps = getConfiguredTargetAndTargetDeps(attributeName);
 
-    if (splitOptions.isEmpty()) {
+    if (SplitTransition.equals(fromOptions, splitOptions)) {
       // The split transition is not active. Defer the decision on which CPU to use.
       return ImmutableMap.of(Optional.<String>absent(), deps);
     }

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java
index 677c028..531c6f1 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java

@@ -261,7 +261,7 @@
     // resolveAspectDependencies don't get a chance to make their own Skyframe requests before
     // bailing out of this ConfiguredTargetFunction call. Ideally we could batch all requests
     // from all methods into a single Skyframe call, but there are enough subtle data flow
-    // dependencies in ConfiguredTargetFucntion to make that impractical.
+    // dependencies in ConfiguredTargetFunction to make that impractical.
     Map<SkyKey, ValueOrException<InvalidConfigurationException>> depConfigValues =
         env.getValuesOrThrow(keysToEntries.keySet(), InvalidConfigurationException.class);
 
@@ -429,14 +429,7 @@
       // TODO(bazel-team): safety-check that this never mutates fromOptions.
       result = ImmutableList.of(((PatchTransition) transition).patch(fromOptions));
     } else if (transition instanceof SplitTransition) {
-      List<BuildOptions> toOptions = ((SplitTransition) transition).split(fromOptions);
-      if (toOptions.isEmpty()) {
-        // When the split returns an empty list, it's signaling it doesn't apply to this instance.
-        // So return the original options.
-        result = ImmutableList.<BuildOptions>of(fromOptions);
-      } else {
-        result = toOptions;
-      }
+      return ((SplitTransition) transition).checkedSplit(fromOptions);
     } else {
       throw new IllegalStateException(String.format(
           "unsupported config transition type: %s", transition.getClass().getName()));

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ComposingSplitTransition.java b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ComposingSplitTransition.java
index 69970c2..71e3770 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ComposingSplitTransition.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ComposingSplitTransition.java

@@ -120,13 +120,7 @@
     if (transition instanceof PatchTransition) {
       return ImmutableList.<BuildOptions>of(((PatchTransition) transition).patch(fromOptions));
     } else if (transition instanceof SplitTransition) {
-      SplitTransition split = (SplitTransition) transition;
-      List<BuildOptions> splitOptions = split.split(fromOptions);
-      if (splitOptions.isEmpty()) {
-        return ImmutableList.<BuildOptions>of(fromOptions);
-      } else {
-        return splitOptions;
-      }
+      return ((SplitTransition) transition).checkedSplit(fromOptions);
     } else {
       throw new IllegalStateException(
           String.format("Unsupported composite transition type: %s",

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/SplitTransition.java b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/SplitTransition.java
index 05ecfaa..eb71869 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/SplitTransition.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/SplitTransition.java

@@ -14,6 +14,8 @@
 
 package com.google.devtools.build.lib.analysis.config.transitions;
 
+import com.google.common.base.Verify;
+import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.concurrent.ThreadSafety;
 import java.util.List;
@@ -27,7 +29,29 @@
 @FunctionalInterface
 public interface SplitTransition extends ConfigurationTransition {
   /**
-   * Return the list of {@code BuildOptions} after splitting; empty if not applicable.
+   * Returns the list of {@code BuildOptions} after splitting, or the original options if this
+   * split is a noop.
+   *
+   * <p>Returning an empty or null list triggers a {@link RuntimeException}.
    */
   List<BuildOptions> split(BuildOptions buildOptions);
+
+  /**
+   * Calls {@link #split} and throws a {@link RuntimeException} if the split output is empty.
+   */
+  default List<BuildOptions> checkedSplit(BuildOptions buildOptions) {
+    List<BuildOptions> splitOptions = split(buildOptions);
+    Verify.verifyNotNull(splitOptions, "Split transition output may not be null");
+    Verify.verify(!splitOptions.isEmpty(), "Split transition output may not be empty");
+    return splitOptions;
+  }
+
+  /**
+   * Returns true iff {@code option} and {@splitOptions} are equal.
+   *
+   * <p>This can be used to determine if a split is a noop.
+   */
+  static boolean equals(BuildOptions options, List<BuildOptions> splitOptions) {
+    return splitOptions.size() == 1 && Iterables.getOnlyElement(splitOptions).equals(options);
+  }
 }

diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java
index 2035a79..815616d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java

@@ -238,7 +238,7 @@
         if (androidOptions.cpu.isEmpty()
             || androidCrosstoolTop == null
             || androidCrosstoolTop.equals(cppOptions.crosstoolTop)) {
-          return ImmutableList.of();
+          return ImmutableList.of(buildOptions);
 
         } else {