Split user_link_flags from legacy_link_flags

The difference between them is that user_link_flags will stay after we remove
legacy fields from the crosstool.

This is an encore of https://github.com/bazelbuild/bazel/commit/2661abb96b1fe51fb726a63eb08698564a82eb20 after mitigating the memory regression.

Original cl regressed 12mb (on a big enough internal project :):

 objsize  chg   instances       space KB    class name
------------------------------------------------------
      24   +0      190265        4459 KB    com.google.common.collect.ImmutableMapEntry
      40   +0       95154        3716 KB    com.google.common.collect.RegularImmutableMap
      47   -2       95154        2230 KB    [Ljava.util.Map$Entry;
      44   -1       95154        2230 KB    [Lcom.google.common.collect.ImmutableMapEntry;
      24   +0       95149        2230 KB    com.google.devtools.build.lib.rules.cpp.CcToolchainVariables$MapVariables
      16   +0       95149        1486 KB    com.google.devtools.build.lib.rules.cpp.CcToolchainVariables$StringSequence
      89   +0        3188         176 KB    [B
      64   -8           0        -743 KB    com.google.devtools.build.lib.rules.cpp.LinkCommandLine
      76   +0       -1918       -1323 KB    [Ljava.lang.Object;
      24   +0      -95149       -2230 KB    com.google.devtools.build.lib.rules.cpp.CcToolchainVariables$SingleVariables
------------------------------------------------------

The reason was that before legacy_link_flags were the only build variable
patched because of thinlto in CppLinkActionBuilder, so it used optimized
SingleVariables subclass.  With the split, the patched variables instance had 2
variables therefore it received the full blown Variables instance. That
accounted for the ~7mb.

The fix was to move the patching logic to the initial link variables creation
and not to create patched variables at all. Now the regression is ~4.8mb, which
is perfectly expected since I introduce another variable and this is the
overhead of additional hashmap entry:

 objsize  chg   instances       space KB    class name
------------------------------------------------------
      24   +0      190418        4462 KB    com.google.common.collect.ImmutableMapEntry
      44   +1          31        1487 KB    [Lcom.google.common.collect.ImmutableMapEntry;
      16   +0       95149        1486 KB    com.google.devtools.build.lib.rules.cpp.CcToolchainVariables$StringSequence
      47   +0          31         744 KB    [Ljava.util.Map$Entry;
      89   +0        5723         315 KB    [B
      24   +0        5721         134 KB    java.lang.String
      64   -8           0        -743 KB    com.google.devtools.build.lib.rules.cpp.LinkCommandLine
      76   +0       -2387        -840 KB    [Ljava.lang.Object;
      24   +0      -95149       -2230 KB    com.google.devtools.build.lib.rules.cpp.CcToolchainVariables$SingleVariables
------------------------------------------------------

I'll pay this dept back once we get rid of legacy crosstool fields (= removal of
legacy_link_flags variable).

RELNOTES: None.
PiperOrigin-RevId: 197574153

1 files changed, 7 insertions, 18 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java
index a1c037f96e..92358daf14 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java
@@ -50,7 +50,6 @@ public final class LinkCommandLine extends CommandLine {
   private final Iterable<Artifact> linkerInputArtifacts;
   private final LinkTargetType linkTargetType;
   private final Link.LinkingMode linkingMode;
-  private final ImmutableList<String> linkopts;
   @Nullable private final PathFragment toolchainLibrariesSolibDir;
   private final boolean nativeDeps;
   private final boolean useTestOnlyFlags;
@@ -65,7 +64,6 @@ public final class LinkCommandLine extends CommandLine {
       Iterable<Artifact> linkerInputArtifacts,
       LinkTargetType linkTargetType,
       Link.LinkingMode linkingMode,
-      ImmutableList<String> linkopts,
       @Nullable PathFragment toolchainLibrariesSolibDir,
       boolean nativeDeps,
       boolean useTestOnlyFlags,
@@ -81,7 +79,6 @@ public final class LinkCommandLine extends CommandLine {
     this.linkerInputArtifacts = Preconditions.checkNotNull(linkerInputArtifacts);
     this.linkTargetType = Preconditions.checkNotNull(linkTargetType);
     this.linkingMode = Preconditions.checkNotNull(linkingMode);
-    this.linkopts = linkopts;
     this.toolchainLibrariesSolibDir = toolchainLibrariesSolibDir;
     this.nativeDeps = nativeDeps;
     this.useTestOnlyFlags = useTestOnlyFlags;
@@ -119,7 +116,12 @@ public final class LinkCommandLine extends CommandLine {
    * Returns the additional linker options for this link.
    */
   public ImmutableList<String> getLinkopts() {
-    return linkopts;
+    if (variables.isAvailable(LinkBuildVariables.USER_LINK_FLAGS.getVariableName())) {
+      return CcToolchainVariables.toStringList(
+          variables, LinkBuildVariables.USER_LINK_FLAGS.getVariableName());
+    } else {
+      return ImmutableList.of();
+    }
   }
 
   /** Returns the path to the linker. */
@@ -413,7 +415,6 @@ public final class LinkCommandLine extends CommandLine {
     private Iterable<Artifact> linkerInputArtifacts = ImmutableList.of();
     @Nullable private LinkTargetType linkTargetType;
     private Link.LinkingMode linkingMode = Link.LinkingMode.LEGACY_FULLY_STATIC;
-    private ImmutableList<String> linkopts = ImmutableList.of();
     @Nullable private PathFragment toolchainLibrariesSolibDir;
     private boolean nativeDeps;
     private boolean useTestOnlyFlags;
@@ -437,7 +438,7 @@ public final class LinkCommandLine extends CommandLine {
       if (ruleContext != null) {
         Preconditions.checkNotNull(featureConfiguration);
       }
-      
+
       if (variables == null) {
         variables = CcToolchainVariables.EMPTY;
       }
@@ -451,7 +452,6 @@ public final class LinkCommandLine extends CommandLine {
           linkerInputArtifacts,
           linkTargetType,
           linkingMode,
-          linkopts,
           toolchainLibrariesSolibDir,
           nativeDeps,
           useTestOnlyFlags,
@@ -495,17 +495,6 @@ public final class LinkCommandLine extends CommandLine {
     }
 
     /**
-     * Sets the linker options. These are passed to the linker in addition to the other linker
-     * options like linker inputs, symbol count options, etc. The {@link #build} method throws an
-     * exception if the linker options are non-empty for a static link (see {@link
-     * LinkTargetType#linkerOrArchiver()}).
-     */
-    public Builder setLinkopts(ImmutableList<String> linkopts) {
-      this.linkopts = linkopts;
-      return this;
-    }
-
-    /**
      * Sets how static the link is supposed to be. For static target types (see {@link
      * LinkTargetType#linkerOrArchiver()}}), the {@link #build} method throws an exception if this
      * is not {@link Link.LinkingMode#LEGACY_FULLY_STATIC}. The default setting is {@link