diff options
author | Ulf Adams <ulfjack@google.com> | 2015-08-03 12:47:41 +0000 |
---|---|---|
committer | Damien Martin-Guillerez <dmarting@google.com> | 2015-08-04 09:08:11 +0000 |
commit | 48685bf34fdde1f68d9decb5060802e7b559db63 (patch) | |
tree | 394345546bf51adbff6be470ca14bd57d0bab347 /src | |
parent | 006bf4fbf65d2bd96dc7d3a26fce5ea4029c5288 (diff) |
Refactor CcBinary / CcCommon a bit.
Don't compute the linkopts in CcCommon unconditionally, only on demand. In
order to only do this once, I had to pull out all calls to getLinkopts and
move them up in the call hierarchy. This in turn resulted in some
simplification and dead code removal in CcBinary.
--
MOS_MIGRATED_REVID=99716999
Diffstat (limited to 'src')
3 files changed, 39 insertions, 75 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 56f6e40b4a..a0d4f052db 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 @@ -160,6 +160,8 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { LinkTargetType linkType = isLinkShared(ruleContext) ? LinkTargetType.DYNAMIC_LIBRARY : LinkTargetType.EXECUTABLE; + List<String> linkopts = common.getLinkopts(); + LinkStaticness linkStaticness = getLinkStaticness(ruleContext, linkopts, cppConfiguration); CcLibraryHelper helper = new CcLibraryHelper(ruleContext, semantics, featureConfiguration) @@ -186,13 +188,13 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { Artifact binary = ruleContext.getPackageRelativeArtifact( binaryPath, ruleContext.getConfiguration().getBinDirectory()); CppLinkAction.Builder linkActionBuilder = determineLinkerArguments( - ruleContext, common, precompiledFiles, cppConfiguration, ccCompilationOutputs, - cppCompilationContext.getCompilationPrerequisites(), fake, binary); + ruleContext, common, precompiledFiles, ccCompilationOutputs, + cppCompilationContext.getCompilationPrerequisites(), fake, binary, linkStaticness, + linkopts); linkActionBuilder.setUseTestOnlyFlags(useTestOnlyFlags); linkActionBuilder.addNonLibraryInputs(ccCompilationOutputs.getHeaderTokenFiles()); CcToolchainProvider ccToolchain = CppHelper.getToolchain(ruleContext); - LinkStaticness linkStaticness = getLinkStaticness(ruleContext, common, cppConfiguration); if (linkStaticness == LinkStaticness.DYNAMIC) { linkActionBuilder.setRuntimeInputs( ccToolchain.getDynamicRuntimeLinkMiddleman(), @@ -252,7 +254,7 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { createStripAction(ruleContext, cppConfiguration, executable, strippedFile); DwoArtifactsCollector dwoArtifacts = - collectTransitiveDwoArtifacts(ruleContext, common, cppConfiguration, ccCompilationOutputs); + collectTransitiveDwoArtifacts(ruleContext, ccCompilationOutputs, linkStaticness); Artifact dwpFile = ruleContext.getImplicitOutputArtifact(CppRuleClasses.CC_BINARY_DEBUG_PACKAGE); createDebugPackagerActions(ruleContext, cppConfiguration, dwpFile, dwoArtifacts); @@ -344,10 +346,11 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { * the object files, libraries, linker options, linkstamps attributes and linker scripts. */ private static CppLinkAction.Builder determineLinkerArguments(RuleContext context, - CcCommon common, PrecompiledFiles precompiledFiles, CppConfiguration cppConfiguration, + CcCommon common, PrecompiledFiles precompiledFiles, CcCompilationOutputs compilationOutputs, ImmutableSet<Artifact> compilationPrerequisites, - boolean fake, Artifact binary) { + boolean fake, Artifact binary, + LinkStaticness linkStaticness, List<String> linkopts) { CppLinkAction.Builder builder = new CppLinkAction.Builder(context, binary) .setCrosstoolInputs(CppHelper.getToolchain(context).getLink()) .addNonLibraryInputs(compilationPrerequisites); @@ -376,38 +379,14 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { } } - // Then libraries from the closure of deps. - // This is true for both FULLY STATIC and MOSTLY STATIC linking. - boolean linkingStatically = - getLinkStaticness(context, common, cppConfiguration) != LinkStaticness.DYNAMIC; + // Then the link params from the closure of deps. CcLinkParams linkParams = collectCcLinkParams( - context, common, linkingStatically, isLinkShared(context)); + context, linkStaticness != LinkStaticness.DYNAMIC, isLinkShared(context), linkopts); builder.addLinkParams(linkParams, context); return builder; } /** - * Gets the linkopts to use for this binary. These options are NOT used when - * linking other binaries that depend on this binary. - * - * @return a new List instance that contains the linkopts for this binary - * target. - */ - private static ImmutableList<String> getBinaryLinkopts(RuleContext context, - CcCommon common) { - List<String> linkopts = new ArrayList<>(); - if (isLinkShared(context)) { - linkopts.add("-shared"); - } - linkopts.addAll(common.getLinkopts()); - return ImmutableList.copyOf(linkopts); - } - - private static boolean linkstaticAttribute(RuleContext context) { - return context.attributes().get("linkstatic", Type.BOOLEAN); - } - - /** * Returns "true" if the {@code linkshared} attribute exists and is set. */ private static final boolean isLinkShared(RuleContext context) { @@ -415,20 +394,20 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { && context.attributes().get("linkshared", Type.BOOLEAN); } - private static final boolean dashStaticInLinkopts(CcCommon common, + private static final boolean dashStaticInLinkopts(List<String> linkopts, CppConfiguration cppConfiguration) { - return common.getLinkopts().contains("-static") + return linkopts.contains("-static") || cppConfiguration.getLinkOptions().contains("-static"); } private static final LinkStaticness getLinkStaticness(RuleContext context, - CcCommon common, CppConfiguration cppConfiguration) { + List<String> linkopts, CppConfiguration cppConfiguration) { if (cppConfiguration.getDynamicMode() == DynamicMode.FULLY) { return LinkStaticness.DYNAMIC; - } else if (dashStaticInLinkopts(common, cppConfiguration)) { + } else if (dashStaticInLinkopts(linkopts, cppConfiguration)) { return LinkStaticness.FULLY_STATIC; } else if (cppConfiguration.getDynamicMode() == DynamicMode.OFF - || linkstaticAttribute(context)) { + || context.attributes().get("linkstatic", Type.BOOLEAN)) { return LinkStaticness.MOSTLY_STATIC; } else { return LinkStaticness.DYNAMIC; @@ -444,8 +423,8 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { * so we don't need them here. */ private static DwoArtifactsCollector collectTransitiveDwoArtifacts(RuleContext context, - CcCommon common, CppConfiguration cppConfiguration, CcCompilationOutputs compilationOutputs) { - if (getLinkStaticness(context, common, cppConfiguration) == LinkStaticness.DYNAMIC) { + CcCompilationOutputs compilationOutputs, LinkStaticness linkStaticness) { + if (linkStaticness == LinkStaticness.DYNAMIC) { return DwoArtifactsCollector.directCollector(compilationOutputs); } else { return CcCommon.collectTransitiveDwoArtifacts(context, compilationOutputs); @@ -584,19 +563,19 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { /** * Collect link parameters from the transitive closure. */ - private static CcLinkParams collectCcLinkParams(RuleContext context, CcCommon common, - boolean linkingStatically, boolean linkShared) { + private static CcLinkParams collectCcLinkParams(RuleContext context, + boolean linkingStatically, boolean linkShared, List<String> linkopts) { CcLinkParams.Builder builder = CcLinkParams.builder(linkingStatically, linkShared); if (isLinkShared(context)) { // CcLinkingOutputs is empty because this target is not configured yet - builder.addCcLibrary(context, common, false, CcLinkingOutputs.EMPTY); + builder.addCcLibrary(context, false, linkopts, CcLinkingOutputs.EMPTY); } else { builder.addTransitiveTargets( context.getPrerequisites("deps", Mode.TARGET), CcLinkParamsProvider.TO_LINK_PARAMS, CcSpecificLinkParamsProvider.TO_LINK_PARAMS); builder.addTransitiveTarget(CppHelper.mallocForTarget(context)); - builder.addLinkOpts(getBinaryLinkopts(context, common)); + builder.addLinkOpts(linkopts); } return builder.build(); } 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 3e6b11c930..770b948088 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 @@ -100,11 +100,6 @@ public final class CcCommon { private final ImmutableList<Pair<Artifact, Label>> cAndCppSources; - /** - * The expanded linkopts for this rule. - */ - private final ImmutableList<String> linkopts; - private final RuleContext ruleContext; public CcCommon(RuleContext ruleContext, FeatureConfiguration featureConfiguration) { @@ -112,7 +107,6 @@ public final class CcCommon { this.cppConfiguration = ruleContext.getFragment(CppConfiguration.class); this.featureConfiguration = featureConfiguration; this.cAndCppSources = collectCAndCppSources(); - linkopts = initLinkopts(); } /** @@ -120,7 +114,20 @@ public final class CcCommon { * options to use when building this target and anything that depends on it. */ public ImmutableList<String> getLinkopts() { - return linkopts; + Preconditions.checkState(hasAttribute("linkopts", Type.STRING_LIST)); + List<String> ourLinkopts = ruleContext.attributes().get("linkopts", Type.STRING_LIST); + List<String> result = new ArrayList<>(); + if (ourLinkopts != null) { + boolean allowDashStatic = !cppConfiguration.forceIgnoreDashStatic() + && (cppConfiguration.getDynamicMode() != DynamicMode.FULLY); + for (String linkopt : ourLinkopts) { + if (linkopt.equals("-static") && !allowDashStatic) { + continue; + } + CppHelper.expandAttribute(ruleContext, result, "linkopts", linkopt, true); + } + } + return ImmutableList.copyOf(result); } public ImmutableList<String> getCopts() { @@ -367,29 +374,6 @@ public final class CcCommon { } /** - * Collects our own linkopts from the rule attribute. This determines linker - * options to use when building this library and anything that depends on it. - */ - private final ImmutableList<String> initLinkopts() { - if (!hasAttribute("linkopts", Type.STRING_LIST)) { - return ImmutableList.<String>of(); - } - List<String> ourLinkopts = ruleContext.attributes().get("linkopts", Type.STRING_LIST); - List<String> result = new ArrayList<>(); - if (ourLinkopts != null) { - boolean allowDashStatic = !cppConfiguration.forceIgnoreDashStatic() - && (cppConfiguration.getDynamicMode() != DynamicMode.FULLY); - for (String linkopt : ourLinkopts) { - if (linkopt.equals("-static") && !allowDashStatic) { - continue; - } - CppHelper.expandAttribute(ruleContext, result, "linkopts", linkopt, true); - } - } - return ImmutableList.copyOf(result); - } - - /** * Determines a list of loose include directories that are only allowed to be referenced when * headers checking is {@link HeadersCheckingMode#LOOSE} or {@link HeadersCheckingMode#WARN}. */ diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkParams.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkParams.java index a256332cc3..84d6ec85b2 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkParams.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkParams.java @@ -28,6 +28,7 @@ import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.rules.cpp.LinkerInputs.LibraryToLink; import java.util.Collection; +import java.util.List; import java.util.Objects; /** @@ -281,7 +282,7 @@ public final class CcLinkParams { * getPreferredLibraries() and getLinkOpts() into the current link params * object. */ - public Builder addCcLibrary(RuleContext context, CcCommon common, boolean neverlink, + public Builder addCcLibrary(RuleContext context, boolean neverlink, List<String> linkopts, CcLinkingOutputs linkingOutputs) { addTransitiveTargets( context.getPrerequisites("deps", Mode.TARGET), @@ -290,7 +291,7 @@ public final class CcLinkParams { if (!neverlink) { addLibraries(linkingOutputs.getPreferredLibraries(linkingStatically, linkShared || context.getFragment(CppConfiguration.class).forcePic())); - addLinkOpts(common.getLinkopts()); + addLinkOpts(linkopts); } return this; } |