Generate a CODEC for CppCompilationContext using AutoCodec. This is required in order to serialize CppCompileAction.

Rephrase CppCompilationContext's pregreppedHeaders field as its own value class instead of Pair<Artifact, Artifact>.  We do this because NestedSet support in AutoCodec cannot serialize a NestedSet of a generic type.

PiperOrigin-RevId: 184740075

6 files changed, 112 insertions, 48 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/BUILD b/src/main/java/com/google/devtools/build/lib/rules/cpp/BUILD
index 89554f1670..52f40ee300 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/BUILD
@@ -27,6 +27,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/analysis/skylark/annotations",
         "//src/main/java/com/google/devtools/build/lib/collect",
         "//src/main/java/com/google/devtools/build/lib/collect/nestedset",
+        "//src/main/java/com/google/devtools/build/lib/collect/nestedset:serialization",
         "//src/main/java/com/google/devtools/build/lib/concurrent",
         "//src/main/java/com/google/devtools/build/lib/profiler",
         "//src/main/java/com/google/devtools/build/lib/rules/apple",
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java
index 1dfa570b35..f4d62f1951 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java
@@ -1493,11 +1493,11 @@ public final class CcLibraryHelper {
     contextBuilder.addModularHdrs(publicHeaders.getHeaders());
     contextBuilder.addModularHdrs(privateHeaders);
     contextBuilder.addTextualHdrs(publicTextualHeaders);
-    contextBuilder.addPregreppedHeaderMap(
+    contextBuilder.addPregreppedHeaders(
         CppHelper.createExtractInclusions(ruleContext, semantics, publicHeaders.getHeaders()));
-    contextBuilder.addPregreppedHeaderMap(
+    contextBuilder.addPregreppedHeaders(
         CppHelper.createExtractInclusions(ruleContext, semantics, publicTextualHeaders));
-    contextBuilder.addPregreppedHeaderMap(
+    contextBuilder.addPregreppedHeaders(
         CppHelper.createExtractInclusions(ruleContext, semantics, privateHeaders));
 
     // Add this package's dir to declaredIncludeDirs, & this rule's headers to declaredIncludeSrcs
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompilationContext.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompilationContext.java
index 5b9059001a..da8827d6ac 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompilationContext.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompilationContext.java
@@ -28,22 +28,30 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
-import com.google.devtools.build.lib.util.Pair;
+import com.google.devtools.build.lib.rules.cpp.CppHelper.PregreppedHeader;
+import com.google.devtools.build.lib.skyframe.serialization.InjectingObjectCodec;
+import com.google.devtools.build.lib.skyframe.serialization.ObjectCodec;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization;
+import com.google.devtools.build.lib.vfs.FileSystemProvider;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.LinkedHashSet;
 import java.util.List;
-import java.util.Map;
 import java.util.Set;
+import java.util.stream.Collectors;
 import javax.annotation.Nullable;
 
 /**
- * Immutable store of information needed for C++ compilation that is aggregated
- * across dependencies.
+ * Immutable store of information needed for C++ compilation that is aggregated across dependencies.
  */
 @Immutable
+@AutoCodec(dependency = FileSystemProvider.class)
 public final class CppCompilationContext implements TransitiveInfoProvider {
+  public static final InjectingObjectCodec<CppCompilationContext, FileSystemProvider> CODEC =
+      new CppCompilationContext_AutoCodec();
+
   /** An empty compilation context. */
   public static final CppCompilationContext EMPTY = new Builder(null).build();
 
@@ -61,8 +69,8 @@ public final class CppCompilationContext implements TransitiveInfoProvider {
   /** Non-code mandatory compilation inputs. */
   private final NestedSet<Artifact> nonCodeInputs;
 
-  private final NestedSet<Pair<Artifact, Artifact>> pregreppedHdrs;
-  
+  private final NestedSet<PregreppedHeader> pregreppedHdrs;
+
   private final ModuleInfo moduleInfo;
   private final ModuleInfo picModuleInfo;
 
@@ -74,13 +82,15 @@ public final class CppCompilationContext implements TransitiveInfoProvider {
   // Derived from depsContexts.
   private final ImmutableSet<Artifact> compilationPrerequisites;
 
-  private CppCompilationContext(
+  @AutoCodec.Instantiator
+  @VisibleForSerialization
+  CppCompilationContext(
       CommandLineContext commandLineContext,
       ImmutableSet<Artifact> compilationPrerequisites,
       NestedSet<PathFragment> declaredIncludeDirs,
       NestedSet<PathFragment> declaredIncludeWarnDirs,
       NestedSet<Artifact> declaredIncludeSrcs,
-      NestedSet<Pair<Artifact, Artifact>> pregreppedHdrs,
+      NestedSet<PregreppedHeader> pregreppedHdrs,
       NestedSet<Artifact> nonCodeInputs,
       ModuleInfo moduleInfo,
       ModuleInfo picModuleInfo,
@@ -202,10 +212,10 @@ public final class CppCompilationContext implements TransitiveInfoProvider {
   }
 
   /**
-   * Returns the immutable pairs of (header file, pregrepped header file).  The value artifacts
+   * Returns the immutable pairs of (header file, pregrepped header file). The value artifacts
    * (pregrepped header file) are generated by {@link ExtractInclusionAction}.
    */
-  NestedSet<Pair<Artifact, Artifact>> getPregreppedHeaders() {
+  NestedSet<PregreppedHeader> getPregreppedHeaders() {
     return pregreppedHdrs;
   }
 
@@ -355,11 +365,15 @@ public final class CppCompilationContext implements TransitiveInfoProvider {
   }
 
   /**
-   * The parts of the compilation context that influence the command line of
-   * compilation actions.
+   * The parts of the compilation context that influence the command line of compilation actions.
    */
   @Immutable
-  private static class CommandLineContext {
+  @AutoCodec
+  @VisibleForSerialization
+  static class CommandLineContext {
+    public static final ObjectCodec<CommandLineContext> CODEC =
+        new CppCompilationContext_CommandLineContext_AutoCodec();
+
     private final ImmutableList<PathFragment> includeDirs;
     private final ImmutableList<PathFragment> quoteIncludeDirs;
     private final ImmutableList<PathFragment> systemIncludeDirs;
@@ -391,7 +405,7 @@ public final class CppCompilationContext implements TransitiveInfoProvider {
         NestedSetBuilder.stableOrder();
     private final NestedSetBuilder<Artifact> declaredIncludeSrcs =
         NestedSetBuilder.stableOrder();
-    private final NestedSetBuilder<Pair<Artifact, Artifact>> pregreppedHdrs =
+    private final NestedSetBuilder<PregreppedHeader> pregreppedHdrs =
         NestedSetBuilder.stableOrder();
     private final NestedSetBuilder<Artifact> nonCodeInputs = NestedSetBuilder.stableOrder();
     private final ModuleInfo.Builder moduleInfo = new ModuleInfo.Builder();
@@ -595,14 +609,16 @@ public final class CppCompilationContext implements TransitiveInfoProvider {
     }
 
     /**
-     * Add a map of generated source or header Artifact to an output Artifact after grepping
-     * the file for include statements.
+     * Add a map of generated source or header Artifact to an output Artifact after grepping the
+     * file for include statements.
      */
-    public Builder addPregreppedHeaderMap(Map<Artifact, Artifact> pregrepped) {
-      addCompilationPrerequisites(pregrepped.values());
-      for (Map.Entry<Artifact, Artifact> entry : pregrepped.entrySet()) {
-        this.pregreppedHdrs.add(Pair.of(entry.getKey(), entry.getValue()));
-      }
+    public Builder addPregreppedHeaders(List<PregreppedHeader> pregrepped) {
+      addCompilationPrerequisites(
+          pregrepped
+              .stream()
+              .map(pregreppedHeader -> pregreppedHeader.greppedHeader())
+              .collect(Collectors.toList()));
+      this.pregreppedHdrs.addAll(pregrepped);
       return this;
     }
 
@@ -760,13 +776,17 @@ public final class CppCompilationContext implements TransitiveInfoProvider {
               ruleContext.getRule().getRepository()));
     }
   }
-  
+
   /**
    * Gathers data about the direct and transitive .pcm files belonging to this context. Can be to
    * either gather data on PIC or on no-PIC .pcm files.
    */
   @Immutable
+  @AutoCodec(dependency = FileSystemProvider.class)
   public static final class ModuleInfo {
+    public static final InjectingObjectCodec<ModuleInfo, FileSystemProvider> CODEC =
+        new CppCompilationContext_ModuleInfo_AutoCodec();
+
     /**
      * The module built for this context. If null, then no module is being compiled for this
      * context.
@@ -896,11 +916,13 @@ public final class CppCompilationContext implements TransitiveInfoProvider {
     }
   }
 
-  /**
-   * Collects data for a specific module in a special format that makes pruning easy.
-   */
+  /** Collects data for a specific module in a special format that makes pruning easy. */
   @Immutable
+  @AutoCodec(dependency = FileSystemProvider.class)
   public static final class TransitiveModuleHeaders {
+    public static final InjectingObjectCodec<TransitiveModuleHeaders, FileSystemProvider> CODEC =
+        new CppCompilationContext_TransitiveModuleHeaders_AutoCodec();
+
     /**
      * The module that we are calculating information for.
      */
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
index d292ceb89e..1968bb9083 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
@@ -57,9 +57,9 @@ import com.google.devtools.build.lib.profiler.ProfilerTask;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
 import com.google.devtools.build.lib.rules.cpp.CppCompileActionContext.Reply;
 import com.google.devtools.build.lib.rules.cpp.CppConfiguration.Tool;
+import com.google.devtools.build.lib.rules.cpp.CppHelper.PregreppedHeader;
 import com.google.devtools.build.lib.util.DependencySet;
 import com.google.devtools.build.lib.util.Fingerprint;
-import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.util.ShellEscaper;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
@@ -552,10 +552,10 @@ public class CppCompileAction extends AbstractAction
         legalOuts.put(a, null);
       }
     }
-    for (Pair<Artifact, Artifact> pregreppedSrcs : context.getPregreppedHeaders()) {
-      Artifact hdr = pregreppedSrcs.getFirst();
+    for (PregreppedHeader pregreppedSrcs : context.getPregreppedHeaders()) {
+      Artifact hdr = pregreppedSrcs.originalHeader();
       Preconditions.checkState(!hdr.isSourceArtifact(), hdr);
-      legalOuts.put(hdr, pregreppedSrcs.getSecond());
+      legalOuts.put(hdr, pregreppedSrcs.greppedHeader());
     }
     return Collections.unmodifiableMap(legalOuts);
   }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
index 5f56c9f1d7..3d93141fc6 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
@@ -17,6 +17,7 @@ package com.google.devtools.build.lib.rules.cpp;
 import static com.google.devtools.build.lib.packages.BuildType.LABEL;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
+import com.google.auto.value.AutoValue;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
@@ -63,13 +64,16 @@ import com.google.devtools.build.lib.rules.cpp.CppCompilationContext.Builder;
 import com.google.devtools.build.lib.rules.cpp.CppConfiguration.DynamicMode;
 import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType;
 import com.google.devtools.build.lib.shell.ShellUtils;
+import com.google.devtools.build.lib.skyframe.serialization.InjectingObjectCodec;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.util.FileTypeSet;
 import com.google.devtools.build.lib.util.Pair;
+import com.google.devtools.build.lib.vfs.FileSystemProvider;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.LipoMode;
 import java.util.ArrayList;
-import java.util.HashMap;
+import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -567,27 +571,52 @@ public class CppHelper {
   }
 
   /**
+   * A header that has been grepped for #include statements. Includes the original header as well as
+   * a stripped version of that header that contains only #include statements.
+   */
+  @AutoCodec(dependency = FileSystemProvider.class)
+  @AutoValue
+  abstract static class PregreppedHeader {
+    public static final InjectingObjectCodec<PregreppedHeader, FileSystemProvider> CODEC =
+        new CppHelper_PregreppedHeader_AutoCodec();
+
+    @AutoCodec.Instantiator
+    static PregreppedHeader create(Artifact originalHeader, Artifact greppedHeader) {
+      return new AutoValue_CppHelper_PregreppedHeader(originalHeader, greppedHeader);
+    }
+
+    /** Returns the original header, before grepping. */
+    abstract Artifact originalHeader();
+
+    /** Returns the grepped header, which contains only #include statements. */
+    abstract Artifact greppedHeader();
+  }
+
+  /**
    * Creates a grep-includes ExtractInclusions action for generated sources/headers in the
-   * needsIncludeScanning() BuildConfiguration case. Returns a map from original header
-   * Artifact to the output Artifact of grepping over it. The return value only includes
-   * entries for generated sources or headers when --extract_generated_inclusions is enabled.
+   * needsIncludeScanning() BuildConfiguration case. Returns a map from original header Artifact to
+   * the output Artifact of grepping over it. The return value only includes entries for generated
+   * sources or headers when --extract_generated_inclusions is enabled.
    *
-   * <p>Previously, incremental rebuilds redid all include scanning work
-   * for a given .cc source in serial. For high-latency file systems, this could cause
-   * performance problems if many headers are generated.
+   * <p>Previously, incremental rebuilds redid all include scanning work for a given .cc source in
+   * serial. For high-latency file systems, this could cause performance problems if many headers
+   * are generated.
    */
   @Nullable
-  public static final Map<Artifact, Artifact> createExtractInclusions(
+  public static final List<PregreppedHeader> createExtractInclusions(
       RuleContext ruleContext, CppSemantics semantics, Iterable<Artifact> prerequisites) {
-    Map<Artifact, Artifact> extractions = new HashMap<>();
+    List<PregreppedHeader> extractions = new ArrayList<>();
+    HashSet<Artifact> alreadyProcessedHeaders = new HashSet<>();
     for (Artifact prerequisite : prerequisites) {
-      if (extractions.containsKey(prerequisite)) {
+      if (alreadyProcessedHeaders.contains(prerequisite)) {
         // Don't create duplicate actions just because user specified same header file twice.
         continue;
       }
+      alreadyProcessedHeaders.add(prerequisite);
+
       Artifact scanned = createExtractInclusions(ruleContext, semantics, prerequisite);
       if (scanned != null) {
-        extractions.put(prerequisite, scanned);
+        extractions.add(PregreppedHeader.create(prerequisite, scanned));
       }
     }
     return extractions;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModuleMap.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModuleMap.java
index 2d52c5acd8..8428d2c419 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModuleMap.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModuleMap.java
@@ -16,24 +16,36 @@ package com.google.devtools.build.lib.rules.cpp;
 import com.google.common.base.Optional;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
+import com.google.devtools.build.lib.skyframe.serialization.InjectingObjectCodec;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization;
+import com.google.devtools.build.lib.vfs.FileSystemProvider;
 
-/**
- * Structure for C++ module maps. Stores the name of the module and a .cppmap artifact.
- */
+/** Structure for C++ module maps. Stores the name of the module and a .cppmap artifact. */
 @Immutable
+@AutoCodec(dependency = FileSystemProvider.class)
 public final class CppModuleMap {
+  public static final InjectingObjectCodec<CppModuleMap, FileSystemProvider> CODEC =
+      new CppModuleMap_AutoCodec();
+
   // NOTE: If you add a field here, you'll likely need to update CppModuleMapAction.computeKey().
   private final Artifact artifact;
   private final String name;
   private final Optional<Artifact> umbrellaHeader;
 
   public CppModuleMap(Artifact artifact, String name) {
-    this(artifact, null, name);
+    this(artifact, Optional.absent(), name);
   }
 
   public CppModuleMap(Artifact artifact, Artifact umbrellaHeader, String name) {
+    this(artifact, Optional.fromNullable(umbrellaHeader), name);
+  }
+
+  @AutoCodec.Instantiator
+  @VisibleForSerialization
+  CppModuleMap(Artifact artifact, Optional<Artifact> umbrellaHeader, String name) {
     this.artifact = artifact;
-    this.umbrellaHeader = Optional.fromNullable(umbrellaHeader);
+    this.umbrellaHeader = umbrellaHeader;
     this.name = name;
   }