Convert the coverage flags to use the feature configuration.

--
MOS_MIGRATED_REVID=100281959

6 files changed, 84 insertions, 34 deletions

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 84c677aa07..69b79edc04 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
@@ -546,6 +546,9 @@ public final class CcCommon {
     if (cppConfiguration.isLipoOptimizationOrInstrumentation()) {
       requestedFeatures.add(CppRuleClasses.LIPO);
     }
+    if (ruleContext.getConfiguration().isCodeCoverageEnabled()) {
+      requestedFeatures.add(CppRuleClasses.COVERAGE);
+    }
 
     FeatureConfiguration configuration =
         toolchain.getFeatures().getFeatureConfiguration(requestedFeatures.build());
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 bd959611c7..e153bd8f17 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
@@ -241,9 +241,17 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable
     if (cppConfiguration != null && !cppConfiguration.shouldScanIncludes()) {
       inputsKnown = true;
     }
-    this.cppCompileCommandLine = new CppCompileCommandLine(sourceFile, dotdFile,
-        copts, coptsFilter, pluginOpts, (gcnoFile != null), features,
-        featureConfiguration, variables, fdoBuildStamp);
+    this.cppCompileCommandLine =
+        new CppCompileCommandLine(
+            sourceFile,
+            dotdFile,
+            copts,
+            coptsFilter,
+            pluginOpts,
+            features,
+            featureConfiguration,
+            variables,
+            fdoBuildStamp);
     this.actionContext = actionContext;
     this.lipoScannables = lipoScannables;
     this.actionClassId = actionClassId;
@@ -1165,7 +1173,6 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable
     private final List<String> copts;
     private final Predicate<String> coptsFilter;
     private final List<String> pluginOpts;
-    private final boolean isInstrumented;
     private final Collection<String> features;
     private final FeatureConfiguration featureConfiguration;
     private final CcToolchainFeatures.Variables variables;
@@ -1173,10 +1180,14 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable
     // The value of the BUILD_FDO_TYPE macro to be defined on command line
     @Nullable private final String fdoBuildStamp;
 
-    public CppCompileCommandLine(Artifact sourceFile, DotdFile dotdFile,
-        ImmutableList<String> copts, Predicate<String> coptsFilter,
-        ImmutableList<String> pluginOpts, boolean isInstrumented,
-        Collection<String> features, FeatureConfiguration featureConfiguration,
+    public CppCompileCommandLine(
+        Artifact sourceFile,
+        DotdFile dotdFile,
+        ImmutableList<String> copts,
+        Predicate<String> coptsFilter,
+        ImmutableList<String> pluginOpts,
+        Collection<String> features,
+        FeatureConfiguration featureConfiguration,
         CcToolchainFeatures.Variables variables,
         @Nullable String fdoBuildStamp) {
       this.sourceFile = Preconditions.checkNotNull(sourceFile);
@@ -1184,7 +1195,6 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable
       this.copts = Preconditions.checkNotNull(copts);
       this.coptsFilter = coptsFilter;
       this.pluginOpts = Preconditions.checkNotNull(pluginOpts);
-      this.isInstrumented = isInstrumented;
       this.features = Preconditions.checkNotNull(features);
       this.featureConfiguration = featureConfiguration;
       this.variables = variables;
@@ -1246,11 +1256,6 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable
       options.addAll(pluginOpts);
       addFilteredOptions(options, toolchain.getCompilerOptions(features));
 
-      // Enable instrumentation if requested.
-      if (isInstrumented) {
-        addFilteredOptions(options, ImmutableList.of("-fprofile-arcs", "-ftest-coverage"));
-      }
-
       String sourceFilename = sourceFile.getExecPathString();
       if (CppFileTypes.C_SOURCE.matches(sourceFilename)) {
         addFilteredOptions(options, toolchain.getCOptions());
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
index f72c4f34c0..8d66f01753 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
@@ -784,6 +784,33 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
             + "}",
             toolchainBuilder);
       }
+      if (!features.contains("coverage")) {
+        TextFormat.merge(
+            ""
+                + "feature {"
+                + "  name: 'coverage'"
+                + "  flag_set {"
+                + "    action: 'preprocess-assemble'"
+                + "    action: 'c-compile'"
+                + "    action: 'c++-compile'"
+                + "    action: 'c++-header-parsing'"
+                + "    action: 'c++-header-preprocessing'"
+                + "    action: 'c++-module-compile'"
+                + "    expand_if_all_available: 'gcov_gcno_file'"
+                + "    flag_group {"
+                + "      flag: '-fprofile-arcs'"
+                + "      flag: '-ftest-coverage'"
+                + "    }"
+                + "  }"
+                + "  flag_set {"
+                + "    action: 'c++-link'"
+                + "    flag_group {"
+                + "      flag: '-lgcov'"
+                + "    }"
+                + "  }"
+                + "}",
+            toolchainBuilder);
+      }
     } catch (ParseException e) {
       // Can only happen if we change the proto definition without changing our configuration above.
       throw new RuntimeException(e);
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 5282dc1188..2f12d4a6fd 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
@@ -329,10 +329,15 @@ public final class CppModel {
     return result;
   }
 
-  private void setupBuildVariables(CppCompileActionBuilder builder,
-      boolean usePic, PathFragment ccRelativeName) {
+  private void setupBuildVariables(
+      CppCompileActionBuilder builder,
+      boolean usePic,
+      PathFragment ccRelativeName,
+      Artifact gcnoFile) {
     CcToolchainFeatures.Variables.Builder buildVariables =
         new CcToolchainFeatures.Variables.Builder();
+    
+    // TODO(bazel-team): Pull out string constants for all build variables.
 
     CppModuleMap cppModuleMap = context.getCppModuleMap();
     if (featureConfiguration.isEnabled(CppRuleClasses.MODULE_MAPS) && cppModuleMap != null) {
@@ -358,6 +363,9 @@ public final class CppModel {
       cppConfiguration.getFdoSupport().configureCompilation(builder, buildVariables, ruleContext,
           ccRelativeName, usePic, featureConfiguration, cppConfiguration);
     }
+    if (gcnoFile != null) {
+      buildVariables.addVariable("gcov_gcno_file", gcnoFile.getExecPathString());
+    }
 
     CcToolchainFeatures.Variables variables = buildVariables.build();
     builder.setVariables(variables);
@@ -412,7 +420,7 @@ public final class CppModel {
         .setDotdFile(outputName, ".h.d")
         // If we generate pic actions, we prefer the header actions to use the pic artifacts.
         .setPicMode(this.getGeneratePicActions());
-    setupBuildVariables(builder, this.getGeneratePicActions(), null);
+    setupBuildVariables(builder, this.getGeneratePicActions(), null, null);
     semantics.finalizeCompileActionBuilder(ruleContext, builder);
     CppCompileAction compileAction = builder.build();
     env.registerAction(compileAction);
@@ -451,7 +459,14 @@ public final class CppModel {
       if (generatePicAction) {
         CppCompileActionBuilder picBuilder =
             copyAsPicBuilder(builder, outputName, outputExtension, dependencyFileExtension);
-        setupBuildVariables(picBuilder, /*usePic=*/true, ccRelativeName);
+        Artifact gcnoFile =
+            isCodeCoverageEnabled()
+                ? ruleContext.getRelatedArtifact(outputName, ".pic.gcno")
+                : null;
+        if (gcnoFile != null) {
+          picBuilder.setGcnoFile(gcnoFile);
+        }
+        setupBuildVariables(picBuilder, /*usePic=*/ true, ccRelativeName, gcnoFile);
 
         if (maySaveTemps) {
           result.addTemps(
@@ -459,10 +474,6 @@ public final class CppModel {
                 ccRelativeName));
         }
 
-        if (isCodeCoverageEnabled()) {
-          picBuilder.setGcnoFile(ruleContext.getRelatedArtifact(outputName, ".pic.gcno"));
-        }
-
         semantics.finalizeCompileActionBuilder(ruleContext, picBuilder);
         CppCompileAction picAction = picBuilder.build();
         env.registerAction(picAction);
@@ -483,7 +494,14 @@ public final class CppModel {
             .setOutputFile(ruleContext.getRelatedArtifact(outputName, outputExtension))
             .setDotdFile(outputName, dependencyFileExtension);
         // Create non-PIC compile actions
-        setupBuildVariables(builder, /*usePic=*/false, ccRelativeName);
+        Artifact gcnoFile =
+            !cppConfiguration.isLipoOptimization() && isCodeCoverageEnabled()
+                ? ruleContext.getRelatedArtifact(outputName, ".gcno")
+                : null;
+        if (gcnoFile != null) {
+          builder.setGcnoFile(gcnoFile);
+        }
+        setupBuildVariables(builder, /*usePic=*/ false, ccRelativeName, gcnoFile);
 
         if (maySaveTemps) {
           result.addTemps(
@@ -491,10 +509,6 @@ public final class CppModel {
                 ccRelativeName));
         }
 
-        if (!cppConfiguration.isLipoOptimization() && isCodeCoverageEnabled()) {
-          builder.setGcnoFile(ruleContext.getRelatedArtifact(outputName, ".gcno"));
-        }
-
         semantics.finalizeCompileActionBuilder(ruleContext, builder);
         CppCompileAction compileAction = builder.build();
         env.registerAction(compileAction);
@@ -530,7 +544,7 @@ public final class CppModel {
         .setOutputFile(outputFile)
         .setDotdFile(outputName, dependencyFileExtension)
         .setTempOutputFile(tempOutputName);
-    setupBuildVariables(builder, getGeneratePicActions(), ccRelativeName);
+    setupBuildVariables(builder, getGeneratePicActions(), ccRelativeName, null);
     semantics.finalizeCompileActionBuilder(ruleContext, builder);
     CppCompileAction action = builder.build();
     env.registerAction(action);
@@ -735,9 +749,9 @@ public final class CppModel {
     String iExt = isCFile ? ".i" : ".ii";
     String picExt = usePic ? ".pic" : "";
     CppCompileActionBuilder dBuilder = new CppCompileActionBuilder(builder);
-    setupBuildVariables(dBuilder, usePic, ccRelativeName);
+    setupBuildVariables(dBuilder, usePic, ccRelativeName, null);
     CppCompileActionBuilder sdBuilder = new CppCompileActionBuilder(builder);
-    setupBuildVariables(sdBuilder, usePic, ccRelativeName);
+    setupBuildVariables(sdBuilder, usePic, ccRelativeName, null);
 
     dBuilder
         .setOutputFile(ruleContext.getRelatedArtifact(outputName, picExt + iExt))
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java
index ea32167f09..de1c92ce96 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java
@@ -178,4 +178,9 @@ public class CppRuleClasses {
    * A string constant for the lipo feature.
    */
   public static final String LIPO = "lipo";
+
+  /**
+   * A string constant for the coverage feature.
+   */
+  public static final String COVERAGE = "coverage";
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java
index 5da414f134..b9a4fb90ee 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java
@@ -668,10 +668,6 @@ public final class LinkCommandLine extends CommandLine {
       argv.addAll(cppConfiguration.getTestOnlyLinkOptions());
     }
 
-    if (configuration.isCodeCoverageEnabled()) {
-      argv.add("-lgcov");
-    }
-
     if (linkTargetType == LinkTargetType.EXECUTABLE && cppConfiguration.forcePic()) {
       argv.add("-pie");
     }