Move FdoProvider under CcToolchainProvider
Currently FdoProfiler is provided by the cc_toolchain separately. This causes
problems to custom skylark rules that store the implicit attribute as
_cc_toolchain, and native rules store it as ':cc_toolchain'. And FdoProvider was
expected to be provided by the ':cc_toolchain'.
Since FdoProvider is used together with the CcToolchainProvider, I'm merging
them together.
#6072.
RELNOTES: None.
PiperOrigin-RevId: 211635916
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 41b0dfd..7ab2102 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
@@ -166,9 +166,7 @@
this.ccToolchain =
Preconditions.checkNotNull(
CppHelper.getToolchainUsingDefaultCcToolchainAttribute(ruleContext));
- this.fdoProvider =
- Preconditions.checkNotNull(
- CppHelper.getFdoProviderUsingDefaultCcToolchainAttribute(ruleContext));
+ this.fdoProvider = ccToolchain.getFdoProvider();
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcImport.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcImport.java
index 14a4092..2ca3dea 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcImport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcImport.java
@@ -69,8 +69,7 @@
CppHelper.getToolchainUsingDefaultCcToolchainAttribute(ruleContext);
FeatureConfiguration featureConfiguration =
CcCommon.configureFeaturesOrReportRuleError(ruleContext, ccToolchain);
- FdoProvider fdoProvider =
- CppHelper.getFdoProviderUsingDefaultCcToolchainAttribute(ruleContext);
+ FdoProvider fdoProvider = ccToolchain.getFdoProvider();
// Add headers to compilation step.
final CcCommon common = new CcCommon(ruleContext);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java
index 1fc0fe2..aa7ea76 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java
@@ -418,8 +418,7 @@
convertFromNoneable(skylarkFeatureConfiguration, null);
Pair<List<Artifact>, List<Artifact>> separatedHeadersAndSources =
separateSourcesFromHeaders(sources);
- FdoProvider fdoProvider =
- CppHelper.getFdoProviderUsingDefaultCcToolchainAttribute(ruleContext);
+ FdoProvider fdoProvider = ccToolchainProvider.getFdoProvider();
// TODO(plf): Need to flatten the nested set to convert the Strings to PathFragment. This could
// be avoided if path fragments are ever added to Skylark or in the C++ code we take Strings
// instead of PathFragments.
@@ -497,8 +496,7 @@
CcToolchainProvider ccToolchainProvider = convertFromNoneable(skylarkCcToolchainProvider, null);
FeatureConfiguration featureConfiguration =
convertFromNoneable(skylarkFeatureConfiguration, null);
- FdoProvider fdoProvider =
- CppHelper.getFdoProviderUsingDefaultCcToolchainAttribute(ruleContext);
+ FdoProvider fdoProvider = ccToolchainProvider.getFdoProvider();
NestedSet<String> linkopts =
convertSkylarkListOrNestedSetToNestedSet(skylarkLinkopts, String.class);
CcLinkingHelper helper =
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 7b2b9e6..b6eae3c 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
@@ -585,6 +585,12 @@
builtInIncludeDirectories,
sysroot,
fdoMode,
+ new FdoProvider(
+ fdoSupport.getFilePath(),
+ fdoMode,
+ cppConfiguration.getFdoInstrument(),
+ profileArtifact,
+ prefetchHintsArtifact),
cppConfiguration.useLLVMCoverageMapFormat(),
configuration.isCodeCoverageEnabled(),
configuration.isHostConfiguration());
@@ -597,13 +603,6 @@
new RuleConfiguredTargetBuilder(ruleContext)
.addNativeDeclaredProvider(ccProvider)
.addNativeDeclaredProvider(templateVariableInfo)
- .addProvider(
- new FdoProvider(
- fdoSupport.getFilePath(),
- fdoMode,
- cppConfiguration.getFdoInstrument(),
- profileArtifact,
- prefetchHintsArtifact))
.setFilesToBuild(crosstool)
.addProvider(RunfilesProvider.simple(Runfiles.EMPTY));
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java
index 5ac50d3..bdeacf0 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java
@@ -81,6 +81,7 @@
/* builtInIncludeDirectories= */ ImmutableList.<PathFragment>of(),
/* sysroot= */ null,
FdoMode.OFF,
+ /* fdoProvider= */ null,
/* useLLVMCoverageMapFormat= */ false,
/* codeCoverageEnabled= */ false,
/* isHostConfiguration= */ false);
@@ -120,6 +121,12 @@
private final boolean isHostConfiguration;
private final boolean forcePic;
private final boolean shouldStripBinaries;
+ /**
+ * WARNING: We don't like {@link FdoProvider}. Its {@link FdoProvider#fdoProfilePath} is pure
+ * path and that is horrible as it breaks many Bazel assumptions! Don't do bad stuff with it,
+ * don't take inspiration from it.
+ */
+ private final FdoProvider fdoProvider;
public CcToolchainProvider(
ImmutableMap<String, Object> values,
@@ -153,6 +160,7 @@
ImmutableList<PathFragment> builtInIncludeDirectories,
@Nullable PathFragment sysroot,
FdoMode fdoMode,
+ FdoProvider fdoProvider,
boolean useLLVMCoverageMapFormat,
boolean codeCoverageEnabled,
boolean isHostConfiguration) {
@@ -188,6 +196,7 @@
this.builtInIncludeDirectories = builtInIncludeDirectories;
this.sysroot = sysroot;
this.fdoMode = fdoMode;
+ this.fdoProvider = fdoProvider;
this.useLLVMCoverageMapFormat = useLLVMCoverageMapFormat;
this.codeCoverageEnabled = codeCoverageEnabled;
this.isHostConfiguration = isHostConfiguration;
@@ -693,6 +702,10 @@
return toolchainInfo.getAdditionalMakeVariables();
}
+ public FdoProvider getFdoProvider() {
+ return fdoProvider;
+ }
+
/**
* Returns whether the toolchain supports "Fission" C++ builds, i.e. builds where compilation
* partitions object code and debug symbols into separate output files.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
index e8ccf33..477a9ef 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
@@ -247,25 +247,6 @@
}
}
- /**
- * Return {@link FdoProvider} using default cc_toolchain attribute name.
- *
- * <p>Be careful to provide explicit attribute name if the rule doesn't store cc_toolchain under
- * the default name.
- */
- @Nullable
- public static FdoProvider getFdoProviderUsingDefaultCcToolchainAttribute(
- RuleContext ruleContext) {
- return getFdoProvider(ruleContext, CcToolchain.CC_TOOLCHAIN_DEFAULT_ATTRIBUTE_NAME);
- }
-
- @Nullable public static FdoProvider getFdoProvider(RuleContext ruleContext,
- String ccToolchainAttribute) {
- return ruleContext
- .getPrerequisite(ccToolchainAttribute, Mode.TARGET)
- .getProvider(FdoProvider.class);
- }
-
public static NestedSet<Pair<String, String>> getCoverageEnvironmentIfNeeded(
RuleContext ruleContext, CcToolchainProvider toolchain) {
if (ruleContext.getConfiguration().isCodeCoverageEnabled()) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java
index 95438a4..a4871d5 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java
@@ -237,13 +237,14 @@
private CcCompilationHelper initializeCompilationHelper(
FeatureConfiguration featureConfiguration) {
+ CcToolchainProvider toolchain = ccToolchain(ruleContext);
CcCompilationHelper helper =
new CcCompilationHelper(
ruleContext,
cppSemantics,
featureConfiguration,
- ccToolchain(ruleContext),
- CppHelper.getFdoProviderUsingDefaultCcToolchainAttribute(ruleContext));
+ toolchain,
+ toolchain.getFdoProvider());
TransitiveInfoCollection runtime = getProtoToolchainProvider().runtime();
if (runtime != null) {
helper.addDeps(ImmutableList.of(runtime));
@@ -257,13 +258,14 @@
}
private CcLinkingHelper initializeLinkingHelper(FeatureConfiguration featureConfiguration) {
+ CcToolchainProvider toolchain = ccToolchain(ruleContext);
CcLinkingHelper helper =
new CcLinkingHelper(
ruleContext,
cppSemantics,
featureConfiguration,
- ccToolchain(ruleContext),
- CppHelper.getFdoProviderUsingDefaultCcToolchainAttribute(ruleContext),
+ toolchain,
+ toolchain.getFdoProvider(),
ruleContext.getConfiguration())
.enableCcNativeLibrariesProvider();
TransitiveInfoCollection runtime = getProtoToolchainProvider().runtime();
@@ -273,7 +275,7 @@
helper.addDeps(ruleContext.getPrerequisites("deps", TARGET));
// TODO(dougk): Configure output artifact with action_config
// once proto compile action is configurable from the crosstool.
- if (!ccToolchain(ruleContext).supportsDynamicLinker()) {
+ if (!toolchain.supportsDynamicLinker()) {
helper.setShouldCreateDynamicLibrary(false);
}
return helper;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/nativedeps/NativeDepsHelper.java b/src/main/java/com/google/devtools/build/lib/rules/nativedeps/NativeDepsHelper.java
index 89e8c48..d480356 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/nativedeps/NativeDepsHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/nativedeps/NativeDepsHelper.java
@@ -216,8 +216,7 @@
} else {
sharedLibrary = nativeDeps;
}
- FdoProvider fdoProvider =
- CppHelper.getFdoProviderUsingDefaultCcToolchainAttribute(ruleContext);
+ FdoProvider fdoProvider = toolchain.getFdoProvider();
FeatureConfiguration featureConfiguration =
CcCommon.configureFeaturesOrReportRuleError(
ruleContext,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibrary.java
index c321398..8f0a4d5 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibrary.java
@@ -35,7 +35,6 @@
import com.google.devtools.build.lib.rules.apple.ApplePlatform.PlatformType;
import com.google.devtools.build.lib.rules.cpp.CcLinkingInfo;
import com.google.devtools.build.lib.rules.cpp.CcToolchainProvider;
-import com.google.devtools.build.lib.rules.cpp.CppHelper;
import com.google.devtools.build.lib.rules.objc.ObjcProvider.Key;
import com.google.devtools.build.lib.rules.proto.ProtoSourcesProvider;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
@@ -161,7 +160,7 @@
objcProvider,
intermediateArtifacts.strippedSingleArchitectureLibrary(),
childConfigurationsAndToolchains.get(childConfig),
- CppHelper.getFdoProviderUsingDefaultCcToolchainAttribute(ruleContext))
+ childToolchain.getFdoProvider())
.validateAttributes();
ruleContext.assertNoErrors();
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java
index 005b5dc..1fcd242 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java
@@ -960,7 +960,7 @@
ExtraCompileArgs.NONE,
ImmutableList.<PathFragment>of(),
toolchain,
- maybeGetFdoProvider());
+ toolchain.getFdoProvider());
}
/**
@@ -1083,7 +1083,7 @@
extraCompileArgs,
priorityHeaders,
toolchain,
- maybeGetFdoProvider());
+ toolchain.getFdoProvider());
}
return this;
}
@@ -1159,8 +1159,7 @@
.addVariableCategory(VariableCategory.EXECUTABLE_LINKING_VARIABLES);
Artifact binaryToLink = getBinaryToLink();
- FdoProvider fdoProvider =
- CppHelper.getFdoProviderUsingDefaultCcToolchainAttribute(ruleContext);
+ FdoProvider fdoProvider = toolchain.getFdoProvider();
CppLinkActionBuilder executableLinkAction =
new CppLinkActionBuilder(
ruleContext,
@@ -1294,7 +1293,8 @@
*/
public CompilationSupport registerFullyLinkAction(
ObjcProvider objcProvider, Artifact outputArchive) throws InterruptedException {
- return registerFullyLinkAction(objcProvider, outputArchive, toolchain, maybeGetFdoProvider());
+ return registerFullyLinkAction(
+ objcProvider, outputArchive, toolchain, toolchain.getFdoProvider());
}
/**
@@ -1862,18 +1862,6 @@
return objcHeaderThinningInfoByCommandLine;
}
- @Nullable
- private FdoProvider maybeGetFdoProvider() {
- // TODO(rduan): Remove this check once all rules are using the crosstool support.
- if (ruleContext
- .attributes()
- .has(CcToolchain.CC_TOOLCHAIN_DEFAULT_ATTRIBUTE_NAME, BuildType.LABEL)) {
- return CppHelper.getFdoProviderUsingDefaultCcToolchainAttribute(ruleContext);
- } else {
- return null;
- }
- }
-
public static Optional<Artifact> getCustomModuleMap(RuleContext ruleContext) {
if (ruleContext.attributes().has("module_map", BuildType.LABEL)) {
return Optional.fromNullable(ruleContext.getPrerequisiteArtifact("module_map", Mode.TARGET));
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java
index 919024a..073a407 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java
@@ -257,8 +257,7 @@
try {
CcToolchainProvider ccToolchain =
CppHelper.getToolchain(ruleContext, ":j2objc_cc_toolchain");
- FdoProvider fdoProvider =
- CppHelper.getFdoProvider(ruleContext, ":j2objc_cc_toolchain");
+ FdoProvider fdoProvider = ccToolchain.getFdoProvider();
CompilationSupport compilationSupport =
new CompilationSupport.Builder()
.setRuleContext(ruleContext)
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java
index 897d4b2..c7804b2 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java
@@ -77,6 +77,7 @@
/* builtInIncludeDirectories= */ ImmutableList.<PathFragment>of(),
/* sysroot= */ null,
FdoMode.OFF,
+ /* fdoProvider= */ null,
/* useLLVMCoverageMapFormat= */ false,
/* codeCoverageEnabled= */ false,
/* isHostConfiguration= */ false);
@@ -114,6 +115,7 @@
/* builtInIncludeDirectories= */ ImmutableList.<PathFragment>of(),
/* sysroot= */ null,
FdoMode.OFF,
+ /* fdoProvider= */ null,
/* useLLVMCoverageMapFormat= */ false,
/* codeCoverageEnabled= */ false,
/* isHostConfiguration= */ false);
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java
index af79589..e2d013c 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java
@@ -309,14 +309,16 @@
@Override
public Action generate(ImmutableSet<NonStaticAttributes> attributesToFlip)
throws InterruptedException {
+ CcToolchainProvider toolchain =
+ CppHelper.getToolchainUsingDefaultCcToolchainAttribute(ruleContext);
CppLinkActionBuilder builder =
new CppLinkActionBuilder(
ruleContext,
attributesToFlip.contains(NonStaticAttributes.OUTPUT_FILE)
? dynamicOutputFile
: staticOutputFile,
- CppHelper.getToolchainUsingDefaultCcToolchainAttribute(ruleContext),
- CppHelper.getFdoProviderUsingDefaultCcToolchainAttribute(ruleContext),
+ toolchain,
+ toolchain.getFdoProvider(),
featureConfiguration,
MockCppSemantics.INSTANCE) {};
if (attributesToFlip.contains(NonStaticAttributes.OUTPUT_FILE)) {
@@ -365,14 +367,16 @@
@Override
public Action generate(ImmutableSet<StaticKeyAttributes> attributes)
throws InterruptedException {
+ CcToolchainProvider toolchain =
+ CppHelper.getToolchainUsingDefaultCcToolchainAttribute(ruleContext);
CppLinkActionBuilder builder =
new CppLinkActionBuilder(
ruleContext,
attributes.contains(StaticKeyAttributes.OUTPUT_FILE)
? staticOutputFile
: dynamicOutputFile,
- CppHelper.getToolchainUsingDefaultCcToolchainAttribute(ruleContext),
- CppHelper.getFdoProviderUsingDefaultCcToolchainAttribute(ruleContext),
+ toolchain,
+ toolchain.getFdoProvider(),
featureConfiguration,
MockCppSemantics.INSTANCE) {};
builder.setLinkType(
@@ -397,12 +401,14 @@
PathFragment.create("output/path.ifso"), getTargetConfiguration().getBinDirectory(
RepositoryName.MAIN),
ActionsTestUtil.NULL_ARTIFACT_OWNER);
+ CcToolchainProvider toolchain =
+ CppHelper.getToolchainUsingDefaultCcToolchainAttribute(ruleContext);
CppLinkActionBuilder builder =
new CppLinkActionBuilder(
ruleContext,
output,
- CppHelper.getToolchainUsingDefaultCcToolchainAttribute(ruleContext),
- CppHelper.getFdoProviderUsingDefaultCcToolchainAttribute(ruleContext),
+ toolchain,
+ toolchain.getFdoProvider(),
FeatureConfiguration.EMPTY,
MockCppSemantics.INSTANCE);
builder.setLinkType(LinkTargetType.STATIC_LIBRARY);
@@ -487,6 +493,8 @@
FeatureConfiguration featureConfiguration)
throws Exception {
RuleContext ruleContext = createDummyRuleContext();
+ CcToolchainProvider toolchain =
+ CppHelper.getToolchainUsingDefaultCcToolchainAttribute(ruleContext);
CppLinkActionBuilder builder =
new CppLinkActionBuilder(
ruleContext,
@@ -495,8 +503,8 @@
getTargetConfiguration()
.getBinDirectory(ruleContext.getRule().getRepository())),
ruleContext.getConfiguration(),
- CppHelper.getToolchainUsingDefaultCcToolchainAttribute(ruleContext),
- CppHelper.getFdoProviderUsingDefaultCcToolchainAttribute(ruleContext),
+ toolchain,
+ toolchain.getFdoProvider(),
featureConfiguration,
MockCppSemantics.INSTANCE)
.addObjectFiles(nonLibraryInputs)
