Permit proto serialization of configured attribute values

This commit adds proto messages that represent configurable values,
and modifies attribute value serialization code to handle those
values, which are called SelectorLists.

--
MOS_MIGRATED_REVID=111149272

7 files changed, 744 insertions, 223 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/packages/AbstractAttributeMapper.java b/src/main/java/com/google/devtools/build/lib/packages/AbstractAttributeMapper.java
index 714086d19a..2c1837a836 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/AbstractAttributeMapper.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/AbstractAttributeMapper.java
@@ -93,6 +93,7 @@ public abstract class AbstractAttributeMapper implements AttributeMap {
    *         type. This happens whether or not it's a computed default.
    */
   @VisibleForTesting // Should be protected
+  @Nullable
   public <T> Attribute.ComputedDefault getComputedDefault(String attributeName, Type<T> type) {
     int index = getIndexWithTypeCheck(attributeName, type);
     Object value = attributes.getAttributeValue(index);
diff --git a/src/main/java/com/google/devtools/build/lib/packages/AggregatingAttributeMapper.java b/src/main/java/com/google/devtools/build/lib/packages/AggregatingAttributeMapper.java
index 7164bf6395..ea2a57e5d8 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/AggregatingAttributeMapper.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/AggregatingAttributeMapper.java
@@ -13,11 +13,34 @@
 // limitations under the License.
 package com.google.devtools.build.lib.packages;
 
+import static com.google.devtools.build.lib.packages.BuildType.DISTRIBUTIONS;
+import static com.google.devtools.build.lib.packages.BuildType.FILESET_ENTRY_LIST;
+import static com.google.devtools.build.lib.packages.BuildType.LABEL;
+import static com.google.devtools.build.lib.packages.BuildType.LABEL_DICT_UNARY;
+import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST;
+import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST_DICT;
+import static com.google.devtools.build.lib.packages.BuildType.LICENSE;
+import static com.google.devtools.build.lib.packages.BuildType.NODEP_LABEL;
+import static com.google.devtools.build.lib.packages.BuildType.NODEP_LABEL_LIST;
+import static com.google.devtools.build.lib.packages.BuildType.OUTPUT;
+import static com.google.devtools.build.lib.packages.BuildType.OUTPUT_LIST;
+import static com.google.devtools.build.lib.packages.BuildType.TRISTATE;
+import static com.google.devtools.build.lib.syntax.Type.BOOLEAN;
+import static com.google.devtools.build.lib.syntax.Type.INTEGER;
+import static com.google.devtools.build.lib.syntax.Type.INTEGER_LIST;
+import static com.google.devtools.build.lib.syntax.Type.STRING;
+import static com.google.devtools.build.lib.syntax.Type.STRING_DICT;
+import static com.google.devtools.build.lib.syntax.Type.STRING_DICT_UNARY;
+import static com.google.devtools.build.lib.syntax.Type.STRING_LIST;
+import static com.google.devtools.build.lib.syntax.Type.STRING_LIST_DICT;
+
 import com.google.common.base.Verify;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableList.Builder;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.collect.CollectionUtils;
 import com.google.devtools.build.lib.packages.BuildType.Selector;
@@ -26,11 +49,13 @@ import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.util.Preconditions;
 
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.LinkedHashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Set;
 
 import javax.annotation.Nullable;
@@ -41,6 +66,10 @@ import javax.annotation.Nullable;
  */
 public class AggregatingAttributeMapper extends AbstractAttributeMapper {
 
+  @SuppressWarnings("unchecked")
+  private static final ImmutableSet<Type<?>> scalarTypes =
+      ImmutableSet.of(INTEGER, STRING, LABEL, NODEP_LABEL, OUTPUT, BOOLEAN, TRISTATE, LICENSE);
+
   /**
    * Store for all of this rule's attributes that are non-configurable. These are
    * unconditionally  available to computed defaults no matter what dependencies
@@ -172,6 +201,106 @@ public class AggregatingAttributeMapper extends AbstractAttributeMapper {
   }
 
   /**
+   * Returns a list of the possible values of the specified attribute in the specified rule.
+   *
+   * <p>If the attribute's value is a simple value, then this returns a singleton list of that
+   * value.
+   *
+   * <p>If the attribute's value is an expression containing one or many {@code select(...)}
+   * expressions, then this returns a list of all values that expression may evaluate to.
+   *
+   * <p>If the attribute does not have an explicit value for this rule, and the rule provides a
+   * computed default, the computed default function is evaluated given the rule's other attribute
+   * values as inputs and the output is returned in a singleton list.
+   *
+   * <p>If the attribute does not have an explicit value for this rule, and the rule provides a
+   * computed default, and the computed default function depends on other attributes whose values
+   * contain {@code select(...)} expressions, then the computed default function is evaluated for
+   * every possible combination of input values, and the list of outputs is returned.
+   */
+  public Iterable<Object> getPossibleAttributeValues(Rule rule, Attribute attr) {
+    // Values may be null, so use normal collections rather than immutable collections.
+    // This special case for the visibility attribute is needed because its value is replaced
+    // with an empty list during package loading if it is public or private in order not to visit
+    // the package called 'visibility'.
+    if (attr.getName().equals("visibility")) {
+      List<Object> result = new ArrayList<>(1);
+      result.add(rule.getVisibility().getDeclaredLabels());
+      return result;
+    }
+    return Lists.<Object>newArrayList(visitAttribute(attr.getName(), attr.getType()));
+  }
+
+  /**
+   * Coerces the list {@param possibleValues} of values of type {@param attrType} to a single
+   * value of that type, in the following way:
+   *
+   * <p>If the list contains a single value, return that value.
+   *
+   * <p>If the list contains zero or multiple values and the type is a scalar type, return {@code
+   * null}.
+   *
+   * <p>If the list contains zero or multiple values and the type is a collection or map type,
+   * merge the collections/maps in the list and return the merged collection/map.
+   */
+  @Nullable
+  @SuppressWarnings("unchecked")
+  public static Object flattenAttributeValues(Type<?> attrType, Iterable<Object> possibleValues) {
+    // If there is only one possible value, return it.
+    if (Iterables.size(possibleValues) == 1) {
+      return Iterables.getOnlyElement(possibleValues);
+    }
+
+    // Otherwise, there are multiple possible values. To conform to the message shape expected by
+    // query output's clients, we must transform the list of possible values. This transformation
+    // will be lossy, but this is the best we can do.
+
+    // If the attribute's type is not a collection type, return null. Query output's clients do
+    // not support list values for scalar attributes.
+    if (scalarTypes.contains(attrType)) {
+      return null;
+    }
+
+    // If the attribute's type is a collection type, merge the list of collections into a single
+    // collection. This is a sensible solution for query output's clients, which are happy to get
+    // the union of possible values.
+    if (attrType == STRING_LIST
+        || attrType == LABEL_LIST
+        || attrType == NODEP_LABEL_LIST
+        || attrType == OUTPUT_LIST
+        || attrType == DISTRIBUTIONS
+        || attrType == INTEGER_LIST
+        || attrType == FILESET_ENTRY_LIST) {
+      Builder<Object> builder = ImmutableList.builder();
+      for (Object possibleValue : possibleValues) {
+        Collection<Object> collection = (Collection<Object>) possibleValue;
+        for (Object o : collection) {
+          builder.add(o);
+        }
+      }
+      return builder.build();
+    }
+
+    // Same for maps as for collections.
+    if (attrType == STRING_DICT
+        || attrType == STRING_DICT_UNARY
+        || attrType == STRING_LIST_DICT
+        || attrType == LABEL_DICT_UNARY
+        || attrType == LABEL_LIST_DICT) {
+      Map<Object, Object> mergedDict = new HashMap<>();
+      for (Object possibleValue : possibleValues) {
+        Map<Object, Object> stringDict = (Map<Object, Object>) possibleValue;
+        for (Entry<Object, Object> entry : stringDict.entrySet()) {
+          mergedDict.put(entry.getKey(), entry.getValue());
+        }
+      }
+      return mergedDict;
+    }
+
+    throw new AssertionError("Unknown type: " + attrType);
+  }
+
+  /**
    * Returns a list of all possible values an attribute can take for this rule.
    *
    * <p>Note that when an attribute uses multiple selects, it can potentially take on many
diff --git a/src/main/java/com/google/devtools/build/lib/packages/AttributeSerializer.java b/src/main/java/com/google/devtools/build/lib/packages/AttributeSerializer.java
index f380041402..915244990a 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/AttributeSerializer.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/AttributeSerializer.java
@@ -34,259 +34,258 @@ import static com.google.devtools.build.lib.syntax.Type.STRING_DICT_UNARY;
 import static com.google.devtools.build.lib.syntax.Type.STRING_LIST;
 import static com.google.devtools.build.lib.syntax.Type.STRING_LIST_DICT;
 
-import com.google.common.collect.Iterables;
-import com.google.common.collect.Lists;
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.packages.BuildType.Selector;
+import com.google.devtools.build.lib.packages.BuildType.SelectorList;
 import com.google.devtools.build.lib.query2.proto.proto2api.Build;
+import com.google.devtools.build.lib.query2.proto.proto2api.Build.Attribute.Discriminator;
+import com.google.devtools.build.lib.query2.proto.proto2api.Build.Attribute.SelectorEntry;
+import com.google.devtools.build.lib.query2.proto.proto2api.Build.Attribute.SelectorEntry.Builder;
+import com.google.devtools.build.lib.query2.proto.proto2api.Build.Attribute.Tristate;
+import com.google.devtools.build.lib.query2.proto.proto2api.Build.LabelDictUnaryEntry;
+import com.google.devtools.build.lib.query2.proto.proto2api.Build.LabelListDictEntry;
+import com.google.devtools.build.lib.query2.proto.proto2api.Build.StringDictEntry;
+import com.google.devtools.build.lib.query2.proto.proto2api.Build.StringDictUnaryEntry;
+import com.google.devtools.build.lib.query2.proto.proto2api.Build.StringListDictEntry;
 import com.google.devtools.build.lib.syntax.GlobCriteria;
 import com.google.devtools.build.lib.syntax.GlobList;
+import com.google.devtools.build.lib.syntax.Type;
 
-import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
+
+import javax.annotation.Nullable;
 
 /** Common utilities for serializing {@link Attribute}s as protocol buffers. */
 public class AttributeSerializer {
 
+  private static final ImmutableSet<Type<?>> depTypes =
+      ImmutableSet.<Type<?>>of(
+          STRING, LABEL, OUTPUT, STRING_LIST, LABEL_LIST, OUTPUT_LIST, DISTRIBUTIONS);
+
+  private static final ImmutableSet<Type<?>> noDepTypes =
+      ImmutableSet.<Type<?>>of(NODEP_LABEL_LIST, NODEP_LABEL);
+
   private AttributeSerializer() {}
 
   /**
-   * Returns the possible values of the specified attribute in the specified rule. For
-   * non-configured attributes, this is a single value. For configurable attributes, this
-   * may be multiple values.
+   * Convert attribute value to proto representation.
+   *
+   * <p>If {@param value} is null, only the {@code name}, {@code explicitlySpecified}, {@code
+   * nodep} (if applicable), and {@code type} fields will be included in the proto message.
+   *
+   * <p>If {@param includeGlobs} is true then {@link GlobCriteria} will be included in the proto
+   * message if present.
+   *
+   * <p>If {@param encodeBooleanAndTriStateAsIntegerAndString} is true then boolean and tristate
+   * values are also encoded as integers and strings.
    */
-  public static Iterable<Object> getAttributeValues(Rule rule, Attribute attr) {
-    // Values may be null, so use normal collections rather than immutable collections.
-    if (attr.getName().equals("visibility")) {
-      List<Object> result = new ArrayList<>(1);
-      result.add(rule.getVisibility().getDeclaredLabels());
-      return result;
+  public static Build.Attribute getAttributeProto(
+      Attribute attr,
+      @Nullable Object value,
+      boolean explicitlySpecified,
+      boolean includeGlobs,
+      boolean encodeBooleanAndTriStateAsIntegerAndString) {
+    Type<?> type = attr.getType();
+    Build.Attribute.Builder attrPb = Build.Attribute.newBuilder();
+    attrPb.setName(attr.getName());
+    attrPb.setExplicitlySpecified(explicitlySpecified);
+    maybeSetNoDep(type, attrPb);
+
+    if (value instanceof SelectorList<?>) {
+      attrPb.setType(Discriminator.SELECTOR_LIST);
+      writeSelectorListToBuilder(attrPb, type, (SelectorList<?>) value, includeGlobs);
     } else {
-      return Lists.<Object>newArrayList(
-          AggregatingAttributeMapper.of(rule).visitAttribute(attr.getName(), attr.getType()));
+      attrPb.setType(ProtoUtils.getDiscriminatorFromType(type));
+      if (value != null) {
+        AttributeBuilderAdapter adapter =
+            new AttributeBuilderAdapter(attrPb, encodeBooleanAndTriStateAsIntegerAndString);
+        writeAttributeValueToBuilder(adapter, type, value, includeGlobs);
+      }
     }
+
+    return attrPb.build();
   }
 
-  /**
-   * Convert Attribute to proto representation. If {@code includeGlobs} is true then include
-   * globs expressions when present, omit otherwise.
-   */
-  @SuppressWarnings("unchecked")
-  public static Build.Attribute getAttributeProto(
-      Attribute attr, Iterable<Object> values, boolean explicitlySpecified, boolean includeGlobs) {
-    // Get the attribute type.  We need to convert and add appropriately
-    com.google.devtools.build.lib.syntax.Type<?> type = attr.getType();
+  private static void maybeSetNoDep(Type<?> type, Build.Attribute.Builder attrPb) {
+    if (depTypes.contains(type)) {
+      attrPb.setNodep(false);
+    } else if (noDepTypes.contains(type)) {
+      attrPb.setNodep(true);
+    }
+  }
 
-    Build.Attribute.Builder attrPb = Build.Attribute.newBuilder();
+  private static void writeSelectorListToBuilder(
+      Build.Attribute.Builder attrPb,
+      Type<?> type,
+      SelectorList<?> selectorList,
+      boolean includeGlobs) {
+    Build.Attribute.SelectorList.Builder selectorListBuilder =
+        Build.Attribute.SelectorList.newBuilder();
+    selectorListBuilder.setType(ProtoUtils.getDiscriminatorFromType(type));
+    for (Selector<?> selector : selectorList.getSelectors()) {
+      Build.Attribute.Selector.Builder selectorBuilder = Build.Attribute.Selector.newBuilder();
+      // Note that the order of entries returned by selector.getEntries is stable. The map's
+      // entries' order is preserved from the sorting performed by the SelectorValue constructor.
+      for (Entry<Label, ?> entry : selector.getEntries().entrySet()) {
+        Builder selectorEntryBuilder = SelectorEntry.newBuilder();
+        selectorEntryBuilder.setLabel(entry.getKey().toString());
+        writeAttributeValueToBuilder(
+            new SelectorEntryBuilderAdapter(selectorEntryBuilder),
+            type,
+            entry.getValue(),
+            includeGlobs);
+        selectorBuilder.addEntries(selectorEntryBuilder);
+      }
 
-    // Set the type, name and source
-    attrPb.setName(attr.getName());
-    attrPb.setType(ProtoUtils.getDiscriminatorFromType(type));
-    attrPb.setExplicitlySpecified(explicitlySpecified);
+      selectorListBuilder.addElements(selectorBuilder);
+    }
+    attrPb.setSelectorList(selectorListBuilder);
+  }
 
-    // Convenience binding for single-value attributes. Because those attributes can only
-    // have a single value, when we encounter configurable versions of them we need to
-    // react somehow to having multiple possible values to report. We currently just
-    // refrain from setting *any* value in that scenario. This variable is set to null
-    // to indicate that.
-    //
-    // For example, for "linkstatic = select({':foo': 0, ':bar': 1})", "values" will contain [0, 1].
-    // Since linkstatic is a single-value string element, its proto field (string_value) can't
-    // store both values. Since no use case today actually needs this, we just skip it.
-    //
-    // TODO(bazel-team): support this properly. This will require syntactic change to build.proto
-    // (or reinterpretation of its current fields).
-    Object singleAttributeValue = Iterables.size(values) == 1
-        ? Iterables.getOnlyElement(values)
-        : null;
-
-    /*
-     * Set the appropriate type and value.  Since string and string list store
-     * values for multiple types, use the toString() method on the objects
-     * instead of casting them.  Note that Boolean and TriState attributes have
-     * both an integer and string representation.
-     */
+  /**
+   * Set the appropriate type and value. Since string and string list store values for multiple
+   * types, use the toString() method on the objects instead of casting them.
+   */
+  @SuppressWarnings("unchecked")
+  private static void writeAttributeValueToBuilder(
+      AttributeValueBuilderAdapter builder, Type<?> type, Object value, boolean includeGlobs) {
     if (type == INTEGER) {
-      if (singleAttributeValue != null) {
-        attrPb.setIntValue((Integer) singleAttributeValue);
-      }
+      builder.setIntValue((Integer) value);
     } else if (type == STRING || type == LABEL || type == NODEP_LABEL || type == OUTPUT) {
-      if (singleAttributeValue != null) {
-        attrPb.setStringValue(singleAttributeValue.toString());
-      }
-      attrPb.setNodep(type == NODEP_LABEL);
+      builder.setStringValue(value.toString());
     } else if (type == STRING_LIST || type == LABEL_LIST || type == NODEP_LABEL_LIST
         || type == OUTPUT_LIST || type == DISTRIBUTIONS) {
-      for (Object value : values) {
-        for (Object entry : (Collection<?>) value) {
-          attrPb.addStringListValue(entry.toString());
-        }
+      for (Object entry : (Collection<?>) value) {
+        builder.addStringListValue(entry.toString());
       }
-      attrPb.setNodep(type == NODEP_LABEL_LIST);
     } else if (type == INTEGER_LIST) {
-      for (Object value : values) {
-        for (Integer entry : (Collection<Integer>) value) {
-          attrPb.addIntListValue(entry);
-        }
+      for (Integer entry : (Collection<Integer>) value) {
+        builder.addIntListValue(entry);
       }
     } else if (type == BOOLEAN) {
-      if (singleAttributeValue != null) {
-        if ((Boolean) singleAttributeValue) {
-          attrPb.setStringValue("true");
-          attrPb.setBooleanValue(true);
-        } else {
-          attrPb.setStringValue("false");
-          attrPb.setBooleanValue(false);
-        }
-        // This maintains partial backward compatibility for external users of the
-        // protobuf that were expecting an integer field and not a true boolean.
-        attrPb.setIntValue((Boolean) singleAttributeValue ? 1 : 0);
-      }
+      builder.setBooleanValue((Boolean) value);
     } else if (type == TRISTATE) {
-      if (singleAttributeValue != null) {
-        switch ((TriState) singleAttributeValue) {
-          case AUTO:
-            attrPb.setIntValue(-1);
-            attrPb.setStringValue("auto");
-            attrPb.setTristateValue(Build.Attribute.Tristate.AUTO);
-            break;
-          case NO:
-            attrPb.setIntValue(0);
-            attrPb.setStringValue("no");
-            attrPb.setTristateValue(Build.Attribute.Tristate.NO);
-            break;
-          case YES:
-            attrPb.setIntValue(1);
-            attrPb.setStringValue("yes");
-            attrPb.setTristateValue(Build.Attribute.Tristate.YES);
-            break;
-          default:
-            throw new AssertionError("Expected AUTO/NO/YES to cover all possible cases");
-        }
-      }
+      builder.setTristateValue(triStateToProto((TriState) value));
     } else if (type == LICENSE) {
-      if (singleAttributeValue != null) {
-        License license = (License) singleAttributeValue;
-        Build.License.Builder licensePb = Build.License.newBuilder();
-        for (License.LicenseType licenseType : license.getLicenseTypes()) {
-          licensePb.addLicenseType(licenseType.toString());
-        }
-        for (Label exception : license.getExceptions()) {
-          licensePb.addException(exception.toString());
-        }
-        attrPb.setLicense(licensePb);
+      License license = (License) value;
+      Build.License.Builder licensePb = Build.License.newBuilder();
+      for (License.LicenseType licenseType : license.getLicenseTypes()) {
+        licensePb.addLicenseType(licenseType.toString());
+      }
+      for (Label exception : license.getExceptions()) {
+        licensePb.addException(exception.toString());
       }
+      builder.setLicense(licensePb);
     } else if (type == STRING_DICT) {
-      // TODO(bazel-team): support better de-duping here and in other dictionaries.
-      for (Object value : values) {
-        Map<String, String> dict = (Map<String, String>) value;
-        for (Map.Entry<String, String> keyValueList : dict.entrySet()) {
-          Build.StringDictEntry entry = Build.StringDictEntry.newBuilder()
-              .setKey(keyValueList.getKey())
-              .setValue(keyValueList.getValue())
-              .build();
-          attrPb.addStringDictValue(entry);
-        }
+      Map<String, String> dict = (Map<String, String>) value;
+      for (Map.Entry<String, String> keyValueList : dict.entrySet()) {
+        StringDictEntry.Builder entry =
+            StringDictEntry.newBuilder()
+                .setKey(keyValueList.getKey())
+                .setValue(keyValueList.getValue());
+        builder.addStringDictValue(entry);
       }
     } else if (type == STRING_DICT_UNARY) {
-      for (Object value : values) {
-        Map<String, String> dict = (Map<String, String>) value;
-        for (Map.Entry<String, String> dictEntry : dict.entrySet()) {
-          Build.StringDictUnaryEntry entry = Build.StringDictUnaryEntry.newBuilder()
-              .setKey(dictEntry.getKey())
-              .setValue(dictEntry.getValue())
-              .build();
-          attrPb.addStringDictUnaryValue(entry);
-        }
+      Map<String, String> dict = (Map<String, String>) value;
+      for (Map.Entry<String, String> dictEntry : dict.entrySet()) {
+        StringDictUnaryEntry.Builder entry =
+            StringDictUnaryEntry.newBuilder()
+                .setKey(dictEntry.getKey())
+                .setValue(dictEntry.getValue());
+        builder.addStringDictUnaryValue(entry);
       }
     } else if (type == STRING_LIST_DICT) {
-      for (Object value : values) {
-        Map<String, List<String>> dict = (Map<String, List<String>>) value;
-        for (Map.Entry<String, List<String>> dictEntry : dict.entrySet()) {
-          Build.StringListDictEntry.Builder entry = Build.StringListDictEntry.newBuilder()
-              .setKey(dictEntry.getKey());
-          for (Object dictEntryValue : dictEntry.getValue()) {
-            entry.addValue(dictEntryValue.toString());
-          }
-          attrPb.addStringListDictValue(entry);
+      Map<String, List<String>> dict = (Map<String, List<String>>) value;
+      for (Map.Entry<String, List<String>> dictEntry : dict.entrySet()) {
+        StringListDictEntry.Builder entry =
+            StringListDictEntry.newBuilder().setKey(dictEntry.getKey());
+        for (Object dictEntryValue : dictEntry.getValue()) {
+          entry.addValue(dictEntryValue.toString());
         }
+        builder.addStringListDictValue(entry);
       }
     } else if (type == LABEL_DICT_UNARY) {
-      for (Object value : values) {
-        Map<String, Label> dict = (Map<String, Label>) value;
-        for (Map.Entry<String, Label> dictEntry : dict.entrySet()) {
-          Build.LabelDictUnaryEntry entry = Build.LabelDictUnaryEntry.newBuilder()
-              .setKey(dictEntry.getKey())
-              .setValue(dictEntry.getValue().toString())
-              .build();
-          attrPb.addLabelDictUnaryValue(entry);
-        }
+      Map<String, Label> dict = (Map<String, Label>) value;
+      for (Map.Entry<String, Label> dictEntry : dict.entrySet()) {
+        LabelDictUnaryEntry.Builder entry =
+            LabelDictUnaryEntry.newBuilder()
+                .setKey(dictEntry.getKey())
+                .setValue(dictEntry.getValue().toString());
+        builder.addLabelDictUnaryValue(entry);
       }
     } else if (type == LABEL_LIST_DICT) {
-      for (Object value : values) {
-        Map<String, List<Label>> dict = (Map<String, List<Label>>) value;
-        for (Map.Entry<String, List<Label>> dictEntry : dict.entrySet()) {
-          Build.LabelListDictEntry.Builder entry = Build.LabelListDictEntry.newBuilder()
-              .setKey(dictEntry.getKey());
-          for (Object dictEntryValue : dictEntry.getValue()) {
-            entry.addValue(dictEntryValue.toString());
-          }
-          attrPb.addLabelListDictValue(entry);
+      Map<String, List<Label>> dict = (Map<String, List<Label>>) value;
+      for (Map.Entry<String, List<Label>> dictEntry : dict.entrySet()) {
+        LabelListDictEntry.Builder entry =
+            LabelListDictEntry.newBuilder().setKey(dictEntry.getKey());
+        for (Object dictEntryValue : dictEntry.getValue()) {
+          entry.addValue(dictEntryValue.toString());
         }
+        builder.addLabelListDictValue(entry);
       }
     } else if (type == FILESET_ENTRY_LIST) {
-      for (Object value : values) {
-        List<FilesetEntry> filesetEntries = (List<FilesetEntry>) value;
-        for (FilesetEntry filesetEntry : filesetEntries) {
-          Build.FilesetEntry.Builder filesetEntryPb = Build.FilesetEntry.newBuilder()
-              .setSource(filesetEntry.getSrcLabel().toString())
-              .setDestinationDirectory(filesetEntry.getDestDir().getPathString())
-              .setSymlinkBehavior(symlinkBehaviorToPb(filesetEntry.getSymlinkBehavior()))
-              .setStripPrefix(filesetEntry.getStripPrefix())
-              .setFilesPresent(filesetEntry.getFiles() != null);
-
-          if (filesetEntry.getFiles() != null) {
-            for (Label file : filesetEntry.getFiles()) {
-              filesetEntryPb.addFile(file.toString());
-            }
-          }
+      List<FilesetEntry> filesetEntries = (List<FilesetEntry>) value;
+      for (FilesetEntry filesetEntry : filesetEntries) {
+        Build.FilesetEntry.Builder filesetEntryPb =
+            Build.FilesetEntry.newBuilder()
+                .setSource(filesetEntry.getSrcLabel().toString())
+                .setDestinationDirectory(filesetEntry.getDestDir().getPathString())
+                .setSymlinkBehavior(symlinkBehaviorToPb(filesetEntry.getSymlinkBehavior()))
+                .setStripPrefix(filesetEntry.getStripPrefix())
+                .setFilesPresent(filesetEntry.getFiles() != null);
 
-          if (filesetEntry.getExcludes() != null) {
-            for (String exclude : filesetEntry.getExcludes()) {
-              filesetEntryPb.addExclude(exclude);
-            }
+        if (filesetEntry.getFiles() != null) {
+          for (Label file : filesetEntry.getFiles()) {
+            filesetEntryPb.addFile(file.toString());
           }
+        }
 
-          attrPb.addFilesetListValue(filesetEntryPb);
+        if (filesetEntry.getExcludes() != null) {
+          for (String exclude : filesetEntry.getExcludes()) {
+            filesetEntryPb.addExclude(exclude);
+          }
         }
+
+        builder.addFilesetListValue(filesetEntryPb);
       }
     } else {
       throw new AssertionError("Unknown type: " + type);
     }
 
-    if (includeGlobs) {
-      for (Object value : values) {
-        if (value instanceof GlobList<?>) {
-          GlobList<?> globList = (GlobList<?>) value;
+    if (includeGlobs && value instanceof GlobList<?>) {
+      GlobList<?> globList = (GlobList<?>) value;
 
-          for (GlobCriteria criteria : globList.getCriteria()) {
-            Build.GlobCriteria.Builder criteriaPb = Build.GlobCriteria.newBuilder()
-                .setGlob(criteria.isGlob());
-            for (String include : criteria.getIncludePatterns()) {
-              criteriaPb.addInclude(include);
-            }
-            for (String exclude : criteria.getExcludePatterns()) {
-              criteriaPb.addExclude(exclude);
-            }
-
-            attrPb.addGlobCriteria(criteriaPb);
-          }
+      for (GlobCriteria criteria : globList.getCriteria()) {
+        Build.GlobCriteria.Builder criteriaPb =
+            Build.GlobCriteria.newBuilder().setGlob(criteria.isGlob());
+        for (String include : criteria.getIncludePatterns()) {
+          criteriaPb.addInclude(include);
+        }
+        for (String exclude : criteria.getExcludePatterns()) {
+          criteriaPb.addExclude(exclude);
         }
+
+        builder.addGlobCriteria(criteriaPb);
       }
     }
+  }
 
-    return attrPb.build();
+  private static Tristate triStateToProto(TriState value) {
+    switch (value) {
+      case AUTO:
+        return Tristate.AUTO;
+      case NO:
+        return Tristate.NO;
+      case YES:
+        return Tristate.YES;
+      default:
+        throw new AssertionError("Expected AUTO/NO/YES to cover all possible cases");
+    }
   }
 
   // This is needed because I do not want to use the SymlinkBehavior from the
@@ -304,5 +303,252 @@ public class AttributeSerializer {
     }
   }
 
+  /**
+   * An adapter used by {@link #writeAttributeValueToBuilder} in order to reuse the same code for
+   * writing to both {@link Build.Attribute.Builder} and {@link SelectorEntry.Builder} objects.
+   */
+  private interface AttributeValueBuilderAdapter {
+
+    void addStringListValue(String s);
+
+    void addFilesetListValue(Build.FilesetEntry.Builder builder);
+
+    void addGlobCriteria(Build.GlobCriteria.Builder builder);
+
+    void addLabelDictUnaryValue(LabelDictUnaryEntry.Builder builder);
+
+    void addLabelListDictValue(LabelListDictEntry.Builder builder);
+
+    void addIntListValue(int i);
+
+    void addStringDictUnaryValue(StringDictUnaryEntry.Builder builder);
+
+    void addStringDictValue(StringDictEntry.Builder builder);
+
+    void addStringListDictValue(StringListDictEntry.Builder builder);
+
+    void setBooleanValue(boolean b);
+
+    void setIntValue(int i);
+
+    void setLicense(Build.License.Builder builder);
+
+    void setStringValue(String s);
+
+    void setTristateValue(Tristate tristate);
+  }
+
+  /**
+   * An {@link AttributeValueBuilderAdapter} which writes to a {@link Build.Attribute.Builder}.
+   *
+   * <p>If {@param encodeBooleanAndTriStateAsIntegerAndString} is {@code true}, then {@link
+   * Boolean} and {@link TriState} attribute values also write to the integer and string fields.
+   * This offers backwards compatibility to clients that expect attribute values of those types.
+   */
+  private static class AttributeBuilderAdapter implements AttributeValueBuilderAdapter {
+    private final boolean encodeBooleanAndTriStateAsIntegerAndString;
+    private final Build.Attribute.Builder attributeBuilder;
+
+    private AttributeBuilderAdapter(
+        Build.Attribute.Builder attributeBuilder,
+        boolean encodeBooleanAndTriStateAsIntegerAndString) {
+      this.attributeBuilder = Preconditions.checkNotNull(attributeBuilder);
+      this.encodeBooleanAndTriStateAsIntegerAndString = encodeBooleanAndTriStateAsIntegerAndString;
+    }
+
+    public void addStringListValue(String s) {
+      attributeBuilder.addStringListValue(s);
+    }
+
+    @Override
+    public void addFilesetListValue(Build.FilesetEntry.Builder builder) {
+      attributeBuilder.addFilesetListValue(builder);
+    }
+
+    @Override
+    public void addGlobCriteria(Build.GlobCriteria.Builder builder) {
+      attributeBuilder.addGlobCriteria(builder);
+    }
+
+    @Override
+    public void addLabelDictUnaryValue(LabelDictUnaryEntry.Builder builder) {
+      attributeBuilder.addLabelDictUnaryValue(builder);
+    }
+
+    @Override
+    public void addLabelListDictValue(LabelListDictEntry.Builder builder) {
+      attributeBuilder.addLabelListDictValue(builder);
+    }
+
+    @Override
+    public void addIntListValue(int i) {
+      attributeBuilder.addIntListValue(i);
+    }
+
+    @Override
+    public void addStringDictUnaryValue(StringDictUnaryEntry.Builder builder) {
+      attributeBuilder.addStringDictUnaryValue(builder);
+    }
+
+    @Override
+    public void addStringDictValue(StringDictEntry.Builder builder) {
+      attributeBuilder.addStringDictValue(builder);
+    }
+
+    @Override
+    public void addStringListDictValue(StringListDictEntry.Builder builder) {
+      attributeBuilder.addStringListDictValue(builder);
+    }
+
+    @Override
+    public void setBooleanValue(boolean b) {
+      if (b) {
+        attributeBuilder.setBooleanValue(true);
+        if (encodeBooleanAndTriStateAsIntegerAndString) {
+          attributeBuilder.setStringValue("true");
+          attributeBuilder.setIntValue(1);
+        }
+      } else {
+        attributeBuilder.setBooleanValue(false);
+        if (encodeBooleanAndTriStateAsIntegerAndString) {
+          attributeBuilder.setStringValue("false");
+          attributeBuilder.setIntValue(0);
+        }
+      }
+    }
+
+    @Override
+    public void setIntValue(int i) {
+      attributeBuilder.setIntValue(i);
+    }
+
+    @Override
+    public void setLicense(Build.License.Builder builder) {
+      attributeBuilder.setLicense(builder);
+    }
+
+    @Override
+    public void setStringValue(String s) {
+      attributeBuilder.setStringValue(s);
+    }
+
+    @Override
+    public void setTristateValue(Tristate tristate) {
+      switch (tristate) {
+        case AUTO:
+          attributeBuilder.setTristateValue(Tristate.AUTO);
+          if (encodeBooleanAndTriStateAsIntegerAndString) {
+            attributeBuilder.setIntValue(-1);
+            attributeBuilder.setStringValue("auto");
+          }
+          break;
+        case NO:
+          attributeBuilder.setTristateValue(Tristate.NO);
+          if (encodeBooleanAndTriStateAsIntegerAndString) {
+            attributeBuilder.setIntValue(0);
+            attributeBuilder.setStringValue("no");
+          }
+          break;
+        case YES:
+          attributeBuilder.setTristateValue(Tristate.YES);
+          if (encodeBooleanAndTriStateAsIntegerAndString) {
+            attributeBuilder.setIntValue(1);
+            attributeBuilder.setStringValue("yes");
+          }
+          break;
+        default:
+          throw new AssertionError("Expected AUTO/NO/YES to cover all possible cases");
+      }
+    }
+  }
+
+  /**
+   * An {@link AttributeValueBuilderAdapter} which writes to a {@link SelectorEntry.Builder}.
+   *
+   * <p>Note that there is no {@code encodeBooleanAndTriStateAsIntegerAndString} parameter needed
+   * here. This is because the clients that expect those alternate encodings of boolean and
+   * tristate attribute values do not support {@link SelectorList} values. When providing output to
+   * those clients, we compute the set of possible attribute values (expanding {@link SelectorList}
+   * values, evaluating computed defaults, and flattening collections of collections; see {@link
+   * com.google.devtools.build.lib.packages.AggregatingAttributeMapper#getPossibleAttributeValues}
+   * and {@link
+   * com.google.devtools.build.lib.packages.AggregatingAttributeMapper#flattenAttributeValues}).
+   */
+  private static class SelectorEntryBuilderAdapter implements AttributeValueBuilderAdapter {
+    private final SelectorEntry.Builder selectorEntryBuilder;
+
+    private SelectorEntryBuilderAdapter(Builder selectorEntryBuilder) {
+      this.selectorEntryBuilder = Preconditions.checkNotNull(selectorEntryBuilder);
+    }
+
+    public void addStringListValue(String s) {
+      selectorEntryBuilder.addStringListValue(s);
+    }
+
+    @Override
+    public void addFilesetListValue(Build.FilesetEntry.Builder builder) {
+      selectorEntryBuilder.addFilesetListValue(builder);
+    }
+
+    @Override
+    public void addGlobCriteria(Build.GlobCriteria.Builder builder) {
+      selectorEntryBuilder.addGlobCriteria(builder);
+    }
+
+    @Override
+    public void addLabelDictUnaryValue(LabelDictUnaryEntry.Builder builder) {
+      selectorEntryBuilder.addLabelDictUnaryValue(builder);
+    }
+
+    @Override
+    public void addLabelListDictValue(LabelListDictEntry.Builder builder) {
+      selectorEntryBuilder.addLabelListDictValue(builder);
+    }
+
+    @Override
+    public void addIntListValue(int i) {
+      selectorEntryBuilder.addIntListValue(i);
+    }
+
+    @Override
+    public void addStringDictUnaryValue(StringDictUnaryEntry.Builder builder) {
+      selectorEntryBuilder.addStringDictUnaryValue(builder);
+    }
+
+    @Override
+    public void addStringDictValue(StringDictEntry.Builder builder) {
+      selectorEntryBuilder.addStringDictValue(builder);
+    }
+
+    @Override
+    public void addStringListDictValue(StringListDictEntry.Builder builder) {
+      selectorEntryBuilder.addStringListDictValue(builder);
+    }
+
+    @Override
+    public void setBooleanValue(boolean b) {
+      selectorEntryBuilder.setBooleanValue(b);
+    }
+
+    @Override
+    public void setIntValue(int i) {
+      selectorEntryBuilder.setIntValue(i);
+    }
+
+    @Override
+    public void setLicense(Build.License.Builder builder) {
+      selectorEntryBuilder.setLicense(builder);
+    }
+
+    @Override
+    public void setStringValue(String s) {
+      selectorEntryBuilder.setStringValue(s);
+    }
+
+    @Override
+    public void setTristateValue(Tristate tristate) {
+      selectorEntryBuilder.setTristateValue(tristate);
+    }
+  }
 }
 
diff --git a/src/main/java/com/google/devtools/build/lib/packages/BuildType.java b/src/main/java/com/google/devtools/build/lib/packages/BuildType.java
index 831b279666..736f09eb40 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/BuildType.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/BuildType.java
@@ -28,7 +28,6 @@ import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.syntax.Type.ConversionException;
 import com.google.devtools.build.lib.syntax.Type.DictType;
 import com.google.devtools.build.lib.syntax.Type.ListType;
-import com.google.devtools.build.lib.util.Preconditions;
 
 import java.util.Collection;
 import java.util.Collections;
@@ -399,6 +398,11 @@ public final class BuildType {
       this.elements = builder.build();
     }
 
+    SelectorList(List<Selector<T>> elements, Type<T> originalType) {
+      this.elements = ImmutableList.copyOf(elements);
+      this.originalType = originalType;
+    }
+
     /**
      * Returns a syntactically order-preserved list of all values and selectors for this attribute.
      */
@@ -444,18 +448,16 @@ public final class BuildType {
         Label.parseAbsoluteUnchecked(DEFAULT_CONDITION_KEY);
 
     private final Type<T> originalType;
-    private final Map<Label, T> map;
+    private final ImmutableMap<Label, T> map;
     private final boolean hasDefaultCondition;
 
     @VisibleForTesting
-    Selector(Object x, String what, @Nullable Label context, Type<T> originalType)
+    Selector(ImmutableMap<?, ?> x, String what, @Nullable Label context, Type<T> originalType)
         throws ConversionException {
-      Preconditions.checkState(x instanceof Map<?, ?>);
-
       this.originalType = originalType;
       Map<Label, T> result = Maps.newLinkedHashMap();
       boolean foundDefaultCondition = false;
-      for (Entry<?, ?> entry : ((Map<?, ?>) x).entrySet()) {
+      for (Entry<?, ?> entry : x.entrySet()) {
         Label key = LABEL.convert(entry.getKey(), what, context);
         if (key.equals(DEFAULT_CONDITION_LABEL)) {
           foundDefaultCondition = true;
@@ -468,8 +470,11 @@ public final class BuildType {
 
     /**
      * Returns the selector's (configurability pattern --gt; matching values) map.
+     *
+     * <p>Entries in this map retain the order of the entries in the map provided to the {@link
+     * #Selector} constructor.
      */
-    public Map<Label, T> getEntries() {
+    public ImmutableMap<Label, T> getEntries() {
       return map;
     }
 
diff --git a/src/main/java/com/google/devtools/build/lib/packages/ProtoUtils.java b/src/main/java/com/google/devtools/build/lib/packages/ProtoUtils.java
index 619b0cb94e..f1e6d4fe58 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/ProtoUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/ProtoUtils.java
@@ -42,6 +42,7 @@ import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSetMultimap;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Sets;
+import com.google.devtools.build.lib.query2.proto.proto2api.Build;
 import com.google.devtools.build.lib.query2.proto.proto2api.Build.Attribute.Discriminator;
 import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.util.Preconditions;
@@ -91,30 +92,49 @@ public class ProtoUtils {
   static final ImmutableSetMultimap<Discriminator, Type<?>> INVERSE_TYPE_MAP =
       TYPE_MAP.asMultimap().inverse();
 
-  /**
-   * Returns the appropriate Attribute.Discriminator value from an internal attribute type.
-   */
-  public static Discriminator getDiscriminatorFromType(
-      Type<?> type) {
+  /** Returns the {@link Discriminator} value corresponding to the provided {@link Type}. */
+  public static Discriminator getDiscriminatorFromType(Type<?> type) {
     Preconditions.checkArgument(TYPE_MAP.containsKey(type), type);
     return TYPE_MAP.get(type);
   }
 
+  /** Returns the {@link Type} associated with an {@link Build.Attribute}. */
+  static Type<?> getTypeFromAttributePb(
+      Build.Attribute attrPb, String ruleClassName, String attrName) {
+    Optional<Boolean> nodepHint =
+        attrPb.hasNodep() ? Optional.of(attrPb.getNodep()) : Optional.<Boolean>absent();
+    Discriminator attrPbDiscriminator = attrPb.getType();
+    boolean isSelectorList = attrPbDiscriminator.equals(Discriminator.SELECTOR_LIST);
+    return getTypeFromDiscriminator(
+        isSelectorList ? attrPb.getSelectorList().getType() : attrPbDiscriminator,
+        nodepHint,
+        ruleClassName,
+        attrName);
+  }
+
   /**
-   * Returns the appropriate set of internal attribute types from an Attribute.Discriminator value.
+   * Returns the set of {@link Type}s associated with a {@link Discriminator} value.
+   *
+   * <p>The set will contain more than one {@link Type} when {@param discriminator} is either
+   * {@link Discriminator#STRING} or {@link Discriminator#STRING_LIST}, because each of them
+   * corresponds with two {@link Type} values. A nodeps hint is needed to determine which {@link
+   * Type} applies.
    */
-  public static ImmutableSet<Type<?>> getTypesFromDiscriminator(Discriminator discriminator) {
+  private static ImmutableSet<Type<?>> getTypesFromDiscriminator(Discriminator discriminator) {
     Preconditions.checkArgument(INVERSE_TYPE_MAP.containsKey(discriminator), discriminator);
     return INVERSE_TYPE_MAP.get(discriminator);
   }
 
   /**
-   * Returns the appropriate internal attribute type from an Attribute.Discriminator value, given
-   * an optional nodeps hint.
+   * Returns the {@link Type} associated with a {@link Discriminator} value, given an optional
+   * nodeps hint.
    */
-  public static Type<?> getTypeFromDiscriminator(Discriminator discriminator,
-      Optional<Boolean> nodeps, String ruleClassName, String attrName) {
-    Preconditions.checkArgument(INVERSE_TYPE_MAP.containsKey(discriminator));
+  private static Type<?> getTypeFromDiscriminator(
+      Discriminator discriminator,
+      Optional<Boolean> nodeps,
+      String ruleClassName,
+      String attrName) {
+    Preconditions.checkArgument(INVERSE_TYPE_MAP.containsKey(discriminator), discriminator);
     ImmutableSet<Type<?>> possibleTypes = ProtoUtils.getTypesFromDiscriminator(discriminator);
     Type<?> preciseType;
     if (possibleTypes.size() == 1) {
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RawAttributeMapper.java b/src/main/java/com/google/devtools/build/lib/packages/RawAttributeMapper.java
index 164b9d37f6..cbe270238a 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RawAttributeMapper.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RawAttributeMapper.java
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.packages;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.packages.Attribute.ComputedDefault;
 import com.google.devtools.build.lib.packages.BuildType.Selector;
 import com.google.devtools.build.lib.packages.BuildType.SelectorList;
 import com.google.devtools.build.lib.syntax.Type;
@@ -27,19 +28,23 @@ import java.util.List;
 import javax.annotation.Nullable;
 
 /**
- * {@link AttributeMap} implementation that returns raw attribute information as contained
- * within a {@link Rule}. In particular, configurable attributes of the form
- * { config1: "value1", config2: "value2" } are passed through without being resolved to a
- * final value.
+ * {@link AttributeMap} implementation that returns raw attribute information as contained within
+ * a {@link Rule} via {@link #getRawAttributeValue}. In particular, configurable attributes
+ * of the form { config1: "value1", config2: "value2" } are passed through without being resolved
+ * to a final value when obtained via that method.
  */
 public class RawAttributeMapper extends AbstractAttributeMapper {
-  RawAttributeMapper(Package pkg, RuleClass ruleClass, Label ruleLabel,
-      AttributeContainer attributes) {
+
+  RawAttributeMapper(
+      Package pkg, RuleClass ruleClass, Label ruleLabel, AttributeContainer attributes) {
     super(pkg, ruleClass, ruleLabel, attributes);
   }
 
   public static RawAttributeMapper of(Rule rule) {
-    return new RawAttributeMapper(rule.getPackage(), rule.getRuleClassObject(), rule.getLabel(),
+    return new RawAttributeMapper(
+        rule.getPackage(),
+        rule.getRuleClassObject(),
+        rule.getLabel(),
         rule.getAttributeContainer());
   }
 
@@ -92,4 +97,61 @@ public class RawAttributeMapper extends AbstractAttributeMapper {
     }
     return builder.build();
   }
+
+  /**
+   * See {@link #getRawAttributeValue(Rule, Attribute)}.
+   *
+   * <p>{@param attrName} must be the name of an {@link Attribute} defined by the {@param rule}'s
+   * {@link RuleClass}.
+   */
+  @Nullable
+  public Object getRawAttributeValue(Rule rule, String attrName) {
+    Attribute attr =
+        Preconditions.checkNotNull(getAttributeDefinition(attrName), "%s %s", rule, attrName);
+    return getRawAttributeValue(rule, attr);
+  }
+
+  /**
+   * Returns the object associated with the {@param rule}'s {@param attr}.
+   *
+   * <p>Handles the special case of the "visibility" attribute by returning {@param rule}'s {@link
+   * RuleVisibility}'s declared labels.
+   *
+   * <p>The returned object will be a {@link SelectorList} if the attribute value contains a
+   * {@code select(...)} expression.
+   *
+   * <p>The returned object will be a {@link ComputedDefault} if the rule doesn't explicitly
+   * declare an attribute value and the rule's class provides a computed default for it.
+   *
+   * <p>Otherwise, the returned object will be the type declared by the {@param attr}, or {@code
+   * null}.
+   */
+  @Nullable
+  public Object getRawAttributeValue(Rule rule, Attribute attr) {
+    // This special case for the visibility attribute is needed because its value is replaced
+    // with an empty list during package loading if it is public or private in order not to visit
+    // the package called 'visibility'.
+    if (attr.getName().equals("visibility")) {
+      return rule.getVisibility().getDeclaredLabels();
+    }
+
+    // If the attribute value contains one or more select(...) expressions, then return
+    // the SelectorList object representing those expressions.
+    SelectorList<?> selectorList = getSelectorList(attr.getName(), attr.getType());
+    if (selectorList != null) {
+      return selectorList;
+    }
+
+    // If the attribute value is not explicitly declared, and the rule class provides a computed
+    // default value for it, then we should return the ComputedDefault object.
+    //
+    // We check for the existence of a ComputedDefault value because AbstractAttributeMapper#get
+    // returns either an explicitly declared attribute value or the result of evaluating the
+    // computed default function, but does not specify which one it is.
+    ComputedDefault computedDefault = getComputedDefault(attr.getName(), attr.getType());
+    if (computedDefault != null) {
+      return computedDefault;
+    }
+    return get(attr.getName(), attr.getType());
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleSerializer.java b/src/main/java/com/google/devtools/build/lib/packages/RuleSerializer.java
index 5d49b4b420..658515cb49 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleSerializer.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleSerializer.java
@@ -13,23 +13,81 @@
 // limitations under the License.
 package com.google.devtools.build.lib.packages;
 
+import com.google.common.collect.ImmutableSet;
+import com.google.devtools.build.lib.packages.Attribute.ComputedDefault;
 import com.google.devtools.build.lib.query2.proto.proto2api.Build;
+import com.google.devtools.build.lib.util.Preconditions;
 
 /** Serialize a {@link Rule} as its protobuf representation. */
 public class RuleSerializer {
 
+  // Skylark doesn't support defining rule classes with ComputedDefault attributes. Therefore, the
+  // only ComputedDefault attributes we expect to see for Skylark-defined rule classes are
+  // those declared in those rule classes' natively defined base rule classes, which are:
+  //
+  // 1. The "timeout" attribute in SkylarkRuleClassFunctions.testBaseRule
+  // 2. The "deprecation" attribute in BaseRuleClasses.commonCoreAndSkylarkAttributes
+  // 3. The "testonly" attribute in BaseRuleClasses.commonCoreAndSkylarkAttributes
+  private static final ImmutableSet<String> SKYLARK_RULE_CLASS_COMPUTED_DEFAULT_ATTRIBUTES =
+      ImmutableSet.of("timeout", "deprecation", "testonly");
+
   public static Build.Rule.Builder serializeRule(Rule rule) {
     Build.Rule.Builder builder = Build.Rule.newBuilder();
     builder.setName(rule.getLabel().getName());
     builder.setRuleClass(rule.getRuleClass());
     builder.setPublicByDefault(rule.getRuleClassObject().isPublicByDefault());
-    for (Attribute attribute : rule.getAttributes()) {
+    RawAttributeMapper rawAttributeMapper = RawAttributeMapper.of(rule);
+    for (Attribute attr : rule.getAttributes()) {
+      Object rawAttributeValue = rawAttributeMapper.getRawAttributeValue(rule, attr);
+
+      Object valueToSerialize;
+      if (rawAttributeValue instanceof ComputedDefault) {
+        if (rule.getRuleClassObject().isSkylark()) {
+          // If the rule class is Skylark-defined (i.e. rule.getRuleClassObject().isSkylark() is
+          // true), and the attribute has a ComputedDefault value, we must serialize it. The
+          // Skylark-defined ComputedDefault function won't be available after deserialization due
+          // to Skylark's non-serializability. Fortunately (from the perspective of rule
+          // serialization), Skylark doesn't support defining rule classes with ComputedDefault
+          // attributes, and so the only ComputedDefault attributes we need to worry about for
+          // Skylark-defined rule classes are those declared in those rule classes' natively
+          // defined base rule classes.
+          //
+          // See the comment for SKYLARK_RULE_CLASS_COMPUTED_DEFAULT_ATTRIBUTES for the locations
+          // of these expected attributes.
+          //
+          // The RawAttributeMapper#get method, inherited from AbstractAttributeMapper, evaluates
+          // the ComputedDefault function, so we use that, after verifying the attribute's name is
+          // expected.
+          Preconditions.checkState(
+              SKYLARK_RULE_CLASS_COMPUTED_DEFAULT_ATTRIBUTES.contains(attr.getName()),
+              "Unexpected ComputedDefault value for %s in %s",
+              attr,
+              rule);
+          valueToSerialize = rawAttributeMapper.get(attr.getName(), attr.getType());
+        } else {
+          // If the rule class is native (i.e. not Skylark-defined), we can skip serialization of
+          // attributes with ComputedDefault values. The native rule class can provide the same
+          // ComputedDefault value for the attribute after deserialization.
+          //
+          // TODO(mschaller): While the native rule class *could* provide it, it doesn't yet. Make
+          // it so! For now, we fall back to flattening the set of all possible values, computed
+          // using AggregatingAttributeMapper.
+          Iterable<Object> possibleValues =
+              AggregatingAttributeMapper.of(rule).getPossibleAttributeValues(rule, attr);
+          valueToSerialize =
+              AggregatingAttributeMapper.flattenAttributeValues(attr.getType(), possibleValues);
+        }
+      } else {
+        valueToSerialize = rawAttributeValue;
+      }
+
       builder.addAttribute(
           AttributeSerializer.getAttributeProto(
-              attribute,
-              AttributeSerializer.getAttributeValues(rule, attribute),
-              rule.isAttributeValueExplicitlySpecified(attribute),
-              /*includeGlobs=*/ true));
+              attr,
+              valueToSerialize,
+              rule.isAttributeValueExplicitlySpecified(attr),
+              /*includeGlobs=*/ true,
+              /*encodeBooleanAndTriStateAsIntegerAndString=*/ false));
     }
     return builder;
   }