Simplify copts handling and expose them as a build variable

This cl changes copts to be immutable (and changes addCopts methods into
setCopts, so it's simpler to reason about copts) and exposes copts as a build
variable. It also introduces CompileBuildVariablesTest, similar to
LinkBuildVariablesTest, to test that right build variables are exposed for right
actions.

RELNOTES: None.
PiperOrigin-RevId: 163876774

9 files changed, 185 insertions, 153 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java
index e940d57823..17e33e8de8 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java
@@ -263,7 +263,7 @@ public final class CcLibraryHelper {
   private final List<Artifact> objectFiles = new ArrayList<>();
   private final List<Artifact> picObjectFiles = new ArrayList<>();
   private final List<Artifact> nonCodeLinkerInputs = new ArrayList<>();
-  private final List<String> copts = new ArrayList<>();
+  private ImmutableList<String> copts = ImmutableList.of();
   private final List<String> linkopts = new ArrayList<>();
   @Nullable private Pattern nocopts;
   private final Set<String> defines = new LinkedHashSet<>();
@@ -371,15 +371,14 @@ public final class CcLibraryHelper {
 
   /** Sets fields that overlap for cc_library and cc_binary rules. */
   public CcLibraryHelper fromCommon(CcCommon common) {
-    this
-        .addCopts(common.getCopts())
-        .addDefines(common.getDefines())
-        .addDeps(ruleContext.getPrerequisites("deps", Mode.TARGET))
-        .addLooseIncludeDirs(common.getLooseIncludeDirs())
-        .addNonCodeLinkerInputs(common.getLinkerScripts())
-        .addSystemIncludeDirs(common.getSystemIncludeDirs())
-        .setNoCopts(common.getNoCopts())
-        .setHeadersCheckingMode(semantics.determineHeadersCheckingMode(ruleContext));
+    setCopts(common.getCopts());
+    addDefines(common.getDefines());
+    addDeps(ruleContext.getPrerequisites("deps", Mode.TARGET));
+    addLooseIncludeDirs(common.getLooseIncludeDirs());
+    addNonCodeLinkerInputs(common.getLinkerScripts());
+    addSystemIncludeDirs(common.getSystemIncludeDirs());
+    setNoCopts(common.getNoCopts());
+    setHeadersCheckingMode(semantics.determineHeadersCheckingMode(ruleContext));
     return this;
   }
 
@@ -635,11 +634,8 @@ public final class CcLibraryHelper {
     return this;
   }
 
-  /**
-   * Adds the copts to the compile command line.
-   */
-  public CcLibraryHelper addCopts(Iterable<String> copts) {
-    Iterables.addAll(this.copts, copts);
+  public CcLibraryHelper setCopts(ImmutableList<String> copts) {
+    this.copts = Preconditions.checkNotNull(copts);
     return this;
   }
 
@@ -1152,9 +1148,14 @@ public final class CcLibraryHelper {
    * Creates the C/C++ compilation action creator.
    */
   private CppModel initializeCppModel() {
-    return new CppModel(ruleContext, semantics, ccToolchain, fdoSupport, configuration)
+    return new CppModel(
+            ruleContext,
+            semantics,
+            ccToolchain,
+            fdoSupport,
+            configuration,
+            copts)
         .addCompilationUnitSources(compilationUnitSources)
-        .addCopts(copts)
         .setLinkTargetType(linkType)
         .setNeverLink(neverlink)
         .addLinkActionInputs(linkActionInputs)
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java
index 2cb61198bf..b06e89d71c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java
@@ -30,6 +30,7 @@ import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Sets;
 import com.google.common.collect.Sets.SetView;
+import com.google.common.collect.Streams;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
@@ -825,6 +826,20 @@ public class CcToolchainFeatures implements Serializable {
     public static final Variables EMPTY = new Variables.Builder().build();
 
     /**
+     * Retrieves a {@link StringSequence} variable named {@code variableName} from {@code variables}
+     * and converts it into a list of plain strings.
+     *
+     * <p>Throws {@link ExpansionException} when the variable is not a {@link StringSequence}.
+     */
+    public static final ImmutableList<String> toStringList(
+        CcToolchainFeatures.Variables variables, String variableName) {
+      return Streams
+          .stream(variables.getSequenceVariable(variableName))
+          .map(variable -> variable.getStringValue(variableName))
+          .collect(ImmutableList.toImmutableList());
+    }
+
+    /**
      * Variables can be either String values or an arbitrarily deeply nested recursive sequences,
      * which we represent as a tree of {@code VariableValue} nodes. The nodes are {@code Sequence}
      * objects, while the leafs are {@code StringSequence} objects. We do not allow {@code
@@ -1144,7 +1159,7 @@ public class CcToolchainFeatures implements Serializable {
      * objects significantly reduces memory overhead.
      */
     @Immutable
-    private static final class StringSequence implements VariableValue {
+    static final class StringSequence implements VariableValue {
 
       private final Iterable<String> values;
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLine.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLine.java
index 1d250c2dc3..3e1eaf69c4 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLine.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLine.java
@@ -36,7 +36,6 @@ public final class CompileCommandLine {
   private final Artifact sourceFile;
   private final Artifact outputFile;
   private final Label sourceLabel;
-  private final List<String> copts;
   private final Predicate<String> coptsFilter;
   private final Collection<String> features;
   private final FeatureConfiguration featureConfiguration;
@@ -50,7 +49,6 @@ public final class CompileCommandLine {
       Artifact sourceFile,
       Artifact outputFile,
       Label sourceLabel,
-      ImmutableList<String> copts,
       Predicate<String> coptsFilter,
       Collection<String> features,
       FeatureConfiguration featureConfiguration,
@@ -62,7 +60,6 @@ public final class CompileCommandLine {
     this.sourceFile = Preconditions.checkNotNull(sourceFile);
     this.outputFile = Preconditions.checkNotNull(outputFile);
     this.sourceLabel = Preconditions.checkNotNull(sourceLabel);
-    this.copts = Preconditions.checkNotNull(copts);
     this.coptsFilter = coptsFilter;
     this.features = Preconditions.checkNotNull(features);
     this.featureConfiguration = Preconditions.checkNotNull(featureConfiguration);
@@ -152,8 +149,6 @@ public final class CompileCommandLine {
     }
     addFilteredOptions(options, featureConfiguration.getCommandLine(actionName, updatedVariables));
 
-    addFilteredOptions(options, copts);
-
     // Unfiltered compiler options contain system include paths. These must be added after
     // the user provided options, otherwise users adding include paths will not pick up their
     // own include paths first.
@@ -204,19 +199,29 @@ public final class CompileCommandLine {
     return dotdFile;
   }
 
-  public List<String> getCopts() {
-    return copts;
-  }
-
   public Variables getVariables() {
     return variables;
   }
 
+  /**
+   * Returns all user provided copts flags.
+   *
+   * TODO(b/64108724): Get rid of this method when we don't need to parse copts to collect include
+   * directories anymore (meaning there is a way of specifying include directories using an
+   * explicit attribute, not using platform-dependent garbage bag that copts is).
+   */
+  public ImmutableList<String> getCopts() {
+    if (variables.isAvailable(CppModel.COPTS_VARIABLE_VALUE)) {
+      return Variables.toStringList(variables, CppModel.COPTS_VARIABLE_VALUE);
+    } else {
+      return ImmutableList.of();
+    }
+  }
+
   public static Builder builder(
       Artifact sourceFile,
       Artifact outputFile,
       Label sourceLabel,
-      ImmutableList<String> copts,
       Predicate<String> coptsFilter,
       ImmutableList<String> features,
       String actionName,
@@ -227,7 +232,6 @@ public final class CompileCommandLine {
         sourceFile,
         outputFile,
         sourceLabel,
-        copts,
         coptsFilter,
         features,
         actionName,
@@ -241,7 +245,6 @@ public final class CompileCommandLine {
     private final Artifact sourceFile;
     private final Artifact outputFile;
     private final Label sourceLabel;
-    private final ImmutableList<String> copts;
     private final Predicate<String> coptsFilter;
     private final Collection<String> features;
     private FeatureConfiguration featureConfiguration;
@@ -256,7 +259,6 @@ public final class CompileCommandLine {
           Preconditions.checkNotNull(sourceFile),
           Preconditions.checkNotNull(outputFile),
           Preconditions.checkNotNull(sourceLabel),
-          Preconditions.checkNotNull(copts),
           Preconditions.checkNotNull(coptsFilter),
           Preconditions.checkNotNull(features),
           Preconditions.checkNotNull(featureConfiguration),
@@ -271,7 +273,6 @@ public final class CompileCommandLine {
         Artifact sourceFile,
         Artifact outputFile,
         Label sourceLabel,
-        ImmutableList<String> copts,
         Predicate<String> coptsFilter,
         Collection<String> features,
         String actionName,
@@ -281,7 +282,6 @@ public final class CompileCommandLine {
       this.sourceFile = sourceFile;
       this.outputFile = outputFile;
       this.sourceLabel = sourceLabel;
-      this.copts = copts;
       this.coptsFilter = coptsFilter;
       this.features = features;
       this.actionName = actionName;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java
index d904494a35..2020b4b2ae 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java
@@ -64,6 +64,7 @@ public class CppActionConfigs {
                 "  tool {",
                 "    tool_path: '" + gccToolPath + "'",
                 "  }",
+                "  implies: 'copts'",
                 "}",
                 "action_config {",
                 "  config_name: 'preprocess-assemble'",
@@ -71,6 +72,7 @@ public class CppActionConfigs {
                 "  tool {",
                 "    tool_path: '" + gccToolPath + "'",
                 "  }",
+                "  implies: 'copts'",
                 "}",
                 "action_config {",
                 "  config_name: 'c-compile'",
@@ -78,6 +80,7 @@ public class CppActionConfigs {
                 "  tool {",
                 "    tool_path: '" + gccToolPath + "'",
                 "  }",
+                "  implies: 'copts'",
                 "}",
                 "action_config {",
                 "  config_name: 'c++-compile'",
@@ -85,6 +88,7 @@ public class CppActionConfigs {
                 "  tool {",
                 "    tool_path: '" + gccToolPath + "'",
                 "  }",
+                "  implies: 'copts'",
                 "}",
                 "action_config {",
                 "  config_name: 'c++-header-parsing'",
@@ -92,6 +96,7 @@ public class CppActionConfigs {
                 "  tool {",
                 "    tool_path: '" + gccToolPath + "'",
                 "  }",
+                "  implies: 'copts'",
                 "}",
                 "action_config {",
                 "  config_name: 'c++-header-preprocessing'",
@@ -99,6 +104,7 @@ public class CppActionConfigs {
                 "  tool {",
                 "    tool_path: '" + gccToolPath + "'",
                 "  }",
+                "  implies: 'copts'",
                 "}",
                 "action_config {",
                 "  config_name: 'c++-module-compile'",
@@ -106,6 +112,7 @@ public class CppActionConfigs {
                 "  tool {",
                 "    tool_path: '" + gccToolPath + "'",
                 "  }",
+                "  implies: 'copts'",
                 "}",
                 "action_config {",
                 "  config_name: 'c++-module-codegen'",
@@ -113,69 +120,70 @@ public class CppActionConfigs {
                 "  tool {",
                 "    tool_path: '" + gccToolPath + "'",
                 "  }",
+                "  implies: 'copts'",
+                "}",
+                // Gcc options:
+                //  -MD turns on .d file output as a side-effect (doesn't imply -E)
+                //  -MM[D] enables user includes only, not system includes
+                //  -MF <name> specifies the dotd file name
+                // Issues:
+                //  -M[M] alone subverts actual .o output (implies -E)
+                //  -M[M]D alone breaks some of the .d naming assumptions
+                // This combination gets user and system includes with specified name:
+                //  -MD -MF <name>
+                "feature {",
+                "  name: 'dependency_file'",
+                "  flag_set {",
+                "    action: 'assemble'",
+                "    action: 'preprocess-assemble'",
+                "    action: 'c-compile'",
+                "    action: 'c++-compile'",
+                "    action: 'c++-module-compile'",
+                "    action: 'objc-compile'",
+                "    action: 'objc++-compile'",
+                "    action: 'c++-header-preprocessing'",
+                "    action: 'c++-header-parsing'",
+                "    action: 'clif-match'",
+                "    expand_if_all_available: 'dependency_file'",
+                "    flag_group {",
+                "      flag: '-MD'",
+                "      flag: '-MF'",
+                "      flag: '%{dependency_file}'",
+                "    }",
+                "  }",
+                "}",
+                // GCC and Clang give randomized names to symbols which are defined in
+                // an anonymous namespace but have external linkage.  To make
+                // computation of these deterministic, we want to override the
+                // default seed for the random number generator.  It's safe to use
+                // any value which differs for all translation units; we use the
+                // path to the object file.
+                "feature {",
+                "  name: 'random_seed'",
+                "  flag_set {",
+                "    action: 'c++-compile'",
+                "    action: 'c++-module-codegen'",
+                "    action: 'c++-module-compile'",
+                "    flag_group {",
+                "      flag: '-frandom-seed=%{output_file}'",
+                "    }",
+                "  }",
+                "}",
+                "feature {",
+                "  name: 'pic'",
+                "  flag_set {",
+                "    action: 'assemble'",
+                "    action: 'preprocess-assemble'",
+                "    action: 'c-compile'",
+                "    action: 'c++-compile'",
+                "    action: 'c++-module-codegen'",
+                "    action: 'c++-module-compile'",
+                "    expand_if_all_available: 'pic'",
+                "    flag_group {",
+                "      flag: '-fPIC'",
+                "    }",
+                "  }",
                 "}",
-                  // Gcc options:
-                  //  -MD turns on .d file output as a side-effect (doesn't imply -E)
-                  //  -MM[D] enables user includes only, not system includes
-                  //  -MF <name> specifies the dotd file name
-                  // Issues:
-                  //  -M[M] alone subverts actual .o output (implies -E)
-                  //  -M[M]D alone breaks some of the .d naming assumptions
-                  // This combination gets user and system includes with specified name:
-                  //  -MD -MF <name>
-                  "feature {",
-                  "  name: 'dependency_file'",
-                  "  flag_set {",
-                  "    action: 'assemble'",
-                  "    action: 'preprocess-assemble'",
-                  "    action: 'c-compile'",
-                  "    action: 'c++-compile'",
-                  "    action: 'c++-module-compile'",
-                  "    action: 'objc-compile'",
-                  "    action: 'objc++-compile'",
-                  "    action: 'c++-header-preprocessing'",
-                  "    action: 'c++-header-parsing'",
-                  "    action: 'clif-match'",
-                  "    expand_if_all_available: 'dependency_file'",
-                  "    flag_group {",
-                  "      flag: '-MD'",
-                  "      flag: '-MF'",
-                  "      flag: '%{dependency_file}'",
-                  "    }",
-                  "  }",
-                  "}",
-                  // GCC and Clang give randomized names to symbols which are defined in
-                  // an anonymous namespace but have external linkage.  To make
-                  // computation of these deterministic, we want to override the
-                  // default seed for the random number generator.  It's safe to use
-                  // any value which differs for all translation units; we use the
-                  // path to the object file.
-                  "feature {",
-                  "  name: 'random_seed'",
-                  "  flag_set {",
-                  "    action: 'c++-compile'",
-                  "    action: 'c++-module-codegen'",
-                  "    action: 'c++-module-compile'",
-                  "    flag_group {",
-                  "      flag: '-frandom-seed=%{output_file}'",
-                  "    }",
-                  "  }",
-                  "}",
-                   "feature {",
-                   "  name: 'pic'",
-                   "  flag_set {",
-                   "    action: 'assemble'",
-                   "    action: 'preprocess-assemble'",
-                   "    action: 'c-compile'",
-                   "    action: 'c++-compile'",
-                   "    action: 'c++-module-codegen'",
-                   "    action: 'c++-module-compile'",
-                   "    expand_if_all_available: 'pic'",
-                   "    flag_group {",
-                   "      flag: '-fPIC'",
-                   "    }",
-                   "  }",
-                   "}",
                 ifTrue(
                     !features.contains(CppRuleClasses.PER_OBJECT_DEBUG_INFO),
                     "feature {",
@@ -192,22 +200,22 @@ public class CppActionConfigs {
                     "    }",
                     "  }",
                     "}"),
-                  "feature {",
-                  "  name: 'preprocessor_defines'",
-                  "  flag_set {",
-                  "    action: 'preprocess-assemble'",
-                  "    action: 'c-compile'",
-                  "    action: 'c++-compile'",
-                  "    action: 'c++-header-parsing'",
-                  "    action: 'c++-header-preprocessing'",
-                  "    action: 'c++-module-compile'",
-                  "    action: 'clif-match'",
-                  "    flag_group {",
-                  "      iterate_over: 'preprocessor_defines'",
-                  "      flag: '-D%{preprocessor_defines}'",
-                  "    }",
-                  "  }",
-                  "}",
+                "feature {",
+                "  name: 'preprocessor_defines'",
+                "  flag_set {",
+                "    action: 'preprocess-assemble'",
+                "    action: 'c-compile'",
+                "    action: 'c++-compile'",
+                "    action: 'c++-header-parsing'",
+                "    action: 'c++-header-preprocessing'",
+                "    action: 'c++-module-compile'",
+                "    action: 'clif-match'",
+                "    flag_group {",
+                "      iterate_over: 'preprocessor_defines'",
+                "      flag: '-D%{preprocessor_defines}'",
+                "    }",
+                "  }",
+                "}",
                 ifTrue(
                     !features.contains(CppRuleClasses.INCLUDE_PATHS),
                     "feature {",
@@ -516,10 +524,7 @@ public class CppActionConfigs {
                     "    action: 'c++-link-pic-static-library'",
                     "    action: 'c++-link-alwayslink-pic-static-library'",
                     "    flag_group {",
-                    ifLinux(
-                        platform,
-                        "  flag: 'rcsD'",
-                        "  flag: '%{output_execpath}'"),
+                    ifLinux(platform, "  flag: 'rcsD'", "  flag: '%{output_execpath}'"),
                     ifMac(
                         platform,
                         "  flag: '-static'",
@@ -819,7 +824,26 @@ public class CppActionConfigs {
                     "  requires {",
                     "    feature: 'coverage'",
                     "  }",
-                    "}")));
+                    "}"),
+                "feature {",
+                "  name: 'copts'",
+                "  flag_set {",
+                "    expand_if_all_available: 'copts'",
+                "    action: 'assemble'",
+                "    action: 'preprocess-assemble'",
+                "    action: 'c-compile'",
+                "    action: 'c++-compile'",
+                "    action: 'c++-header-parsing'",
+                "    action: 'c++-header-preprocessing'",
+                "    action: 'c++-module-compile'",
+                "    action: 'c++-module-codegen'",
+                "    action: 'lto-backend'",
+                "    flag_group {",
+                "      iterate_over: 'copts'",
+                "      flag: '%{copts}'",
+                "    }",
+                "  }",
+                "}"));
   }
 
   private static String ifLinux(CppPlatform platform, String... lines) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
index 3559fffde8..b4cc77960c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
@@ -267,7 +267,6 @@ public class CppCompileAction extends AbstractAction
    * @param cppConfiguration TODO(bazel-team): Add parameter description.
    * @param context the compilation context
    * @param actionContext TODO(bazel-team): Add parameter description.
-   * @param copts options for the compiler
    * @param coptsFilter regular expression to remove options from {@code copts}
    * @param specialInputsHandler TODO(bazel-team): Add parameter description.
    * @param lipoScannables List of artifacts to include-scan when this action is a lipo action
@@ -305,7 +304,6 @@ public class CppCompileAction extends AbstractAction
       CppConfiguration cppConfiguration,
       CppCompilationContext context,
       Class<? extends CppCompileActionContext> actionContext,
-      ImmutableList<String> copts,
       Predicate<String> coptsFilter,
       SpecialInputsHandler specialInputsHandler,
       Iterable<IncludeScannable> lipoScannables,
@@ -349,7 +347,6 @@ public class CppCompileAction extends AbstractAction
                 sourceFile,
                 outputFile,
                 sourceLabel,
-                copts,
                 coptsFilter,
                 features,
                 actionName,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java
index 505758b016..f2af6109b6 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java
@@ -69,7 +69,6 @@ public class CppCompileActionBuilder {
   private DotdFile dotdFile;
   private Artifact gcnoFile;
   private CppCompilationContext context = CppCompilationContext.EMPTY;
-  private final List<String> copts = new ArrayList<>();
   private final List<String> pluginOpts = new ArrayList<>();
   private final List<Pattern> nocopts = new ArrayList<>();
   private ImmutableList<PathFragment> extraSystemIncludePrefixes = ImmutableList.of();
@@ -176,7 +175,6 @@ public class CppCompileActionBuilder {
     this.dotdFile = other.dotdFile;
     this.gcnoFile = other.gcnoFile;
     this.context = other.context;
-    this.copts.addAll(other.copts);
     this.pluginOpts.addAll(other.pluginOpts);
     this.nocopts.addAll(other.nocopts);
     this.extraSystemIncludePrefixes = ImmutableList.copyOf(other.extraSystemIncludePrefixes);
@@ -405,7 +403,6 @@ public class CppCompileActionBuilder {
               cppConfiguration,
               context,
               actionContext,
-              ImmutableList.copyOf(copts),
               getNocoptPredicate(nocopts),
               getLipoScannables(realMandatoryInputs),
               cppSemantics,
@@ -437,7 +434,6 @@ public class CppCompileActionBuilder {
               cppConfiguration,
               context,
               actionContext,
-              ImmutableList.copyOf(copts),
               getNocoptPredicate(nocopts),
               specialInputsHandler,
               getLipoScannables(realMandatoryInputs),
@@ -678,16 +674,6 @@ public class CppCompileActionBuilder {
     return this;
   }
 
-  public CppCompileActionBuilder addCopt(String copt) {
-    copts.add(copt);
-    return this;
-  }
-
-  public CppCompileActionBuilder addCopts(Iterable<? extends String> copts) {
-    Iterables.addAll(this.copts, copts);
-    return this;
-  }
-
   public CppCompileActionBuilder addNocopts(Pattern nocopts) {
     this.nocopts.add(nocopts);
     return this;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java
index 99222ecc6f..2d3a3a1254 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java
@@ -134,6 +134,9 @@ public final class CppModel {
   /** Name of the build variable for the LTO indexing bitcode file. */
   public static final String LTO_INDEXING_BITCODE_FILE_VARIABLE_NAME = "lto_indexing_bitcode_file";
 
+  /** Build variable for all user provided copt flags. */
+  public static final String COPTS_VARIABLE_VALUE = "copts";
+
   private final CppSemantics semantics;
   private final RuleContext ruleContext;
   private final BuildConfiguration configuration;
@@ -143,7 +146,7 @@ public final class CppModel {
   private CppCompilationContext context;
   private final Set<CppSource> sourceFiles = new LinkedHashSet<>();
   private final List<Artifact> mandatoryInputs = new ArrayList<>();
-  private final List<String> copts = new ArrayList<>();
+  private final ImmutableList<String> copts;
   @Nullable private Pattern nocopts;
   private boolean fake;
   private boolean maySaveTemps;
@@ -164,19 +167,28 @@ public final class CppModel {
   private final FdoSupportProvider fdoSupport;
   private String linkedArtifactNameSuffix = "";
 
-  public CppModel(RuleContext ruleContext, CppSemantics semantics,
-      CcToolchainProvider ccToolchain, FdoSupportProvider fdoSupport) {
-    this(ruleContext, semantics, ccToolchain, fdoSupport, ruleContext.getConfiguration());
+  public CppModel(
+      RuleContext ruleContext,
+      CppSemantics semantics,
+      CcToolchainProvider ccToolchain,
+      FdoSupportProvider fdoSupport,
+      ImmutableList<String> copts) {
+    this(ruleContext, semantics, ccToolchain, fdoSupport, ruleContext.getConfiguration(), copts);
  }
 
-  public CppModel(RuleContext ruleContext, CppSemantics semantics,
-      CcToolchainProvider ccToolchain, FdoSupportProvider fdoSupport,
-      BuildConfiguration configuration) {
+  public CppModel(
+      RuleContext ruleContext,
+      CppSemantics semantics,
+      CcToolchainProvider ccToolchain,
+      FdoSupportProvider fdoSupport,
+      BuildConfiguration configuration,
+      ImmutableList<String> copts) {
     this.ruleContext = Preconditions.checkNotNull(ruleContext);
     this.semantics = semantics;
     this.ccToolchain = Preconditions.checkNotNull(ccToolchain);
     this.fdoSupport = Preconditions.checkNotNull(fdoSupport);
     this.configuration = configuration;
+    this.copts = copts;
     cppConfiguration = ruleContext.getFragment(CppConfiguration.class);
   }
 
@@ -255,14 +267,6 @@ public final class CppModel {
   }
 
   /**
-   * Adds the given copts.
-   */
-  public CppModel addCopts(Collection<String> copts) {
-    this.copts.addAll(copts);
-    return this;
-  }
-
-  /**
    * Sets the nocopts pattern. This is used to filter out flags from the system defined set of
    * flags. By default no filter is applied.
    */
@@ -462,6 +466,7 @@ public final class CppModel {
 
     buildVariables.addStringVariable(SOURCE_FILE_VARIABLE_NAME, sourceFile.getExecPathString());
     buildVariables.addStringVariable(OUTPUT_FILE_VARIABLE_NAME, outputFile.getExecPathString());
+    buildVariables.addStringSequenceVariable(COPTS_VARIABLE_VALUE, copts);
 
     if (builder.getTempOutputFile() != null) {
       realOutputFilePath = builder.getTempOutputFile().getPathString();
@@ -1342,7 +1347,7 @@ public final class CppModel {
     CppCompileActionBuilder builder =
         new CppCompileActionBuilder(ruleContext, label, ccToolchain, configuration);
     builder.setSourceFile(source);
-    builder.setContext(context).addCopts(copts);
+    builder.setContext(context);
     builder.addEnvironment(ccToolchain.getEnvironment());
     return builder;
   }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java
index a8c0160791..a73ea273ec 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java
@@ -74,7 +74,6 @@ public class FakeCppCompileAction extends CppCompileAction {
       CppConfiguration cppConfiguration,
       CppCompilationContext context,
       Class<? extends CppCompileActionContext> actionContext,
-      ImmutableList<String> copts,
       Predicate<String> nocopts,
       Iterable<IncludeScannable> lipoScannables,
       CppSemantics cppSemantics,
@@ -111,7 +110,6 @@ public class FakeCppCompileAction extends CppCompileAction {
         // time, so they can't depend on the contents of the ".d" file.)
         CppCompilationContext.disallowUndeclaredHeaders(context),
         actionContext,
-        copts,
         nocopts,
         VOID_SPECIAL_INPUTS_HANDLER,
         lipoScannables,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/CrosstoolCompilationSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/CrosstoolCompilationSupport.java
index 6541177500..c609fa8139 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/CrosstoolCompilationSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/CrosstoolCompilationSupport.java
@@ -448,10 +448,16 @@ public class CrosstoolCompilationSupport extends CompilationSupport {
             // For example, objc_proto_library can depend on a proto_library rule that does not
             // generate C++ protos.
             .setCheckDepsGenerateCpp(false)
-            .addCopts(getCompileRuleCopts())
+            .setCopts(
+                ImmutableList.<String>builder()
+                    .addAll(getCompileRuleCopts())
+                    .addAll(
+                        ruleContext
+                            .getFragment(ObjcConfiguration.class)
+                            .getCoptsForCompilationMode())
+                    .build())
             .addIncludeDirs(priorityHeaders)
             .addIncludeDirs(objcProvider.get(INCLUDE))
-            .addCopts(ruleContext.getFragment(ObjcConfiguration.class).getCoptsForCompilationMode())
             .addSystemIncludeDirs(objcProvider.get(INCLUDE_SYSTEM))
             .setCppModuleMap(intermediateArtifacts.moduleMap())
             .setLinkedArtifactNameSuffix(intermediateArtifacts.archiveFileNameSuffix())