Replace all tool getters in CppConfiguration with corresponding calls to

CcToolchainProvider#getToolPathFragment.

PiperOrigin-RevId: 171837541

5 files changed, 61 insertions, 144 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 416cf4612f..c4a342024f 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
@@ -49,6 +49,7 @@ import com.google.devtools.build.lib.rules.cpp.CcCommon.CcFlagsSupplier;
 import com.google.devtools.build.lib.rules.cpp.CcLibraryHelper.Info;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
 import com.google.devtools.build.lib.rules.cpp.CppConfiguration.DynamicMode;
+import com.google.devtools.build.lib.rules.cpp.CppConfiguration.Tool;
 import com.google.devtools.build.lib.rules.cpp.Link.LinkStaticness;
 import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType;
 import com.google.devtools.build.lib.rules.cpp.LinkerInputs.LibraryToLink;
@@ -391,7 +392,7 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
             ltoBackendArtifacts);
     Artifact dwpFile =
         ruleContext.getImplicitOutputArtifact(CppRuleClasses.CC_BINARY_DEBUG_PACKAGE);
-    createDebugPackagerActions(ruleContext, ccToolchain, cppConfiguration, dwpFile, dwoArtifacts);
+    createDebugPackagerActions(ruleContext, ccToolchain, dwpFile, dwoArtifacts);
 
     // The debug package should include the dwp file only if it was explicitly requested.
     Artifact explicitDwpFile = dwpFile;
@@ -639,11 +640,12 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
   }
 
   /**
-   * Creates the actions needed to generate this target's "debug info package"
-   * (i.e. its .dwp file).
+   * Creates the actions needed to generate this target's "debug info package" (i.e. its .dwp file).
    */
