Move all crosstool patches to CppActionConfigs

As the ordering in which features are defined matters, let's move all the
crosstool patching logic to a single place in CppActionConfigs.

RELNOTES: None.
PiperOrigin-RevId: 162712170

2 files changed, 254 insertions, 272 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java
index 5229ddf19f..d904494a35 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java
@@ -55,7 +55,6 @@ public class CppActionConfigs {
               + "  }"
               + "}";
     }
-
     return Joiner.on("\n")
         .join(
             ImmutableList.of(
@@ -115,6 +114,194 @@ public class CppActionConfigs {
                 "    tool_path: '" + gccToolPath + "'",
                 "  }",
                 "}",
+                  // Gcc options:
+                  //  -MD turns on .d file output as a side-effect (doesn't imply -E)
+                  //  -MM[D] enables user includes only, not system includes
+                  //  -MF <name> specifies the dotd file name
+                  // Issues:
+                  //  -M[M] alone subverts actual .o output (implies -E)
+                  //  -M[M]D alone breaks some of the .d naming assumptions
+                  // This combination gets user and system includes with specified name:
+                  //  -MD -MF <name>
+                  "feature {",
+                  "  name: 'dependency_file'",
+                  "  flag_set {",
+                  "    action: 'assemble'",
+                  "    action: 'preprocess-assemble'",
+                  "    action: 'c-compile'",
+                  "    action: 'c++-compile'",
+                  "    action: 'c++-module-compile'",
+                  "    action: 'objc-compile'",
+                  "    action: 'objc++-compile'",
+                  "    action: 'c++-header-preprocessing'",
+                  "    action: 'c++-header-parsing'",
+                  "    action: 'clif-match'",
+                  "    expand_if_all_available: 'dependency_file'",
+                  "    flag_group {",
+                  "      flag: '-MD'",
+                  "      flag: '-MF'",
+                  "      flag: '%{dependency_file}'",
+                  "    }",
+                  "  }",
+                  "}",
+                  // GCC and Clang give randomized names to symbols which are defined in
+                  // an anonymous namespace but have external linkage.  To make
+                  // computation of these deterministic, we want to override the
+                  // default seed for the random number generator.  It's safe to use
+                  // any value which differs for all translation units; we use the
+                  // path to the object file.
+                  "feature {",
+                  "  name: 'random_seed'",
+                  "  flag_set {",
+                  "    action: 'c++-compile'",
+                  "    action: 'c++-module-codegen'",
+                  "    action: 'c++-module-compile'",
+                  "    flag_group {",
+                  "      flag: '-frandom-seed=%{output_file}'",
+                  "    }",
+                  "  }",
+                  "}",
+                   "feature {",
+                   "  name: 'pic'",
+                   "  flag_set {",
+                   "    action: 'assemble'",
+                   "    action: 'preprocess-assemble'",
+                   "    action: 'c-compile'",
+                   "    action: 'c++-compile'",
+                   "    action: 'c++-module-codegen'",
+                   "    action: 'c++-module-compile'",
+                   "    expand_if_all_available: 'pic'",
+                   "    flag_group {",
+                   "      flag: '-fPIC'",
+                   "    }",
+                   "  }",
+                   "}",
+                ifTrue(
+                    !features.contains(CppRuleClasses.PER_OBJECT_DEBUG_INFO),
+                    "feature {",
+                    "  name: 'per_object_debug_info'",
+                    "  flag_set {",
+                    "    action: 'assemble'",
+                    "    action: 'preprocess-assemble'",
+                    "    action: 'c-compile'",
+                    "    action: 'c++-compile'",
+                    "    action: 'c++-module-codegen'",
+                    "    expand_if_all_available: 'per_object_debug_info_file'",
+                    "    flag_group {",
+                    "      flag: '-gsplit-dwarf'",
+                    "    }",
+                    "  }",
+                    "}"),
+                  "feature {",
+                  "  name: 'preprocessor_defines'",
+                  "  flag_set {",
+                  "    action: 'preprocess-assemble'",
+                  "    action: 'c-compile'",
+                  "    action: 'c++-compile'",
+                  "    action: 'c++-header-parsing'",
+                  "    action: 'c++-header-preprocessing'",
+                  "    action: 'c++-module-compile'",
+                  "    action: 'clif-match'",
+                  "    flag_group {",
+                  "      iterate_over: 'preprocessor_defines'",
+                  "      flag: '-D%{preprocessor_defines}'",
+                  "    }",
+                  "  }",
+                  "}",
+                ifTrue(
+                    !features.contains(CppRuleClasses.INCLUDE_PATHS),
+                    "feature {",
+                    "  name: 'include_paths'",
+                    "  flag_set {",
+                    "    action: 'preprocess-assemble'",
+                    "    action: 'c-compile'",
+                    "    action: 'c++-compile'",
+                    "    action: 'c++-header-parsing'",
+                    "    action: 'c++-header-preprocessing'",
+                    "    action: 'c++-module-compile'",
+                    "    action: 'clif-match'",
+                    "    action: 'objc-compile'",
+                    "    action: 'objc++-compile'",
+                    "    flag_group {",
+                    "      iterate_over: 'quote_include_paths'",
+                    "      flag: '-iquote'",
+                    "      flag: '%{quote_include_paths}'",
+                    "    }",
+                    "    flag_group {",
+                    "      iterate_over: 'include_paths'",
+                    "      flag: '-I%{include_paths}'",
+                    "    }",
+                    "    flag_group {",
+                    "      iterate_over: 'system_include_paths'",
+                    "      flag: '-isystem'",
+                    "      flag: '%{system_include_paths}'",
+                    "    }",
+                    "  }",
+                    "}"),
+                ifTrue(
+                    !features.contains(CppRuleClasses.FDO_INSTRUMENT),
+                    "feature {",
+                    "  name: 'fdo_instrument'",
+                    "  provides: 'profile'",
+                    "  flag_set {",
+                    "    action: 'c-compile'",
+                    "    action: 'c++-compile'",
+                    "    action: 'c++-link-interface-dynamic-library'",
+                    "    action: 'c++-link-dynamic-library'",
+                    "    action: 'c++-link-executable'",
+                    "    flag_group {",
+                    "      flag: '-fprofile-generate=%{fdo_instrument_path}'",
+                    "      flag: '-fno-data-sections'",
+                    "    }",
+                    "  }",
+                    "}"),
+                ifTrue(
+                    !features.contains(CppRuleClasses.FDO_OPTIMIZE),
+                    "feature {",
+                    "  name: 'fdo_optimize'",
+                    "  provides: 'profile'",
+                    "  flag_set {",
+                    "    action: 'c-compile'",
+                    "    action: 'c++-compile'",
+                    "    expand_if_all_available: 'fdo_profile_path'",
+                    "    flag_group {",
+                    "      flag: '-fprofile-use=%{fdo_profile_path}'",
+                    "      flag: '-Xclang-only=-Wno-profile-instr-unprofiled'",
+                    "      flag: '-Xclang-only=-Wno-profile-instr-out-of-date'",
+                    "      flag: '-fprofile-correction'",
+                    "    }",
+                    "  }",
+                    "}"),
+                ifTrue(
+                    !features.contains(CppRuleClasses.AUTOFDO),
+                    "feature {",
+                    "  name: 'autofdo'",
+                    "  provides: 'profile'",
+                    "  flag_set {",
+                    "    action: 'c-compile'",
+                    "    action: 'c++-compile'",
+                    "    expand_if_all_available: 'fdo_profile_path'",
+                    "    flag_group {",
+                    "      flag: '-fauto-profile=%{fdo_profile_path}'",
+                    "      flag: '-fprofile-correction'",
+                    "    }",
+                    "  }",
+                    "}"),
+                ifTrue(
+                    !features.contains(CppRuleClasses.LIPO),
+                    "feature {",
+                    "  name: 'lipo'",
+                    "  requires { feature: 'autofdo' }",
+                    "  requires { feature: 'fdo_optimize' }",
+                    "  requires { feature: 'fdo_instrument' }",
+                    "  flag_set {",
+                    "    action: 'c-compile'",
+                    "    action: 'c++-compile'",
+                    "    flag_group {",
+                    "      flag: '-fripa'",
+                    "    }",
+                    "  }",
+                    "}"),
                 "action_config {",
                 "  config_name: 'c++-link-executable'",
                 "  action_name: 'c++-link-executable'",
@@ -567,7 +754,72 @@ public class CppActionConfigs {
                 "      flag: '@%{linker_param_file}'",
                 "    }",
                 "  }",
-                "}"));
+                "}",
+                ifTrue(
+                    !features.contains(CppRuleClasses.COVERAGE),
+                    "feature {",
+                    "  name: 'coverage'",
+                    "}",
+                    "feature {",
+                    "  name: 'llvm_coverage_map_format'",
+                    "  provides: 'profile'",
+                    "  flag_set {",
+                    "    action: 'preprocess-assemble'",
+                    "    action: 'c-compile'",
+                    "    action: 'c++-compile'",
+                    "    action: 'c++-module-compile'",
+                    "    action: 'objc-compile'",
+                    "    action: 'objc++-compile'",
+                    "    flag_group {",
+                    "      flag: '-fprofile-instr-generate'",
+                    "      flag: '-fcoverage-mapping'",
+                    "      flag: '-g'",
+                    "    }",
+                    "  }",
+                    "  flag_set {",
+                    "    action: 'c++-link-interface-dynamic-library'",
+                    "    action: 'c++-link-dynamic-library'",
+                    "    action: 'c++-link-executable'",
+                    "    action: 'objc-executable'",
+                    "    action: 'objc++-executable'",
+                    "    flag_group {",
+                    "      flag: '-fprofile-instr-generate'",
+                    "    }",
+                    "  }",
+                    "  requires {",
+                    "    feature: 'coverage'",
+                    "  }",
+                    "}",
+                    "feature {",
+                    "  name: 'gcc_coverage_map_format'",
+                    "  provides: 'profile'",
+                    "  flag_set {",
+                    "    action: 'preprocess-assemble'",
+                    "    action: 'c-compile'",
+                    "    action: 'c++-compile'",
+                    "    action: 'c++-module-compile'",
+                    "    action: 'objc-compile'",
+                    "    action: 'objc++-compile'",
+                    "    action: 'objc-executable'",
+                    "    action: 'objc++-executable'",
+                    "    flag_group {",
+                    "      flag: '-fprofile-arcs'",
+                    "      flag: '-ftest-coverage'",
+                    "      flag: '-g'",
+                    "    }",
+                    "  }",
+                    "  flag_set {",
+                    "    action: 'c++-link-interface-dynamic-library'",
+                    "    action: 'c++-link-dynamic-library'",
+                    "    action: 'c++-link-executable'",
+                    "    flag_group {",
+                    "      flag: '-lgcov'",
+                    "    }",
+                    "  }",
+                    "  requires {",
+                    "    feature: 'coverage'",
+                    "  }",
+                    "}")));
   }
 
   private static String ifLinux(CppPlatform platform, String... lines) {
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 89f82b292d..8d00fc9ff5 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
@@ -659,277 +659,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
                 arToolPath,
                 supportsEmbeddedRuntimes,
                 toolchain.getSupportsInterfaceSharedObjects()),
