Align action configs with legacy ones to allow LtoBackend actions to succeed

[dzbarsky]

This matches how it was previously setup:

rules_cc/cc/private/toolchain_config/legacy_features.bzl

Lines 169 to 187 in 9338446

	result.append(feature(
	name = "preprocessor_defines",
	enabled = True,
	flag_sets = [flag_set(
	actions = [
	ACTION_NAMES.c_compile,
	ACTION_NAMES.cpp_compile,
	ACTION_NAMES.cpp_header_parsing,
	ACTION_NAMES.cpp_module_compile,
	ACTION_NAMES.clif_match,
	ACTION_NAMES.linkstamp_compile,
	ACTION_NAMES.preprocess_assemble,
	],
	flag_groups = [flag_group(
	iterate_over = "preprocessor_defines",
	flags = ["-D%{preprocessor_defines}"],
	)],
	)],
	))

Without this, we get:

FATAL: bazel crashed due to an internal error. Printing stack trace:
java.lang.AssertionError: LtoBackendAction command line expansion cannot fail
	at com.google.devtools.build.lib.rules.cpp.LtoBackendAction.computeKey(LtoBackendAction.java:288)
	at com.google.devtools.build.lib.actions.ActionKeyComputer.getKey(ActionKeyComputer.java:43)
	at com.google.devtools.build.lib.actions.ActionCacheChecker.mustExecute(ActionCacheChecker.java:575)
	at com.google.devtools.build.lib.actions.ActionCacheChecker.getTokenIfNeedToExecute(ActionCacheChecker.java:507)
	at com.google.devtools.build.lib.skyframe.SkyframeActionExecutor.checkActionCache(SkyframeActionExecutor.java:764)
	at com.google.devtools.build.lib.skyframe.ActionExecutionFunction.checkCacheAndExecuteIfNeeded(ActionExecutionFunction.java:706)
	at com.google.devtools.build.lib.skyframe.ActionExecutionFunction.computeInternal(ActionExecutionFunction.java:345)
	at com.google.devtools.build.lib.skyframe.ActionExecutionFunction.compute(ActionExecutionFunction.java:202)
	at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:471)
	at com.google.devtools.build.lib.concurrent.AbstractQueueVisitor$WrappedRunnable.run(AbstractQueueVisitor.java:435)
	at java.base/java.util.concurrent.ForkJoinTask$RunnableExecuteAction.compute(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinTask$RunnableExecuteAction.compute(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinTask$InterruptibleTask.exec(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinTask.doExec(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinPool.runWorker(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinWorkerThread.run(Unknown Source)
Caused by: com.google.devtools.build.lib.actions.CommandLineExpansionException: Invalid toolchain configuration: Cannot find variable named 'preprocessor_defines'.
	at com.google.devtools.build.lib.rules.cpp.LtoBackendArtifacts$1.arguments(LtoBackendArtifacts.java:129)
	at com.google.devtools.build.lib.rules.cpp.LtoBackendArtifacts$1.arguments(LtoBackendArtifacts.java:109)
	at com.google.devtools.build.lib.actions.CommandLines.allArguments(CommandLines.java:171)
	at com.google.devtools.build.lib.analysis.actions.SpawnAction.getArguments(SpawnAction.java:208)
	at com.google.devtools.build.lib.rules.cpp.LtoBackendAction.computeKey(LtoBackendAction.java:286)

I now have this fully working e2e at https://github.com/cerisier/toolchains_llvm_bootstrapped/pull/56/files#diff-cce4f0083471dff12279a3b4e3b3206404e4932a3d60f3443c0f60900706093e so happy to contribute those feature targets as well if someone tells me where to put them :)

[keith]

i was testing a local version of this with:

cc_action_type_set(
    name = "non_lto_compile_actions",
    actions = [
        ":assembly_actions",
        ":c_compile_actions",
        ":clif_match",
        ":cpp_compile",
        ":cpp_header_parsing",
        ":cpp_module_codegen",
        ":cpp_module_compile",
        ":linkstamp_compile",
        ":objc_compile",
        ":objcpp_compile",
    ],
)

not sure if it's entirely correct tho

[dzbarsky]

certainly seems more correct :) I'm not sure why the legacy configs don't include the objc actions (and I guess cpp_module actions are too new and legacy config was never updated?)

I don't have super strong opinions here, happy to do whatever's best as long as we can LTO working properly

[armandomontanez]

Internally, the list I'm seeing says this:

"preprocess-assemble",
"linkstamp-compile",
"c-compile",
"c++-compile",
"c++-header-parsing",
"c++-header-analysis",
"c++-module-compile",
"clif-match",
"objc-compile",
"objc++-compile",

Which omits cpp_module_codegen and adds c++-header-analysis. There are few people that actually understand all of this. @lberki might know who to ask to understand the nuances.

[dzbarsky]

I think the header analysis stuff may have not made it into open-source world? (See https://github.com/search?q=repo%3Abazelbuild%2Frules_cc+cpp_header_analysis&type=code and https://github.com/search?q=repo%3Abazelbuild%2Fbazel%20cpp_header_analysis&type=code). I guess it doesn't hurt to add if Bazel/rules_cc aren't generating these actions anyway, though I also thought the the header analysis isn't aware of the preprocessor. Or maybe that's the header parsing. Or something else I'm misremembering :)

[armandomontanez]

Good opportunity to update //cc/toolchains/variables/BUILD to ensure the problematic variables cannot be used by argument collections that apply to lto_backend.

[armandomontanez]

To make this as clear as possible:

• Place next to compile_actions
• Leave a comment that this should match compile_actions but omit the lto_backend action, and highlight the intent
• Perhaps rename to source_compile_actions

[dzbarsky]

thanks for the suggestions, addressed!