-  private static void createDebugPackagerActions(RuleContext context, CcToolchainProvider toolchain,
-      CppConfiguration cppConfiguration, Artifact dwpOutput,
+  private static void createDebugPackagerActions(
+      RuleContext context,
+      CcToolchainProvider toolchain,
+      Artifact dwpOutput,
       DwoArtifactsCollector dwoArtifactsCollector) {
     Iterable<Artifact> allInputs = getDwpInputs(context,
         dwoArtifactsCollector.getPicDwoArtifacts(),
@@ -679,8 +681,7 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
     // at the leaves than the root, but that both increases parallelism and reduces the final
     // action's input size.
     Packager packager =
-        createIntermediateDwpPackagers(
-            context, dwpOutput, cppConfiguration, dwpTools, allInputs, 1);
+        createIntermediateDwpPackagers(context, dwpOutput, toolchain, dwpTools, allInputs, 1);
     packager.spawnAction.setMnemonic("CcGenerateDwp").addOutput(dwpOutput);
     packager.commandLine.addExecPath("-o", dwpOutput);
     context.registerAction(packager.build(context));
@@ -704,7 +705,7 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
   private static Packager createIntermediateDwpPackagers(
       RuleContext context,
       Artifact dwpOutput,
-      CppConfiguration cppConfiguration,
+      CcToolchainProvider toolchain,
       NestedSet<Artifact> dwpTools,
       Iterable<Artifact> inputs,
       int intermediateDwpCount) {
@@ -712,13 +713,13 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
 
     // Step 1: generate our batches. We currently break into arbitrary batches of fixed maximum
     // input counts, but we can always apply more intelligent heuristics if the need arises.
-    Packager currentPackager = newDwpAction(cppConfiguration, dwpTools);
+    Packager currentPackager = newDwpAction(toolchain, dwpTools);
     int inputsForCurrentPackager = 0;
 
     for (Artifact dwoInput : inputs) {
       if (inputsForCurrentPackager == MAX_INPUTS_PER_DWP_ACTION) {
         packagers.add(currentPackager);
-        currentPackager = newDwpAction(cppConfiguration, dwpTools);
+        currentPackager = newDwpAction(toolchain, dwpTools);
         inputsForCurrentPackager = 0;
       }
       currentPackager.spawnAction.addInput(dwoInput);
@@ -741,12 +742,7 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
         intermediateOutputs.add(intermediateOutput);
       }
       return createIntermediateDwpPackagers(
-          context,
-          dwpOutput,
-          cppConfiguration,
-          dwpTools,
-          intermediateOutputs,
-          intermediateDwpCount);
+          context, dwpOutput, toolchain, dwpTools, intermediateOutputs, intermediateDwpCount);
     }
     return Iterables.getOnlyElement(packagers);
   }
@@ -779,12 +775,12 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
    * settings.
    */
   private static Packager newDwpAction(
-      CppConfiguration cppConfiguration, NestedSet<Artifact> dwpTools) {
+      CcToolchainProvider toolchain, NestedSet<Artifact> dwpTools) {
     Packager packager = new Packager();
     packager
         .spawnAction
         .addTransitiveInputs(dwpTools)
-        .setExecutable(cppConfiguration.getDwpExecutable());
+        .setExecutable(toolchain.getToolPathFragment(Tool.DWP));
     return packager;
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java
index a9b82e4982..3283d815f9 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java
@@ -48,6 +48,7 @@ import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.packages.License;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.Builder;
+import com.google.devtools.build.lib.rules.cpp.CppConfiguration.Tool;
 import com.google.devtools.build.lib.rules.cpp.FdoSupport.FdoException;
 import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.util.FileType;
@@ -99,7 +100,10 @@ public class CcToolchain implements RuleConfiguredTargetFactory {
    * the indexed format (.profdata) if necessary.
    */
   private Artifact convertLLVMRawProfileToIndexed(
-      Path fdoProfile, CppConfiguration cppConfiguration, RuleContext ruleContext)
+      Path fdoProfile,
+      CppToolchainInfo toolchainInfo,
+      CppConfiguration cppConfiguration,
+      RuleContext ruleContext)
       throws InterruptedException {
 
     Artifact profileArtifact =
@@ -183,7 +187,7 @@ public class CcToolchain implements RuleConfiguredTargetFactory {
               "Symlinking LLVM Raw Profile " + fdoProfile.getPathString()));
     }
 
-    if (cppConfiguration.getLLVMProfDataExecutable() == null) {
+    if (toolchainInfo.getToolPathFragment(Tool.LLVM_PROFDATA) == null) {
       ruleContext.ruleError(
           "llvm-profdata not available with this crosstool, needed for profile conversion");
       return null;
@@ -196,7 +200,7 @@ public class CcToolchain implements RuleConfiguredTargetFactory {
             .addTransitiveInputs(getFiles(ruleContext, "all_files"))
             .addOutput(profileArtifact)
             .useDefaultShellEnvironment()
-            .setExecutable(cppConfiguration.getLLVMProfDataExecutable())
+            .setExecutable(toolchainInfo.getToolPathFragment(Tool.LLVM_PROFDATA))
             .setProgressMessage("LLVMProfDataAction: Generating %s", profileArtifact.prettyPrint())
             .setMnemonic("LLVMProfDataAction")
             .addCommandLine(
@@ -248,15 +252,6 @@ public class CcToolchain implements RuleConfiguredTargetFactory {
       return null;
     }
 
-    // This tries to convert LLVM profiles to the indexed format if necessary.
-    Artifact profileArtifact = null;
-    if (cppConfiguration.isLLVMOptimizedFdo()) {
-      profileArtifact = convertLLVMRawProfileToIndexed(fdoZip, cppConfiguration, ruleContext);
-      if (ruleContext.hasErrors()) {
-        return null;
-      }
-    }
-
     final Label label = ruleContext.getLabel();
     final NestedSet<Artifact> crosstool = ruleContext.getPrerequisite("all_files", Mode.HOST)
         .getProvider(FileProvider.class).getFilesToBuild();
@@ -351,13 +346,6 @@ public class CcToolchain implements RuleConfiguredTargetFactory {
 
     NestedSetBuilder<Pair<String, String>> coverageEnvironment = NestedSetBuilder.compileOrder();
 
-    coverageEnvironment.add(Pair.of(
-        "COVERAGE_GCOV_PATH", cppConfiguration.getGcovExecutable().getPathString()));
-    if (cppConfiguration.getFdoInstrument() != null) {
-      coverageEnvironment.add(Pair.of(
-          "FDO_DIR", cppConfiguration.getFdoInstrument().getPathString()));
-    }
-
     NestedSet<Artifact> coverage = getOptionalFiles(ruleContext, "coverage_files");
     if (coverage.isEmpty()) {
       coverage = crosstool;
@@ -397,6 +385,24 @@ public class CcToolchain implements RuleConfiguredTargetFactory {
       toolchainInfo = cppConfiguration.getCppToolchainInfo();
     }
 
+    coverageEnvironment.add(
+        Pair.of(
+            "COVERAGE_GCOV_PATH", toolchainInfo.getToolPathFragment(Tool.GCOV).getPathString()));
+    if (cppConfiguration.getFdoInstrument() != null) {
+      coverageEnvironment.add(
+          Pair.of("FDO_DIR", cppConfiguration.getFdoInstrument().getPathString()));
+    }
+
+    // This tries to convert LLVM profiles to the indexed format if necessary.
+    Artifact profileArtifact = null;
+    if (cppConfiguration.isLLVMOptimizedFdo()) {
+      profileArtifact =
+          convertLLVMRawProfileToIndexed(fdoZip, toolchainInfo, cppConfiguration, ruleContext);
+      if (ruleContext.hasErrors()) {
+        return null;
+      }
+    }
+
     CcToolchainProvider ccProvider =
         new CcToolchainProvider(
             cppConfiguration,
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 5a6dec8dda..5296948c6a 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
@@ -1338,74 +1338,31 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
     doc = "Path to GNU binutils 'objcopy' binary."
   )
   public String getObjCopyExecutableForSkylark() {
-    PathFragment objCopyExecutable = getObjCopyExecutable();
+    PathFragment objCopyExecutable = getToolPathFragment(Tool.OBJCOPY);
     return objCopyExecutable != null ? objCopyExecutable.getPathString() : "";
   }
 
-  /**
-   * Returns the path to the GNU binutils 'objcopy' binary to use for this build. (Corresponds to
-   * $(OBJCOPY) in make-dbg.) Relative paths are relative to the execution root.
-   */
-  public PathFragment getObjCopyExecutable() {
-    return getToolPathFragment(CppConfiguration.Tool.OBJCOPY);
-  }
-
   @SkylarkCallable(
     name = "compiler_executable",
     structField = true,
     doc = "Path to C/C++ compiler binary."
   )
   public String getCppExecutableForSkylark() {
-    PathFragment cppExecutable = getCppExecutable();
+    PathFragment cppExecutable = getToolPathFragment(Tool.GCC);
     return cppExecutable != null ? cppExecutable.getPathString() : "";
   }
 
-  /**
-   * Returns the path to the GNU binutils 'gcc' binary that should be used by this build. This
-   * binary should support compilation of both C (*.c) and C++ (*.cc) files. Relative paths are
-   * relative to the execution root.
-   */
-  public PathFragment getCppExecutable() {
-    return getToolPathFragment(CppConfiguration.Tool.GCC);
-  }
-
-  /**
-   * Returns the path to the GNU binutils 'g++' binary that should be used
-   * by this build.  This binary should support linking of both C (*.c)
-   * and C++ (*.cc) files. Relative paths are relative to the execution root.
-   */
-  public PathFragment getCppLinkExecutable() {
-    return getToolPathFragment(CppConfiguration.Tool.GCC);
-  }
-
   @SkylarkCallable(
     name = "preprocessor_executable",
     structField = true,
     doc = "Path to C/C++ preprocessor binary."
   )
   public String getCpreprocessorExecutableForSkylark() {
-    PathFragment cpreprocessorExecutable = getCpreprocessorExecutable();
+    PathFragment cpreprocessorExecutable = getToolPathFragment(Tool.CPP);
     return cpreprocessorExecutable != null ? cpreprocessorExecutable.getPathString() : "";
   }
 
   /**
-   * Returns the path to the GNU binutils 'cpp' binary that should be used by this build. Relative
-   * paths are relative to the execution root.
-   */
-  public PathFragment getCpreprocessorExecutable() {
-    return getToolPathFragment(CppConfiguration.Tool.CPP);
-  }
-
-  /**
-   * Returns the path to the GNU binutils 'gcov' binary that should be used
-   * by this build to analyze C++ coverage data. Relative paths are relative to
-   * the execution root.
-   */
-  public PathFragment getGcovExecutable() {
-    return getToolPathFragment(CppConfiguration.Tool.GCOV);
-  }
-
-  /**
    * Returns the path to the 'gcov-tool' executable that should be used
    * by this build. Relative paths are relative to the execution root.
    */
@@ -1419,87 +1376,41 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
     doc = "Path to GNU binutils 'nm' binary."
   )
   public String getNmExecutableForSkylark() {
-    PathFragment nmExecutable = getNmExecutable();
+    PathFragment nmExecutable = getToolPathFragment(Tool.NM);
     return nmExecutable != null ? nmExecutable.getPathString() : "";
   }
 
-  /**
-   * Returns the path to the GNU binutils 'nm' executable that should be used by this build. Used
-   * only for testing. Relative paths are relative to the execution root.
-   */
-  public PathFragment getNmExecutable() {
-    return getToolPathFragment(CppConfiguration.Tool.NM);
-  }
-
   @SkylarkCallable(
     name = "objdump_executable",
     structField = true,
     doc = "Path to GNU binutils 'objdump' binary."
   )
   public String getObjdumpExecutableForSkylark() {
-    PathFragment objdumpExecutable = getObjdumpExecutable();
+    PathFragment objdumpExecutable = getToolPathFragment(Tool.OBJDUMP);
     return objdumpExecutable != null ? objdumpExecutable.getPathString() : "";
   }
 
-  /**
-   * Returns the path to the GNU binutils 'objdump' executable that should be used by this build.
-   * Used only for testing. Relative paths are relative to the execution root.
-   */
-  public PathFragment getObjdumpExecutable() {
-    return getToolPathFragment(CppConfiguration.Tool.OBJDUMP);
-  }
-
   @SkylarkCallable(
     name = "ar_executable",
     structField = true,
     doc = "Path to GNU binutils 'ar' binary."
   )
   public String getArExecutableForSkylark() {
-    PathFragment arExecutable = getArExecutable();
+    PathFragment arExecutable = getToolPathFragment(Tool.AR);
     return arExecutable != null ? arExecutable.getPathString() : "";
   }
 
-  /**
-   * Returns the path to the GNU binutils 'ar' binary to use for this build. Relative paths are
-   * relative to the execution root.
-   */
-  public PathFragment getArExecutable() {
-    return getToolPathFragment(CppConfiguration.Tool.AR);
-  }
-
   @SkylarkCallable(
     name = "strip_executable",
     structField = true,
     doc = "Path to GNU binutils 'strip' binary."
   )
   public String getStripExecutableForSkylark() {
-    PathFragment stripExecutable = getStripExecutable();
+    PathFragment stripExecutable = getToolPathFragment(Tool.STRIP);
     return stripExecutable != null ? stripExecutable.getPathString() : "";
   }
 
   /**
-   * Returns the path to the GNU binutils 'strip' executable that should be used by this build.
-   * Relative paths are relative to the execution root.
-   */
-  public PathFragment getStripExecutable() {
-    return getToolPathFragment(CppConfiguration.Tool.STRIP);
-  }
-
-  /**
-   * Returns the path to the GNU binutils 'dwp' binary that should be used by this
-   * build to combine debug info output from individual C++ compilations (i.e. .dwo
-   * files) into aggregate target-level debug packages. Relative paths are relative to the
-   * execution root. See https://gcc.gnu.org/wiki/DebugFission .
-   */
-  public PathFragment getDwpExecutable() {
-    return getToolPathFragment(CppConfiguration.Tool.DWP);
-  }
-
-  public PathFragment getLLVMProfDataExecutable() {
-    return getToolPathFragment(CppConfiguration.Tool.LLVM_PROFDATA);
-  }
-
-  /**
    * Returns the GNU System Name
    */
   @SkylarkCallable(name = "target_gnu_system_name", structField = true,
@@ -1620,20 +1531,20 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
   @Override
   public void addGlobalMakeVariables(Builder<String, String> globalMakeEnvBuilder) {
     // hardcoded CC->gcc setting for unit tests
-    globalMakeEnvBuilder.put("CC", getCppExecutable().getPathString());
+    globalMakeEnvBuilder.put("CC", getToolPathFragment(Tool.GCC).getPathString());
 
     // Make variables provided by crosstool/gcc compiler suite.
-    globalMakeEnvBuilder.put("AR", getArExecutable().getPathString());
-    globalMakeEnvBuilder.put("NM", getNmExecutable().getPathString());
+    globalMakeEnvBuilder.put("AR", getToolPathFragment(Tool.AR).getPathString());
+    globalMakeEnvBuilder.put("NM", getToolPathFragment(Tool.NM).getPathString());
     globalMakeEnvBuilder.put("LD", getToolPathFragment(Tool.LD).getPathString());
-    PathFragment objcopyTool = getObjCopyExecutable();
+    PathFragment objcopyTool = getToolPathFragment(Tool.OBJCOPY);
     if (objcopyTool != null) {
       // objcopy is optional in Crosstool
       globalMakeEnvBuilder.put("OBJCOPY", objcopyTool.getPathString());
     }
-    globalMakeEnvBuilder.put("STRIP", getStripExecutable().getPathString());
+    globalMakeEnvBuilder.put("STRIP", getToolPathFragment(Tool.STRIP).getPathString());
 
-    PathFragment gcovtool = getGcovToolExecutable();
+    PathFragment gcovtool = getToolPathFragment(Tool.GCOVTOOL);
     if (gcovtool != null) {
       // gcov-tool is optional in Crosstool
       globalMakeEnvBuilder.put("GCOVTOOL", gcovtool.getPathString());
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendArtifacts.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendArtifacts.java
index 97774974b8..6c714c560a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendArtifacts.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendArtifacts.java
@@ -20,6 +20,7 @@ import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables;
+import com.google.devtools.build.lib.rules.cpp.CppConfiguration.Tool;
 import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -157,9 +158,7 @@ public final class LtoBackendArtifacts {
 
     // The command-line doesn't specify the full path to clang++, so we set it in the
     // environment.
-    CppConfiguration cppConfiguration = ruleContext.getFragment(CppConfiguration.class);
-
-    PathFragment compiler = cppConfiguration.getCppExecutable();
+    PathFragment compiler = ccToolchain.getToolPathFragment(Tool.GCC);
 
     builder.setExecutable(compiler);
     Variables.Builder buildVariablesBuilder =
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleConfiguredTargetTest.java
index 15923bf164..a68b5450f5 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleConfiguredTargetTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleConfiguredTargetTest.java
@@ -32,7 +32,9 @@ import com.google.devtools.build.lib.analysis.configuredtargets.FileConfiguredTa
 import com.google.devtools.build.lib.analysis.util.AnalysisMock;
 import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
 import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.rules.cpp.CppConfiguration;
+import com.google.devtools.build.lib.rules.cpp.CcToolchainProvider;
+import com.google.devtools.build.lib.rules.cpp.CppConfiguration.Tool;
+import com.google.devtools.build.lib.rules.cpp.CppHelper;
 import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.List;
@@ -317,7 +319,10 @@ public class GenRuleConfiguredTargetTest extends BuildViewTestCase {
 
     SpawnAction barAction = (SpawnAction) getGeneratingAction(barOutTarget.getArtifact());
 
-    String cc = "" + targetConfig.getFragment(CppConfiguration.class).getCppExecutable();
+    CcToolchainProvider toolchain =
+        CppHelper.getToolchainUsingDefaultCcToolchainAttribute(
+            getRuleContext(getConfiguredTarget("//foo:bar")));
+    String cc = toolchain.getToolPathFragment(Tool.GCC).getPathString();
     String expected =
         cc
             + " -o "