Skip to content

Commit

Permalink
Rewrite CcLinkingHelper.convertLibraryToLinkListToLinkerInputList to …
Browse files Browse the repository at this point in the history
…Starlark

Return None from LibraryToLink (pic_)object_files. That semantics is needed for the reset of the linking implementation.

Expose LegacyLinkerInput fields to internal Starlark implementation. Generally users shouldn't be able to get their hands on those structures.

Expose constructors of LegacyLinkerInput to cc_internal, for C++ internal consumption only.

Expose missing fields on LibraryToLink to Starlark, but only as a guarded APIs. Some of those fields weren't exposed before and we need
to evaluate if they are truly needed, before exposing them to all the users.

PiperOrigin-RevId: 634393984
Change-Id: Id31597a3b87f48566d5734d61fbb2d9ba5050e7e
  • Loading branch information
comius authored and copybara-github committed May 16, 2024
1 parent 3fddc7f commit f9741e3
Show file tree
Hide file tree
Showing 7 changed files with 380 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -730,13 +730,15 @@ private boolean createDynamicLinkAction(
|| dynamicLinkType != LinkTargetType.NODEPS_DYNAMIC_LIBRARY;

if (shouldLinkTransitively) {
// LINT.IfChange
CcLinkingContext ccLinkingContext = CcLinkingContext.merge(ccLinkingContexts);
ImmutableList<LibraryInput> libraries =
convertLibraryToLinkListToLinkerInputList(
ccLinkingContext.getLibraries(),
linkingMode != LinkingMode.DYNAMIC,
dynamicLinkType.isDynamicLibrary(),
featureConfiguration);
// LINT.ThenChange(//src/main/starlark/builtins_bzl/common/cc/link/convert_linker_inputs.bzl)
ImmutableList<CcLinkingContext.Linkstamp> linkstamps =
ccLinkingContext.getLinkstamps().toList();
if (dynamicLinkType == LinkTargetType.NODEPS_DYNAMIC_LIBRARY) {
Expand Down Expand Up @@ -910,6 +912,7 @@ private Artifact getLinkedArtifact(LinkTargetType linkTargetType) throws RuleErr
artifactFragment);
}

// LINT.IfChange
private static ImmutableList<LibraryInput> convertLibraryToLinkListToLinkerInputList(
NestedSet<LibraryToLink> librariesToLink,
boolean staticMode,
Expand Down Expand Up @@ -974,6 +977,8 @@ private static ImmutableList<LibraryInput> convertLibraryToLinkListToLinkerInput
return libraryInputsBuilder.build();
}

// LINT.ThenChange(//src/main/starlark/builtins_bzl/common/cc/link/convert_linker_inputs.bzl)

@Nullable
private Artifact getDynamicLibrarySolibSymlinkOutput(Artifact linkerOutputArtifact)
throws EvalException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import static com.google.devtools.build.lib.skyframe.BzlLoadValue.keyForBuild;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.docgen.annot.DocCategory;
import com.google.devtools.build.lib.actions.Actions;
import com.google.devtools.build.lib.actions.Artifact;
Expand Down Expand Up @@ -58,6 +59,7 @@
import net.starlark.java.eval.Dict;
import net.starlark.java.eval.EvalException;
import net.starlark.java.eval.Sequence;
import net.starlark.java.eval.Starlark;
import net.starlark.java.eval.StarlarkValue;
import net.starlark.java.syntax.Location;

Expand Down Expand Up @@ -485,4 +487,58 @@ public void absoluteSymlink(
progressMessage);
ctx.getRuleContext().registerAction(action);
}

@StarlarkMethod(
name = "library_linker_input",
documented = false,
parameters = {
@Param(name = "input", named = true),
@Param(name = "artifact_category", named = true),
@Param(name = "library_identifier", named = true),
@Param(name = "object_files", named = true),
@Param(name = "lto_compilation_context", named = true),
@Param(name = "shared_non_lto_backends", defaultValue = "None", named = true),
@Param(name = "must_keep_debug", defaultValue = "False", named = true),
@Param(name = "disable_whole_archive", defaultValue = "False", named = true),
})
public LegacyLinkerInput libraryLinkerInput(
Artifact input,
String artifactCategory,
String libraryIdentifier,
Object objectFiles,
Object ltoCompilationContext,
Object sharedNonLtoBackends,
boolean mustKeepDebug,
boolean disableWholeArchive)
throws EvalException {
return LegacyLinkerInputs.newInputLibrary(
input,
ArtifactCategory.valueOf(artifactCategory),
libraryIdentifier,
objectFiles == Starlark.NONE
? null
: Sequence.cast(objectFiles, Artifact.class, "object_files").getImmutableList(),
ltoCompilationContext instanceof LtoCompilationContext lto ? lto : null,
/* sharedNonLtoBackends= */ ImmutableMap.copyOf(
Dict.noneableCast(
sharedNonLtoBackends,
Artifact.class,
LtoBackendArtifacts.class,
"shared_non_lto_backends")),
mustKeepDebug,
disableWholeArchive);
}

