Do not compile source files mentioned in 'hdrs'.

Currently for a library:
  cc_library(name='a', hdrs=['a.cc'])
we compile a.cc into a.pic.o and link that into its reverse dependencies.

With this change, a .cc file in hdrs will be treated like a .inc file or
a file in textual_hdrs.

--
MOS_MIGRATED_REVID=112770625

7 files changed, 158 insertions, 93 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java
index ff5feff2bc..3afa030e1c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java
@@ -45,7 +45,6 @@ import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType;
 import com.google.devtools.build.lib.rules.cpp.LinkerInputs.LibraryToLink;
 import com.google.devtools.build.lib.rules.test.InstrumentedFilesProvider;
 import com.google.devtools.build.lib.syntax.Type;
-import com.google.devtools.build.lib.util.FileType;
 import com.google.devtools.build.lib.util.FileTypeSet;
 import com.google.devtools.build.lib.util.OsUtils;
 import com.google.devtools.build.lib.util.Pair;
@@ -153,7 +152,7 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
       boolean useTestOnlyFlags) throws InterruptedException {
     ruleContext.checkSrcsSamePackage(true);
     FeatureConfiguration featureConfiguration = CcCommon.configureFeatures(ruleContext);
-    CcCommon common = new CcCommon(ruleContext, featureConfiguration);
+    CcCommon common = new CcCommon(ruleContext);
     CppConfiguration cppConfiguration = ruleContext.getFragment(CppConfiguration.class);
     PrecompiledFiles precompiledFiles = new PrecompiledFiles(ruleContext);
 
@@ -162,16 +161,11 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
     List<String> linkopts = common.getLinkopts();
     LinkStaticness linkStaticness = getLinkStaticness(ruleContext, linkopts, cppConfiguration);
 
-    ImmutableList<Pair<Artifact, Label>> cAndCppSources = common.getCAndCppSources();
     CcLibraryHelper helper =
         new CcLibraryHelper(ruleContext, semantics, featureConfiguration)
             .fromCommon(common)
-            .addSources(cAndCppSources)
+            .addSources(common.getSources())
             .addDeps(ImmutableList.of(CppHelper.mallocForTarget(ruleContext)))
-            .addPrivateHeaders(
-                FileType.filter(
-                    ruleContext.getPrerequisiteArtifacts("srcs", Mode.TARGET).list(),
-                    CppFileTypes.CPP_HEADER))
             .setFake(fake)
             .setLinkType(linkType)
             .addPrecompiledFiles(precompiledFiles);
@@ -284,9 +278,16 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
     // logical since all symlinked libraries will be linked anyway and would
     // not require manual loading but if we do, then we would need to collect
     // their names and use a different constructor below.
-    Runfiles runfiles = collectRunfiles(
-        ruleContext, linkingOutputs, cppCompilationContext, linkStaticness, filesToBuild,
-        fakeLinkerInputs, fake, cAndCppSources);
+    Runfiles runfiles =
+        collectRunfiles(
+            ruleContext,
+            linkingOutputs,
+            cppCompilationContext,
+            linkStaticness,
+            filesToBuild,
+            fakeLinkerInputs,
+            fake,
+            helper.getCompilationUnitSources());
     RunfilesSupport runfilesSupport = RunfilesSupport.withExecutable(
         ruleContext, runfiles, executable, ruleContext.getConfiguration().buildRunfiles());
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
index 1825742e82..2ede292346 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
@@ -41,7 +41,6 @@ import com.google.devtools.build.lib.rules.test.InstrumentedFilesProviderImpl;
 import com.google.devtools.build.lib.shell.ShellUtils;
 import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.util.FileType;
-import com.google.devtools.build.lib.util.FileTypeSet;
 import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -60,13 +59,6 @@ public final class CcCommon {
 
   private static final String NO_COPTS_ATTRIBUTE = "nocopts";
 
-  private static final FileTypeSet SOURCE_TYPES = FileTypeSet.of(
-      CppFileTypes.CPP_SOURCE,
-      CppFileTypes.CPP_HEADER,
-      CppFileTypes.C_SOURCE,
-      CppFileTypes.ASSEMBLER,
-      CppFileTypes.ASSEMBLER_WITH_C_PREPROCESSOR);
-
   /**
    * Collects all metadata files generated by C++ compilation actions that output the .o files
    * on the input.
@@ -97,15 +89,11 @@ public final class CcCommon {
   /** C++ configuration */
   private final CppConfiguration cppConfiguration;
   
-  /** Active toolchain features. */
-  private final FeatureConfiguration featureConfiguration;
-
   private final RuleContext ruleContext;
 
-  public CcCommon(RuleContext ruleContext, FeatureConfiguration featureConfiguration) {
+  public CcCommon(RuleContext ruleContext) {
     this.ruleContext = ruleContext;
     this.cppConfiguration = ruleContext.getFragment(CppConfiguration.class);
-    this.featureConfiguration = featureConfiguration;
   }
 
   /**
@@ -201,39 +189,23 @@ public final class CcCommon {
     return new TransitiveLipoInfoProvider(scannableBuilder.build());
   }
 
-  private boolean shouldProcessHeaders() {
-    return featureConfiguration.isEnabled(CppRuleClasses.PREPROCESS_HEADERS)
-        || featureConfiguration.isEnabled(CppRuleClasses.PARSE_HEADERS);
-  }
-
   /**
    * Returns a list of ({@link Artifact}, {@link Label}) pairs. Each pair represents an input
    * source file and the label of the rule that generates it (or the label of the source file
-   * itself if it is an input file)
+   * itself if it is an input file).
    */
-  ImmutableList<Pair<Artifact, Label>> getCAndCppSources() {
+  List<Pair<Artifact, Label>> getSources() {
     Map<Artifact, Label> map = Maps.newLinkedHashMap();
     Iterable<FileProvider> providers =
         ruleContext.getPrerequisites("srcs", Mode.TARGET, FileProvider.class);
-    // TODO(bazel-team): Move header processing logic down in the stack (to CcLibraryHelper or
-    // such).
-    boolean processHeaders = shouldProcessHeaders();
-    if (processHeaders && hasAttribute("hdrs", BuildType.LABEL_LIST)) {
-      providers = Iterables.concat(providers,
-          ruleContext.getPrerequisites("hdrs", Mode.TARGET, FileProvider.class));
-    }
     for (FileProvider provider : providers) {
-      for (Artifact artifact : FileType.filter(provider.getFilesToBuild(), SOURCE_TYPES)) {
-        boolean isHeader = CppFileTypes.CPP_HEADER.matches(artifact.getExecPath());
-        if ((isHeader && !processHeaders)
-            || CppFileTypes.CPP_TEXTUAL_INCLUDE.matches(artifact.getExecPath())) {
-          continue;
-        }
-        Label oldLabel = map.put(artifact, provider.getLabel());
+      for (Artifact artifact : provider.getFilesToBuild()) {
         // TODO(bazel-team): We currently do not warn for duplicate headers with
         // different labels, as that would require cleaning up the code base
         // without significant benefit; we should eventually make this
         // consistent one way or the other.
+        Label oldLabel = map.put(artifact, provider.getLabel());
+        boolean isHeader = CppFileTypes.CPP_HEADER.matches(artifact.getExecPath());
         if (!isHeader && oldLabel != null && !oldLabel.equals(provider.getLabel())) {
           ruleContext.attributeError("srcs", String.format(
               "Artifact '%s' is duplicated (through '%s' and '%s')",
@@ -246,7 +218,6 @@ public final class CcCommon {
     for (Map.Entry<Artifact, Label> entry : map.entrySet()) {
       result.add(Pair.of(entry.getKey(), entry.getValue()));
     }
-
     return result.build();
   }
 
@@ -255,21 +226,44 @@ public final class CcCommon {
    * warnings to the {@link RuleContext} as a side effect, and so should only be called once for any
    * given rule.
    */
-  public static List<Artifact> getHeaders(RuleContext ruleContext) {
-    List<Artifact> hdrs = new ArrayList<>();
+  public static List<Pair<Artifact, Label>> getHeaders(RuleContext ruleContext) {
+    Map<Artifact, Label> map = Maps.newLinkedHashMap();
     for (TransitiveInfoCollection target :
         ruleContext.getPrerequisitesIf("hdrs", Mode.TARGET, FileProvider.class)) {
       FileProvider provider = target.getProvider(FileProvider.class);
       for (Artifact artifact : provider.getFilesToBuild()) {
-        if (!CppRuleClasses.DISALLOWED_HDRS_FILES.matches(artifact.getFilename())) {
-          hdrs.add(artifact);
-        } else {
+        if (CppRuleClasses.DISALLOWED_HDRS_FILES.matches(artifact.getFilename())) {
           ruleContext.attributeWarning("hdrs", "file '" + artifact.getFilename()
               + "' from target '" + target.getLabel() + "' is not allowed in hdrs");
+          continue;
+        }
+        Label oldLabel = map.put(artifact, provider.getLabel());
+        if (oldLabel != null && !oldLabel.equals(provider.getLabel())) {
+          ruleContext.attributeWarning(
+              "hdrs",
+              String.format(
+                  "Artifact '%s' is duplicated (through '%s' and '%s')",
+                  artifact.getExecPathString(),
+                  oldLabel,
+                  provider.getLabel()));
         }
       }
     }
-    return hdrs;
+    
+    ImmutableList.Builder<Pair<Artifact, Label>> result = ImmutableList.builder();
+    for (Map.Entry<Artifact, Label> entry : map.entrySet()) {
+      result.add(Pair.of(entry.getKey(), entry.getValue()));
+    }
+    return result.build();
+  }
+
+  /**
+   * Returns the files from headers and does some sanity checks. Note that this method reports
+   * warnings to the {@link RuleContext} as a side effect, and so should only be called once for any
+   * given rule.
+   */
+  public List<Pair<Artifact, Label>> getHeaders() {
+    return getHeaders(ruleContext);
   }
 
   private static ImmutableList<String> getPackageCopts(RuleContext ruleContext) {
@@ -423,12 +417,16 @@ public final class CcCommon {
    */
   static NestedSet<Artifact> collectCompilationPrerequisites(
       RuleContext ruleContext, CppCompilationContext context) {
-    // TODO(bazel-team): Use context.getCompilationPrerequisites() instead.
+    // TODO(bazel-team): Use context.getCompilationPrerequisites() instead; note that this will
+    // need cleaning up the prerequisites, as the compilation context currently collects them
+    // transitively (to get transitive headers), but source files are not transitive compilation
+    // prerequisites.
     NestedSetBuilder<Artifact> prerequisites = NestedSetBuilder.stableOrder();
     if (ruleContext.attributes().has("srcs", BuildType.LABEL_LIST)) {
-      for (FileProvider provider : ruleContext
-          .getPrerequisites("srcs", Mode.TARGET, FileProvider.class)) {
-        prerequisites.addAll(FileType.filter(provider.getFilesToBuild(), SOURCE_TYPES));
+      for (FileProvider provider :
+          ruleContext.getPrerequisites("srcs", Mode.TARGET, FileProvider.class)) {
+        prerequisites.addAll(
+            FileType.filter(provider.getFilesToBuild(), CcLibraryHelper.SOURCE_TYPES));
       }
     }
     prerequisites.addTransitive(context.getDeclaredIncludeSrcs());
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java
index a8c9f4d827..764b7f4441 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java
@@ -40,7 +40,6 @@ import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType;
 import com.google.devtools.build.lib.rules.cpp.LinkerInputs.LibraryToLink;
 import com.google.devtools.build.lib.rules.test.InstrumentedFilesProvider;
 import com.google.devtools.build.lib.syntax.Type;
-import com.google.devtools.build.lib.util.FileType;
 import com.google.devtools.build.lib.util.FileTypeSet;
 import com.google.devtools.build.lib.vfs.PathFragment;
 
@@ -112,16 +111,15 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory {
       boolean collectLinkstamp,
       boolean addDynamicRuntimeInputArtifactsToRunfiles) {
     FeatureConfiguration featureConfiguration = CcCommon.configureFeatures(ruleContext);
-    final CcCommon common = new CcCommon(ruleContext, featureConfiguration);
+    final CcCommon common = new CcCommon(ruleContext);
     PrecompiledFiles precompiledFiles = new PrecompiledFiles(ruleContext);
 
     CcLibraryHelper helper =
         new CcLibraryHelper(ruleContext, semantics, featureConfiguration)
             .fromCommon(common)
-
             .addLinkopts(common.getLinkopts())
-            .addSources(common.getCAndCppSources())
-            .addPublicHeaders(CcCommon.getHeaders(ruleContext))
+            .addSources(common.getSources())
+            .addPublicHeaders(common.getHeaders())
             .enableCcNativeLibrariesProvider()
             .enableCompileProviders()
             .enableInterfaceSharedObjects()
@@ -165,9 +163,6 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory {
     }
 
     if (ruleContext.getRule().isAttrDefined("srcs", BuildType.LABEL_LIST)) {
-      helper.addPrivateHeaders(FileType.filter(
-          ruleContext.getPrerequisiteArtifacts("srcs", Mode.TARGET).list(),
-          CppFileTypes.CPP_HEADER));
       ruleContext.checkSrcsSamePackage(true);
     }
     if (ruleContext.getRule().isAttrDefined("textual_hdrs", BuildType.LABEL_LIST)) {
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 200edc4913..6fd24c9659 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
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.rules.cpp;
 
 import com.google.common.base.Function;
 import com.google.common.base.Predicates;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.Artifact;
@@ -39,6 +40,7 @@ import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfig
 import com.google.devtools.build.lib.rules.cpp.CppConfiguration.HeadersCheckingMode;
 import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType;
 import com.google.devtools.build.lib.rules.cpp.LinkerInputs.LibraryToLink;
+import com.google.devtools.build.lib.util.FileTypeSet;
 import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -67,6 +69,14 @@ import javax.annotation.Nullable;
  * methods.
  */
 public final class CcLibraryHelper {
+  static final FileTypeSet SOURCE_TYPES =
+      FileTypeSet.of(
+          CppFileTypes.CPP_SOURCE,
+          CppFileTypes.CPP_HEADER,
+          CppFileTypes.C_SOURCE,
+          CppFileTypes.ASSEMBLER,
+          CppFileTypes.ASSEMBLER_WITH_C_PREPROCESSOR);
+  
   /** Function for extracting module maps from CppCompilationDependencies. */
   public static final Function<TransitiveInfoCollection, CppModuleMap> CPP_DEPS_TO_MODULES =
     new Function<TransitiveInfoCollection, CppModuleMap>() {
@@ -155,7 +165,7 @@ public final class CcLibraryHelper {
   private final List<Artifact> publicTextualHeaders = new ArrayList<>();
   private final List<Artifact> privateHeaders = new ArrayList<>();
   private final List<PathFragment> additionalExportedHeaders = new ArrayList<>();
-  private final List<Pair<Artifact, Label>> sources = new ArrayList<>();
+  private final List<Pair<Artifact, Label>> compilationUnitSources = new ArrayList<>();
   private final List<Artifact> objectFiles = new ArrayList<>();
   private final List<Artifact> picObjectFiles = new ArrayList<>();
   private final List<String> copts = new ArrayList<>();
@@ -214,31 +224,39 @@ public final class CcLibraryHelper {
   }
 
   /**
-   * Add the corresponding files as header files, i.e., these files will not be compiled, but are
-   * made visible as includes to dependent rules.
+   * Adds {@code headers} as public header files. These files will be made visible to dependent
+   * rules. They may be parsed/preprocessed or compiled into a header module depending on the
+   * configuration.
    */
   public CcLibraryHelper addPublicHeaders(Collection<Artifact> headers) {
-    this.publicHeaders.addAll(headers);
+    for (Artifact header : headers) {
+      addHeader(header, ruleContext.getLabel());
+    }
     return this;
   }
-
+  
   /**
-   * Add the corresponding files as public header files, i.e., these files will not be compiled, but
-   * are made visible as includes to dependent rules in module maps.
+   * Adds {@code headers} as public header files. These files will be made visible to dependent
+   * rules. They may be parsed/preprocessed or compiled into a header module depending on the
+   * configuration.
    */
   public CcLibraryHelper addPublicHeaders(Artifact... headers) {
-    return addPublicHeaders(Arrays.asList(headers));
+    addPublicHeaders(Arrays.asList(headers));
+    return this;
   }
-
+  
   /**
-   * Add the corresponding files as private header files, i.e., these files will not be compiled,
-   * but are not made visible as includes to dependent rules in module maps.
+   * Adds {@code headers} as public header files. These files will be made visible to dependent
+   * rules. They may be parsed/preprocessed or compiled into a header module depending on the
+   * configuration.
    */
-  public CcLibraryHelper addPrivateHeaders(Iterable<Artifact> privateHeaders) {
-    Iterables.addAll(this.privateHeaders, privateHeaders);
+  public CcLibraryHelper addPublicHeaders(Iterable<Pair<Artifact, Label>> headers) {
+    for (Pair<Artifact, Label> header : headers) {
+      addHeader(header.first, header.second);
+    }
     return this;
   }
-
+  
   /**
    * Add the corresponding files as public header files, i.e., these files will not be compiled, but
    * are made visible as includes to dependent rules in module maps.
@@ -265,10 +283,9 @@ public final class CcLibraryHelper {
    * Add the corresponding files as source files. These may also be header files, in which case
    * they will not be compiled, but also not made visible as includes to dependent rules.
    */
-  // TODO(bazel-team): This is inconsistent with the documentation on CppModel.
   public CcLibraryHelper addSources(Collection<Artifact> sources) {
     for (Artifact source : sources) {
-      this.sources.add(Pair.of(source, ruleContext.getLabel()));
+      addSource(source, ruleContext.getLabel());
     }
     return this;
   }
@@ -277,9 +294,10 @@ public final class CcLibraryHelper {
    * Add the corresponding files as source files. These may also be header files, in which case
    * they will not be compiled, but also not made visible as includes to dependent rules.
    */
-  // TODO(bazel-team): This is inconsistent with the documentation on CppModel.
   public CcLibraryHelper addSources(Iterable<Pair<Artifact, Label>> sources) {
-    Iterables.addAll(this.sources, sources);
+    for (Pair<Artifact, Label> source : sources) {
+      addSource(source.first, source.second);
+    }
     return this;
   }
 
@@ -290,6 +308,50 @@ public final class CcLibraryHelper {
   public CcLibraryHelper addSources(Artifact... sources) {
     return addSources(Arrays.asList(sources));
   }
+  
+  /**
+   * Adds a header to {@code publicHeaders} and in case header processing is switched on for the
+   * file type also to compilationUnitSources.
+   */
+  private void addHeader(Artifact header, Label label) {
+    boolean isHeader = CppFileTypes.CPP_HEADER.matches(header.getExecPath());
+    boolean isTextualInclude = CppFileTypes.CPP_TEXTUAL_INCLUDE.matches(header.getExecPath());
+    publicHeaders.add(header);
+    if (isTextualInclude || !isHeader || !shouldProcessHeaders()) {
+      return;
+    }
+    compilationUnitSources.add(Pair.of(header, label));
+  }
+
+  /**
+   * Adds a source to {@code compilationUnitSources} if it is a compiled file type (including
+   * parsed/preprocessed header) and to {@code privateHeaders} if it is a header.
+   */
+  private void addSource(Artifact source, Label label) {
+    boolean isHeader = CppFileTypes.CPP_HEADER.matches(source.getExecPath());
+    boolean isTextualInclude = CppFileTypes.CPP_TEXTUAL_INCLUDE.matches(source.getExecPath());
+    boolean isCompiledSource = SOURCE_TYPES.matches(source.getExecPathString());
+    if (isHeader || isTextualInclude) {
+      privateHeaders.add(source);
+    }
+    if (isTextualInclude || !isCompiledSource || (isHeader && !shouldProcessHeaders())) {
+      return;
+    }
+    compilationUnitSources.add(Pair.of(source, label));
+  }
+
+  private boolean shouldProcessHeaders() {
+    return featureConfiguration.isEnabled(CppRuleClasses.PREPROCESS_HEADERS)
+        || featureConfiguration.isEnabled(CppRuleClasses.PARSE_HEADERS);
+  }
+
+  /**
+   * Returns the compilation unit sources. That includes all compiled source files as well
+   * as headers that will be parsed or preprocessed.
+   */
+  public ImmutableList<Pair<Artifact, Label>> getCompilationUnitSources() {
+    return ImmutableList.copyOf(this.compilationUnitSources);
+  }
 
   /**
    * Add the corresponding files as linker inputs for non-PIC links. If the corresponding files are
@@ -713,7 +775,7 @@ public final class CcLibraryHelper {
    */
   private CppModel initializeCppModel() {
     return new CppModel(ruleContext, semantics)
-        .addSources(sources)
+        .addCompilationUnitSources(compilationUnitSources)
         .addCopts(copts)
         .setLinkTargetType(linkType)
         .setNeverLink(neverlink)
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java
index 1ddbff3789..91ab7d89f1 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java
@@ -124,9 +124,9 @@ public final class CppModel {
 
   /**
    * Adds a single source file to be compiled. Note that this should only be called for primary
-   * compilation units, not for header files or files that are otherwise included.
+   * compilation units, including module files or headers to be parsed or preprocessed.
    */
-  public CppModel addSources(Iterable<Artifact> sourceFiles, Label sourceLabel) {
+  public CppModel addCompilationUnitSources(Iterable<Artifact> sourceFiles, Label sourceLabel) {
     for (Artifact sourceFile : sourceFiles) {
       this.sourceFiles.add(Pair.of(sourceFile, sourceLabel));
     }
@@ -134,10 +134,10 @@ public final class CppModel {
   }
 
   /**
-   * Adds all the source files. Note that this should only be called for primary compilation units,
-   * not for header files or files that are otherwise included.
+   * Adds all the source files. Note that this should only be called for primary
+   * compilation units, including module files or headers to be parsed or preprocessed.
    */
-  public CppModel addSources(Iterable<Pair<Artifact, Label>> sources) {
+  public CppModel addCompilationUnitSources(Iterable<Pair<Artifact, Label>> sources) {
     Iterables.addAll(this.sourceFiles, sources);
     return this;
   }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/IosFramework.java b/src/main/java/com/google/devtools/build/lib/rules/objc/IosFramework.java
index 0c553b2bfb..d60a62f4d9 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/IosFramework.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/IosFramework.java
@@ -32,9 +32,11 @@ import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.actions.SymlinkAction;
+import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.rules.apple.DottedVersion;
 import com.google.devtools.build.lib.rules.cpp.CcCommon;
 import com.google.devtools.build.lib.rules.objc.ReleaseBundlingSupport.SplitArchTransition.ConfigurationDistinguisher;
+import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.vfs.PathFragment;
 
 /**
@@ -84,7 +86,8 @@ public class IosFramework extends ReleaseBundlingTargetFactory {
     IntermediateArtifacts intermediateArtifacts =
         ObjcRuleClasses.intermediateArtifacts(ruleContext);
 
-    ImmutableList<Artifact> headers = ImmutableList.copyOf(CcCommon.getHeaders(ruleContext));
+    ImmutableList<Pair<Artifact, Label>> headers =
+        ImmutableList.copyOf(CcCommon.getHeaders(ruleContext));
 
     ImmutableMap.Builder<Artifact, Artifact> builder = new ImmutableMap.Builder<>();
 
@@ -94,11 +97,11 @@ public class IosFramework extends ReleaseBundlingTargetFactory {
     builder.put(intermediateArtifacts.combinedArchitectureBinary(), frameworkBinary);
 
     // Create framework headers
-    for (Artifact header : headers) {
+    for (Pair<Artifact, Label> header : headers) {
       Artifact frameworkHeader =
-          outputArtifact(ruleContext, new PathFragment("Headers/" + header.getFilename()));
+          outputArtifact(ruleContext, new PathFragment("Headers/" + header.first.getFilename()));
 
-      builder.put(header, frameworkHeader);
+      builder.put(header.first, frameworkHeader);
     }
 
     return builder.build();
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommon.java
index b469b22c5c..4b8412ee36 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommon.java
@@ -61,6 +61,7 @@ import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
+import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.packages.BuildType;
 import com.google.devtools.build.lib.rules.apple.AppleToolchain;
 import com.google.devtools.build.lib.rules.cpp.CcCommon;
@@ -71,6 +72,7 @@ import com.google.devtools.build.lib.rules.cpp.CppModuleMap;
 import com.google.devtools.build.lib.rules.cpp.CppRunfilesProvider;
 import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.util.FileType;
+import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.lib.vfs.PathFragment;
 
@@ -106,7 +108,11 @@ public final class ObjcCommon {
       if (!ruleContext.attributes().has("hdrs", BuildType.LABEL_LIST)) {
         return ImmutableList.of();
       }
-      return ImmutableList.copyOf(CcCommon.getHeaders(ruleContext));
+      ImmutableList.Builder<Artifact> headers = ImmutableList.builder();
+      for (Pair<Artifact, Label> header : CcCommon.getHeaders(ruleContext)) {
+        headers.add(header.first);
+      }
+      return headers.build();
     }
 
     /**