-            toolchainBuilder);
-
-        if (!features.contains("dependency_file")) {
-          // Gcc options:
-          //  -MD turns on .d file output as a side-effect (doesn't imply -E)
-          //  -MM[D] enables user includes only, not system includes
-          //  -MF <name> specifies the dotd file name
-          // Issues:
-          //  -M[M] alone subverts actual .o output (implies -E)
-          //  -M[M]D alone breaks some of the .d naming assumptions
-          // This combination gets user and system includes with specified name:
-          //  -MD -MF <name>
-          TextFormat.merge(
-              ""
-                  + "feature {"
-                  + "  name: 'dependency_file'"
-                  + "  flag_set {"
-                  + "    action: 'assemble'"
-                  + "    action: 'preprocess-assemble'"
-                  + "    action: 'c-compile'"
-                  + "    action: 'c++-compile'"
-                  + "    action: 'c++-module-compile'"
-                  + "    action: 'objc-compile'"
-                  + "    action: 'objc++-compile'"
-                  + "    action: 'c++-header-preprocessing'"
-                  + "    action: 'c++-header-parsing'"
-                  + "    action: 'clif-match'"
-                  + "    expand_if_all_available: 'dependency_file'"
-                  + "    flag_group {"
-                  + "      flag: '-MD'"
-                  + "      flag: '-MF'"
-                  + "      flag: '%{dependency_file}'"
-                  + "    }"
-                  + "  }"
-                  + "}",
               toolchainBuilder);
-        }
-
-        if (!features.contains("random_seed")) {
-          // GCC and Clang give randomized names to symbols which are defined in
-          // an anonymous namespace but have external linkage.  To make
-          // computation of these deterministic, we want to override the
-          // default seed for the random number generator.  It's safe to use
-          // any value which differs for all translation units; we use the
-          // path to the object file.
-          TextFormat.merge(
-              ""
-                  + "feature {"
-                  + "  name: 'random_seed'"
-                  + "  flag_set {"
-                  + "    action: 'c++-compile'"
-                  + "    action: 'c++-module-codegen'"
-                  + "    action: 'c++-module-compile'"
-                  + "    flag_group {"
-                  + "      flag: '-frandom-seed=%{output_file}'"
-                  + "    }"
-                  + "  }"
-                  + "}",
-              toolchainBuilder);
-        }
-
-        if (!features.contains("pic")) {
-          TextFormat.merge(
-              ""
-                  + "feature {"
-                  + "  name: 'pic'"
-                  + "  flag_set {"
-                  + "    action: 'c-compile'"
-                  + "    action: 'c++-compile'"
-                  + "    action: 'c++-module-codegen'"
-                  + "    action: 'c++-module-compile'"
-                  + "    action: 'preprocess-assemble'"
-                  + "    expand_if_all_available: 'pic'"
-                  + "    flag_group {"
-                  + "      flag: '-fPIC'"
-                  + "    }"
-                  + "  }"
-                  + "}",
-              toolchainBuilder);
-        }
-
-        if (!features.contains("per_object_debug_info")) {
-          TextFormat.merge(
-              ""
-                  + "feature {"
-                  + "  name: 'per_object_debug_info'"
-                  + "  flag_set {"
-                  + "    action: 'c-compile'"
-                  + "    action: 'c++-compile'"
-                  + "    action: 'c++-module-codegen'"
-                  + "    action: 'assemble'"
-                  + "    action: 'preprocess-assemble'"
-                  + "    action: 'lto-backend'"
-                  + "    expand_if_all_available: 'per_object_debug_info_file'"
-                  + "    flag_group {"
-                  + "      flag: '-gsplit-dwarf'"
-                  + "    }"
-                  + "  }"
-                  + "}",
-              toolchainBuilder);
-        }
-
-        if (!features.contains("preprocessor_defines")) {
-          TextFormat.merge(
-              ""
-                  + "feature {"
-                  + "  name: 'preprocessor_defines'"
-                  + "  flag_set {"
-                  + "    action: 'preprocess-assemble'"
-                  + "    action: 'c-compile'"
-                  + "    action: 'c++-compile'"
-                  + "    action: 'c++-header-parsing'"
-                  + "    action: 'c++-header-preprocessing'"
-                  + "    action: 'c++-module-compile'"
-                  + "    action: 'clif-match'"
-                  + "    flag_group {"
-                  + "      iterate_over: 'preprocessor_defines'"
-                  + "      flag: '-D%{preprocessor_defines}'"
-                  + "    }"
-                  + "  }"
-                  + "}",
-              toolchainBuilder);
-        }
-        if (!features.contains("include_paths")) {
-          TextFormat.merge(
-              ""
-                  + "feature {"
-                  + "  name: 'include_paths'"
-                  + "  flag_set {"
-                  + "    action: 'preprocess-assemble'"
-                  + "    action: 'c-compile'"
-                  + "    action: 'c++-compile'"
-                  + "    action: 'c++-header-parsing'"
-                  + "    action: 'c++-header-preprocessing'"
-                  + "    action: 'c++-module-compile'"
-                  + "    action: 'clif-match'"
-                  + "    action: 'objc-compile'"
-                  + "    action: 'objc++-compile'"
-                  + "    flag_group {"
-                  + "      iterate_over: 'quote_include_paths'"
-                  + "      flag: '-iquote'"
-                  + "      flag: '%{quote_include_paths}'"
-                  + "    }"
-                  + "    flag_group {"
-                  + "      iterate_over: 'include_paths'"
-                  + "      flag: '-I%{include_paths}'"
-                  + "    }"
-                  + "    flag_group {"
-                  + "      iterate_over: 'system_include_paths'"
-                  + "      flag: '-isystem'"
-                  + "      flag: '%{system_include_paths}'"
-                  + "    }"
-                  + "  }"
-                  + "}",
-              toolchainBuilder);
-        }
-        if (!features.contains("fdo_instrument")) {
-          TextFormat.merge(
-              ""
-                  + "feature {"
-                  + "  name: 'fdo_instrument'"
-                  + "  provides: 'profile'"
-                  + "  flag_set {"
-                  + "    action: 'c-compile'"
-                  + "    action: 'c++-compile'"
-                  + "    action: 'c++-link-interface-dynamic-library'"
-                  + "    action: 'c++-link-dynamic-library'"
-                  + "    action: 'c++-link-executable'"
-                  + "    flag_group {"
-                  + "      flag: '-fprofile-generate=%{fdo_instrument_path}'"
-                  + "      flag: '-fno-data-sections'"
-                  + "    }"
-                  + "  }"
-                  + "}",
-              toolchainBuilder);
-        }
-        if (!features.contains("fdo_optimize")) {
-          TextFormat.merge(
-              ""
-                  + "feature {"
-                  + "  name: 'fdo_optimize'"
-                  + "  provides: 'profile'"
-                  + "  flag_set {"
-                  + "    action: 'c-compile'"
-                  + "    action: 'c++-compile'"
-                  + "    expand_if_all_available: 'fdo_profile_path'"
-                  + "    flag_group {"
-                  + "      flag: '-fprofile-use=%{fdo_profile_path}'"
-                  + "      flag: '-Xclang-only=-Wno-profile-instr-unprofiled'"
-                  + "      flag: '-Xclang-only=-Wno-profile-instr-out-of-date'"
-                  + "      flag: '-fprofile-correction'"
-                  + "    }"
-                  + "  }"
-                  + "}",
-              toolchainBuilder);
-        }
-        if (!features.contains("autofdo")) {
-          TextFormat.merge(
-              ""
-                  + "feature {"
-                  + "  name: 'autofdo'"
-                  + "  provides: 'profile'"
-                  + "  flag_set {"
-                  + "    action: 'c-compile'"
-                  + "    action: 'c++-compile'"
-                  + "    expand_if_all_available: 'fdo_profile_path'"
-                  + "    flag_group {"
-                  + "      flag: '-fauto-profile=%{fdo_profile_path}'"
-                  + "      flag: '-fprofile-correction'"
-                  + "    }"
-                  + "  }"
-                  + "}",
-              toolchainBuilder);
-        }
-        if (!features.contains("lipo")) {
-          TextFormat.merge(
-              ""
-                  + "feature {"
-                  + "  name: 'lipo'"
-                  + "  requires { feature: 'autofdo' }"
-                  + "  requires { feature: 'fdo_optimize' }"
-                  + "  requires { feature: 'fdo_instrument' }"
-                  + "  flag_set {"
-                  + "    action: 'c-compile'"
-                  + "    action: 'c++-compile'"
-                  + "    flag_group {"
-                  + "      flag: '-fripa'"
-                  + "    }"
-                  + "  }"
-                  + "}",
-              toolchainBuilder);
-        }
-        if (!features.contains("coverage")) {
-          String compileFlags;
-          String linkerFlags;
-          if (useLLVMCoverageMap) {
-            compileFlags =
-                "flag_group { flag: '-fprofile-instr-generate' flag: '-fcoverage-mapping' }";
-            linkerFlags = "flag_group { flag: '-fprofile-instr-generate' }";
-          } else {
-            compileFlags =
-                "  expand_if_all_available: 'gcov_gcno_file'"
-                    + "flag_group {"
-                    + "  flag: '-fprofile-arcs'"
-                    + "  flag: '-ftest-coverage'"
-                    + "}";
-            linkerFlags = "  flag_group { flag: '-lgcov' }";
-          }
-          TextFormat.merge(
-              ""
-                  + "feature {"
-                  + "  name: 'coverage'"
-                  + "  provides: 'profile'"
-                  + "  flag_set {"
-                  + "    action: 'preprocess-assemble'"
-                  + "    action: 'c-compile'"
-                  + "    action: 'c++-compile'"
-                  + "    action: 'c++-header-parsing'"
-                  + "    action: 'c++-header-preprocessing'"
-                  + "    action: 'c++-module-compile'"
-                  + compileFlags
-                  + "  }"
-                  + "  flag_set {"
-                  + "    action: 'c++-link-interface-dynamic-library'"
-                  + "    action: 'c++-link-dynamic-library'"
-                  + "    action: 'c++-link-executable'"
-                  + linkerFlags
-                  + "  }"
-                  + "}",
-              toolchainBuilder);
-        }
       } catch (ParseException e) {
         // Can only happen if we change the proto definition without changing our
         // configuration above.