@StarlarkMethod(
name = "solib_linker_input",
documented = false,
parameters = {
@Param(name = "solib_symlink", named = true),
@Param(name = "original", named = true),
@Param(name = "library_identifier", named = true),
})
public LegacyLinkerInput solibLinkerInput(
Artifact solibSymlink, Artifact original, String libraryIdentifier) throws EvalException {
return LegacyLinkerInputs.solibLibraryInput(solibSymlink, original, libraryIdentifier);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,31 +16,48 @@

import com.google.common.collect.ImmutableCollection;
import com.google.devtools.build.lib.actions.Artifact;
import net.starlark.java.annot.StarlarkBuiltin;
import net.starlark.java.annot.StarlarkMethod;
import net.starlark.java.eval.StarlarkValue;

/**
* Something that appears on the command line of the linker. Since we sometimes expand archive files
* to their constituent object files, we need to keep information whether a certain file contains
* embedded objects and if so, the list of the object files themselves.
*
* <p>This is exposed to Starlark via StalarkValue only for the internal C++ code. It should never
* find its way to public interfaces.
*
* @deprecated Will be removed with the Starlarkification of C++ code.
*/
public interface LegacyLinkerInput {
@Deprecated
@StarlarkBuiltin(name = "LegacyLinkerInput", documented = false)
public interface LegacyLinkerInput extends StarlarkValue {

/** Returns the type of the linker input. */
ArtifactCategory getArtifactCategory();

@StarlarkMethod(name = "artifact_category", structField = true, documented = false)
default String getArtifactCategoryForStarlark() {
return getArtifactCategory().toString();
}

/** Returns the artifact that is the input of the linker. */
Artifact getArtifact();
@StarlarkMethod(name = "file", structField = true, documented = false)
public Artifact getArtifact();

/**
* Returns the original library to link. If this library is a solib symlink, returns the artifact
* the symlink points to, otherwise, the library itself.
*/
Artifact getOriginalLibraryArtifact();
@StarlarkMethod(name = "original_file", structField = true, documented = false)
public Artifact getOriginalLibraryArtifact();

/**
* Whether the input artifact contains object files or is opaque.
*/
/** Whether the input artifact contains object files or is opaque. */
@StarlarkMethod(name = "object_file_container", structField = true, documented = false)
boolean containsObjectFiles();

@StarlarkMethod(name = "is_linkstamp", structField = true, documented = false)
default boolean isLinkstamp() {
return false;
}
Expand All @@ -49,20 +66,27 @@ default boolean isLinkstamp() {
* Return the list of object files included in the input artifact, if there are any. It is legal
* to call this only when {@link #containsObjectFiles()} returns true.
*/
@StarlarkMethod(name = "object_files", structField = true, documented = false)
ImmutableCollection<Artifact> getObjectFiles();

/**
* Returns whether we must keep debug symbols for this input.
*/
/** Returns whether we must keep debug symbols for this input. */
@StarlarkMethod(name = "must_keep_debug", structField = true, documented = false)
boolean isMustKeepDebug();

/** If true, Bazel will not wrap this input in whole-archive block. */
@StarlarkMethod(name = "disable_whole_archive", structField = true, documented = false)
boolean disableWholeArchive();

/**
* Return the identifier for the library. This is used for de-duplication of linker inputs: two
* libraries should have the same identifier iff they are in fact the same library but linked in a
* different way (e.g. static/dynamic, PIC/no-PIC)
*/
@StarlarkMethod(name = "library_identifier", structField = true, documented = false)
String getLibraryIdentifier();

@Override
default boolean isImmutable() {
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,11 @@
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.concurrent.ThreadSafety;
import javax.annotation.Nullable;
import net.starlark.java.annot.StarlarkMethod;

/** Factory for creating new {@link LegacyLinkerInput} objects. */
@Deprecated
public abstract class LegacyLinkerInputs {
/**
* An opaque linker input that is not a library, for example a linker script or an individual
Expand Down Expand Up @@ -152,6 +155,7 @@ public boolean isLinkstamp() {
* has a library identifier.
*/
public interface LibraryInput extends LegacyLinkerInput {
@StarlarkMethod(name = "lto_compilation_context", structField = true, documented = false)
LtoCompilationContext getLtoCompilationContext();

/**
Expand All @@ -161,6 +165,7 @@ public interface LibraryInput extends LegacyLinkerInput {
* static linking. ThinLTO is otherwise too expensive when statically linking tests, due to the
* number of LTO backends that can be generated for a single blaze test invocation.
*/
@StarlarkMethod(name = "shared_non_lto_backends", structField = true, documented = false)
ImmutableMap<Artifact, LtoBackendArtifacts> getSharedNonLtoBackends();
}

Expand Down Expand Up @@ -266,7 +271,7 @@ private static class CompoundLibraryInput implements LibraryInput {
private final Artifact libraryArtifact;
private final ArtifactCategory category;
private final String libraryIdentifier;
private final ImmutableCollection<Artifact> objectFiles;
@Nullable private final ImmutableCollection<Artifact> objectFiles;
private final LtoCompilationContext ltoCompilationContext;
private final ImmutableMap<Artifact, LtoBackendArtifacts> sharedNonLtoBackends;
private final boolean mustKeepDebug;
Expand All @@ -276,7 +281,7 @@ private static class CompoundLibraryInput implements LibraryInput {
Artifact libraryArtifact,
ArtifactCategory category,
String libraryIdentifier,
ImmutableCollection<Artifact> objectFiles,
@Nullable ImmutableCollection<Artifact> objectFiles,
LtoCompilationContext ltoCompilationContext,
ImmutableMap<Artifact, LtoBackendArtifacts> sharedNonLtoBackends,
boolean allowArchiveTypeInAlwayslink,
Expand Down Expand Up @@ -481,7 +486,7 @@ static LibraryInput newInputLibrary(
Artifact library,
ArtifactCategory category,
String libraryIdentifier,
ImmutableCollection<Artifact> objectFiles,
@Nullable ImmutableCollection<Artifact> objectFiles,
LtoCompilationContext ltoCompilationContext,
ImmutableMap<Artifact, LtoBackendArtifacts> sharedNonLtoBackends,
boolean mustKeepDebug,
Expand Down
Loading

0 comments on commit f9741e3

Please sign in to comment.