aboutsummaryrefslogtreecommitdiffhomepage
path: root/src/main
diff options
context:
space:
mode:
authorGravatar hlopko <hlopko@google.com>2017-09-08 15:17:18 +0200
committerGravatar Philipp Wollermann <philwo@google.com>2017-09-11 13:06:46 +0200
commitf322ba774727597b3238c33929c7ef2071f134b4 (patch)
tree9794635aecee9c5b98eeef12cd1b341fd425b707 /src/main
parente4f390420c6b6ceef93e5e620a747a6d196bd172 (diff)
Introduce unfiltered_compile_flags build variable, rename copts variable to user_compile_flags
Also add magic to a feature named 'unfiltered_compile_flags' so the flags expanded from it are not subject to nocopt filtering. This is encore of https://github.com/bazelbuild/bazel/commit/268c0bcbf79f9f3f72d95fa51af0f1b18c5ce29e that was rolled back because it regressed memory. RELNOTES: None. PiperOrigin-RevId: 167989075
Diffstat (limited to 'src/main')
-rw-r--r--src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppSemantics.java9
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java65
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java22
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java125
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java6
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLine.java75
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java191
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java7
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java50
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java75
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java169
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java11
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/cpp/CppSemantics.java6
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java2
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCppSemantics.java6
15 files changed, 557 insertions, 262 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppSemantics.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppSemantics.java
index 507cc76de4..48af3cff2b 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppSemantics.java
@@ -14,6 +14,8 @@
package com.google.devtools.build.lib.bazel.rules.cpp;
+import com.google.common.base.Predicate;
+import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
@@ -51,13 +53,16 @@ public class BazelCppSemantics implements CppSemantics {
public void finalizeCompileActionBuilder(
RuleContext ruleContext,
CppCompileActionBuilder actionBuilder,
- FeatureSpecification featureSpecification) {
+ FeatureSpecification featureSpecification,
+ Predicate<String> coptsFilter,
+ ImmutableSet<String> features) {
actionBuilder.setCppConfiguration(ruleContext.getFragment(CppConfiguration.class));
actionBuilder.setActionContext(CppCompileActionContext.class);
// Because Bazel does not support include scanning, we need the entire crosstool filegroup,
// including header files, as opposed to just the "compile" filegroup.
actionBuilder.addTransitiveMandatoryInputs(actionBuilder.getToolchain().getCrosstool());
actionBuilder.setShouldScanIncludes(false);
+ actionBuilder.setCoptsFilter(coptsFilter);
}
@Override
@@ -88,7 +93,7 @@ public class BazelCppSemantics implements CppSemantics {
public boolean needsDotdInputPruning() {
return true;
}
-
+
@Override
public void validateAttributes(RuleContext ruleContext) {
}
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 4856b0a1e6..47aa40a0ec 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
@@ -14,6 +14,8 @@
package com.google.devtools.build.lib.rules.cpp;
import com.google.common.base.Joiner;
+import com.google.common.base.Predicate;
+import com.google.common.base.Predicates;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
@@ -163,12 +165,15 @@ public final class CcCommon {
}
}
- Pattern nocopts = getNoCopts(ruleContext);
- if (nocopts != null && nocopts.matcher("-Wno-future-warnings").matches()) {
- ruleContext.attributeWarning("nocopts",
- "Regular expression '" + nocopts.pattern() + "' is too general; for example, it matches "
- + "'-Wno-future-warnings'. Thus it might *re-enable* compiler warnings we wish to "
- + "disable globally. To disable all compiler warnings, add '-w' to copts instead");
+ if (!getCoptsFilter(ruleContext).apply("-Wno-future-warnings")) {
+ ruleContext.attributeWarning(
+ "nocopts",
+ String.format(
+ "Regular expression '%s' is too general; for example, it matches "
+ + "'-Wno-future-warnings'. Thus it might *re-enable* compiler warnings we wish "
+ + "to disable globally. To disable all compiler warnings, add '-w' to copts "
+ + "instead",
+ Preconditions.checkNotNull(getNoCoptsPattern(ruleContext))));
}
return ImmutableList.<String>builder()
@@ -349,27 +354,36 @@ public final class CcCommon {
return ImmutableList.copyOf(CppHelper.expandMakeVariables(ruleContext, "copts", unexpanded));
}
- Pattern getNoCopts() {
- return getNoCopts(ruleContext);
+ /** Returns copts filter built from the make variable expanded nocopts attribute. */
+ Predicate<String> getCoptsFilter() {
+ return getCoptsFilter(ruleContext);
}
- /**
- * Returns nocopts pattern built from the make variable expanded nocopts
- * attribute.
- */
- private static Pattern getNoCopts(RuleContext ruleContext) {
- Pattern nocopts = null;
- if (ruleContext.getRule().isAttrDefined(NO_COPTS_ATTRIBUTE, Type.STRING)) {
- String nocoptsAttr = ruleContext.expandMakeVariables(NO_COPTS_ATTRIBUTE,
- ruleContext.attributes().get(NO_COPTS_ATTRIBUTE, Type.STRING));
- try {
- nocopts = Pattern.compile(nocoptsAttr);
- } catch (PatternSyntaxException e) {
- ruleContext.attributeError(NO_COPTS_ATTRIBUTE,
- "invalid regular expression '" + nocoptsAttr + "': " + e.getMessage());
- }
+ /** @see CcCommon#getCoptsFilter() */
+ private static Predicate<String> getCoptsFilter(RuleContext ruleContext) {
+ Pattern noCoptsPattern = getNoCoptsPattern(ruleContext);
+ if (noCoptsPattern == null) {
+ return Predicates.alwaysTrue();
+ }
+ return flag -> !noCoptsPattern.matcher(flag).matches();
+ }
+
+ @Nullable
+ private static Pattern getNoCoptsPattern(RuleContext ruleContext) {
+ if (!ruleContext.getRule().isAttrDefined(NO_COPTS_ATTRIBUTE, Type.STRING)) {
+ return null;
+ }
+ String nocoptsAttr =
+ ruleContext.expandMakeVariables(
+ NO_COPTS_ATTRIBUTE, ruleContext.attributes().get(NO_COPTS_ATTRIBUTE, Type.STRING));
+ try {
+ return Pattern.compile(nocoptsAttr);
+ } catch (PatternSyntaxException e) {
+ ruleContext.attributeError(
+ NO_COPTS_ATTRIBUTE,
+ "invalid regular expression '" + nocoptsAttr + "': " + e.getMessage());
+ return null;
}
- return nocopts;
}
// TODO(bazel-team): calculating nocopts every time is not very efficient,
@@ -381,8 +395,7 @@ public final class CcCommon {
* otherwise.
*/
static boolean noCoptsMatches(String option, RuleContext ruleContext) {
- Pattern nocopts = getNoCopts(ruleContext);
- return nocopts == null ? false : nocopts.matcher(option).matches();
+ return !getCoptsFilter(ruleContext).apply(option);
}
private static final String DEFINES_ATTRIBUTE = "defines";
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 a7ffab3456..02c7c26002 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
@@ -19,6 +19,7 @@ import static java.util.stream.Collectors.toCollection;
import com.google.common.base.Function;
import com.google.common.base.Optional;
+import com.google.common.base.Predicate;
import com.google.common.base.Predicates;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
@@ -67,7 +68,6 @@ import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.TreeMap;
-import java.util.regex.Pattern;
import javax.annotation.Nullable;
/**
@@ -266,7 +266,7 @@ public final class CcLibraryHelper {
private final List<Artifact> nonCodeLinkerInputs = new ArrayList<>();
private ImmutableList<String> copts = ImmutableList.of();
private final List<String> linkopts = new ArrayList<>();
- @Nullable private Pattern nocopts;
+ private Predicate<String> coptsFilter = Predicates.alwaysTrue();
private final Set<String> defines = new LinkedHashSet<>();
private final List<TransitiveInfoCollection> deps = new ArrayList<>();
private final List<CppCompilationContext> depContexts = new ArrayList<>();
@@ -378,7 +378,7 @@ public final class CcLibraryHelper {
addLooseIncludeDirs(common.getLooseIncludeDirs());
addNonCodeLinkerInputs(common.getLinkerScripts());
addSystemIncludeDirs(common.getSystemIncludeDirs());
- setNoCopts(common.getNoCopts());
+ setCoptsFilter(common.getCoptsFilter());
setHeadersCheckingMode(semantics.determineHeadersCheckingMode(ruleContext));
return this;
}
@@ -640,11 +640,9 @@ public final class CcLibraryHelper {
return this;
}
- /**
- * Sets a pattern that is used to filter copts; set to {@code null} for no filtering.
- */
- public CcLibraryHelper setNoCopts(@Nullable Pattern nocopts) {
- this.nocopts = nocopts;
+ /** Sets a pattern that is used to filter copts; set to {@code null} for no filtering. */
+ public CcLibraryHelper setCoptsFilter(Predicate<String> coptsFilter) {
+ this.coptsFilter = Preconditions.checkNotNull(coptsFilter);
return this;
}
@@ -1150,12 +1148,7 @@ public final class CcLibraryHelper {
*/
private CppModel initializeCppModel() {
return new CppModel(
- ruleContext,
- semantics,
- ccToolchain,
- fdoSupport,
- configuration,
- copts)
+ ruleContext, semantics, ccToolchain, fdoSupport, configuration, copts, coptsFilter)
.addCompilationUnitSources(compilationUnitSources)
.setLinkTargetType(linkType)
.setNeverLink(neverlink)
@@ -1166,7 +1159,6 @@ public final class CcLibraryHelper {
// Note: this doesn't actually save the temps, it just makes the CppModel use the
// configurations --save_temps setting to decide whether to actually save the temps.
.setSaveTemps(true)
- .setNoCopts(nocopts)
.setDynamicLibrary(dynamicLibrary)
.addLinkopts(linkopts)
.setFeatureConfiguration(featureConfiguration)
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 705dc80a26..4a31bd7791 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
@@ -19,6 +19,7 @@ import com.google.common.base.Joiner;
import com.google.common.base.Optional;
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
+import com.google.common.base.Supplier;
import com.google.common.base.Throwables;
import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
@@ -31,10 +32,12 @@ 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.RuleContext;
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;
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.VariableValue;
+import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CToolchain;
import java.io.IOException;
@@ -1001,6 +1004,52 @@ public class CcToolchainFeatures implements Serializable {
}
/**
+ * Lazily computed string sequence. Exists as a memory optimization. Make sure the {@param
+ * supplier} doesn't capture anything that shouldn't outlive analysis phase (e.g. {@link
+ * RuleContext}).
+ */
+ private static final class LazyStringSequence implements VariableValue {
+
+ private final Supplier<ImmutableList<String>> supplier;
+
+ private LazyStringSequence(Supplier<ImmutableList<String>> supplier) {
+ this.supplier = Preconditions.checkNotNull(supplier);
+ }
+
+ @Override
+ public VariableValue getFieldValue(String variableName, String field) {
+ throw new ExpansionException(
+ String.format(
+ "Invalid toolchain configuration: Cannot expand variable '%s.%s': variable '%s' is "
+ + "sequence, expected structure",
+ variableName, field, variableName));
+ }
+
+ @Override
+ public String getStringValue(String variableName) {
+ throw new ExpansionException(
+ String.format(
+ "Invalid toolchain configuration: Cannot expand variable '%s': expected string, "
+ + "found sequence",
+ variableName));
+ }
+
+ @Override
+ public Iterable<? extends VariableValue> getSequenceValue(String variableName) {
+ return supplier
+ .get()
+ .stream()
+ .map(flag -> new StringValue(flag))
+ .collect(ImmutableList.toImmutableList());
+ }
+
+ @Override
+ public boolean isTruthy() {
+ return !supplier.get().isEmpty();
+ }
+ }
+
+ /**
* A sequence of structure values. Exists as a memory optimization - a typical build can contain
* millions of feature values, so getting rid of the overhead of {@code StructureValue} objects
* significantly reduces memory overhead.
@@ -1375,6 +1424,7 @@ public class CcToolchainFeatures implements Serializable {
/**
* Builder for {@code Variables}.
*/
+ // TODO(b/65472725): Forbid sequences with empty string in them.
public static class Builder {
private final Map<String, VariableValue> variablesMap = new LinkedHashMap<>();
private final Map<String, String> stringVariablesMap = new LinkedHashMap<>();
@@ -1394,10 +1444,7 @@ public class CcToolchainFeatures implements Serializable {
/** Add a string variable that expands {@code name} to {@code value}. */
public Builder addStringVariable(String name, String value) {
- Preconditions.checkArgument(
- !variablesMap.containsKey(name), "Cannot overwrite variable '%s'", name);
- Preconditions.checkArgument(
- !stringVariablesMap.containsKey(name), "Cannot overwrite variable '%s'", name);
+ checkVariableNotPresentAlready(name);
Preconditions.checkNotNull(
value, "Cannot set null as a value for variable '%s'", name);
stringVariablesMap.put(name, value);
@@ -1412,6 +1459,14 @@ public class CcToolchainFeatures implements Serializable {
return this;
}
+ /** Overrides a variable to expands {@code name} to {@code value} instead. */
+ public Builder overrideLazyStringSequenceVariable(
+ String name, Supplier<ImmutableList<String>> supplier) {
+ Preconditions.checkNotNull(supplier, "Cannot set null as a value for variable '%s'", name);
+ variablesMap.put(name, new LazyStringSequence(supplier));
+ return this;
+ }
+
/**
* Add a sequence variable that expands {@code name} to {@code values}.
*
@@ -1419,8 +1474,8 @@ public class CcToolchainFeatures implements Serializable {
* the values into a new list.
*/
public Builder addStringSequenceVariable(String name, ImmutableSet<String> values) {
- Preconditions.checkArgument(
- !variablesMap.containsKey(name), "Cannot overwrite variable '%s'", name);
+ checkVariableNotPresentAlready(name);
+ Preconditions.checkNotNull(values, "Cannot set null as a value for variable '%s'", name);
ImmutableList.Builder<String> builder = ImmutableList.builder();
builder.addAll(values);
variablesMap.put(name, new StringSequence(builder.build()));
@@ -1433,8 +1488,8 @@ public class CcToolchainFeatures implements Serializable {
* <p>Accepts values as NestedSet. Nested set is stored directly, not cloned, not flattened.
*/
public Builder addStringSequenceVariable(String name, NestedSet<String> values) {
- Preconditions.checkArgument(
- !variablesMap.containsKey(name), "Cannot overwrite variable '%s'", name);
+ checkVariableNotPresentAlready(name);
+ Preconditions.checkNotNull(values, "Cannot set null as a value for variable '%s'", name);
variablesMap.put(name, new StringSequence(values));
return this;
}
@@ -1448,19 +1503,26 @@ public class CcToolchainFeatures implements Serializable {
* side effects.
*/
public Builder addStringSequenceVariable(String name, Iterable<String> values) {
- Preconditions.checkArgument(
- !variablesMap.containsKey(name), "Cannot overwrite variable '%s'", name);
+ checkVariableNotPresentAlready(name);
+ Preconditions.checkNotNull(values, "Cannot set null as a value for variable '%s'", name);
variablesMap.put(name, new StringSequence(values));
return this;
}
+ public Builder addLazyStringSequenceVariable(
+ String name, Supplier<ImmutableList<String>> supplier) {
+ checkVariableNotPresentAlready(name);
+ Preconditions.checkNotNull(supplier, "Cannot set null as a value for variable '%s'", name);
+ variablesMap.put(name, new LazyStringSequence(supplier));
+ return this;
+ }
+
/**
* Add a variable built using {@code VariableValueBuilder} api that expands {@code name} to
* the value returned by the {@code builder}.
*/
public Builder addCustomBuiltVariable(String name, Variables.VariableValueBuilder builder) {
- Preconditions.checkArgument(
- !variablesMap.containsKey(name), "Cannot overwrite variable '%s'", name);
+ checkVariableNotPresentAlready(name);
Preconditions.checkNotNull(
builder,
"Cannot use null builder to get variable value for variable '%s'",
@@ -1472,15 +1534,19 @@ public class CcToolchainFeatures implements Serializable {
/** Add all string variables in a map. */
public Builder addAllStringVariables(Map<String, String> variables) {
for (String name : variables.keySet()) {
- Preconditions.checkArgument(
- !variablesMap.containsKey(name), "Cannot overwrite variable '%s'", name);
- Preconditions.checkArgument(
- !stringVariablesMap.containsKey(name), "Cannot overwrite variable '%s'", name);
+ checkVariableNotPresentAlready(name);
}
stringVariablesMap.putAll(variables);
return this;
}
+ private void checkVariableNotPresentAlready(String name) {
+ Preconditions.checkArgument(
+ !variablesMap.containsKey(name), "Cannot overwrite variable '%s'", name);
+ Preconditions.checkArgument(
+ !stringVariablesMap.containsKey(name), "Cannot overwrite variable '%s'", name);
+ }
+
/** Adds all variables to this builder. Note: cannot override already added variables. */
public Builder addAll(Variables variables) {
SetView<String> intersection =
@@ -1508,10 +1574,8 @@ public class CcToolchainFeatures implements Serializable {
return this;
}
- /**
- * @return a new {@Variables} object.
- */
- Variables build() {
+ /** @return a new {@Variables} object. */
+ public Variables build() {
return new Variables(
ImmutableMap.copyOf(variablesMap), ImmutableMap.copyOf(stringVariablesMap));
}
@@ -1732,6 +1796,27 @@ public class CcToolchainFeatures implements Serializable {
return commandLine;
}
+ /** @return the flags expanded for the given {@code action} in per-feature buckets. */
+ public ImmutableList<Pair<String, List<String>>> getPerFeatureExpansions(
+ String action, Variables variables) {
+ ImmutableList.Builder<Pair<String, List<String>>> perFeatureExpansions =
+ ImmutableList.builder();
+ if (actionIsConfigured(action)) {
+ List<String> commandLine = new ArrayList<>();
+ ActionConfig actionConfig = actionConfigByActionName.get(action);
+ actionConfig.expandCommandLine(variables, enabledFeatureNames, commandLine);
+ perFeatureExpansions.add(Pair.of(actionConfig.getName(), commandLine));
+ }
+
+ for (Feature feature : enabledFeatures) {
+ List<String> commandLine = new ArrayList<>();
+ feature.expandCommandLine(action, variables, enabledFeatureNames, commandLine);
+ perFeatureExpansions.add(Pair.of(feature.getName(), commandLine));
+ }
+
+ return perFeatureExpansions.build();
+ }
+
/** @return the environment variables (key/value pairs) for the given {@code action}. */
ImmutableMap<String, String> getEnvironmentVariables(String action, Variables variables) {
ImmutableMap.Builder<String, String> envBuilder = ImmutableMap.builder();
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 4a7cc2d2d4..6526877565 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
@@ -354,10 +354,14 @@ public final class CcToolchainProvider extends ToolchainInfo {
"Returns the default list of options which cannot be filtered by BUILD "
+ "rules. These should be appended to the command line after filtering."
)
- public ImmutableList<String> getUnfilteredCompilerOptions(Iterable<String> features) {
+ public ImmutableList<String> getUnfilteredCompilerOptionsWithSysroot(Iterable<String> features) {
return cppConfiguration.getUnfilteredCompilerOptions(features, sysroot);
}
+ public ImmutableList<String> getUnfilteredCompilerOptions(Iterable<String> features) {
+ return cppConfiguration.getUnfilteredCompilerOptions(features, null);
+ }
+
@SkylarkCallable(
name = "link_options_do_not_use",
structField = true,
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 3e1eaf69c4..db43758289 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
@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.rules.cpp;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Predicate;
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.actions.Artifact;
@@ -22,10 +23,10 @@ import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfig
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables;
import com.google.devtools.build.lib.rules.cpp.CppCompileAction.DotdFile;
import com.google.devtools.build.lib.util.FileType;
+import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.util.Preconditions;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.ArrayList;
-import java.util.Collection;
import java.util.List;
import java.util.Map;
import javax.annotation.Nullable;
@@ -37,7 +38,6 @@ public final class CompileCommandLine {
private final Artifact outputFile;
private final Label sourceLabel;
private final Predicate<String> coptsFilter;
- private final Collection<String> features;
private final FeatureConfiguration featureConfiguration;
private final CcToolchainFeatures.Variables variables;
private final String actionName;
@@ -50,7 +50,6 @@ public final class CompileCommandLine {
Artifact outputFile,
Label sourceLabel,
Predicate<String> coptsFilter,
- Collection<String> features,
FeatureConfiguration featureConfiguration,
CppConfiguration cppConfiguration,
CcToolchainFeatures.Variables variables,
@@ -61,7 +60,6 @@ public final class CompileCommandLine {
this.outputFile = Preconditions.checkNotNull(outputFile);
this.sourceLabel = Preconditions.checkNotNull(sourceLabel);
this.coptsFilter = coptsFilter;
- this.features = Preconditions.checkNotNull(features);
this.featureConfiguration = Preconditions.checkNotNull(featureConfiguration);
this.cppConfiguration = Preconditions.checkNotNull(cppConfiguration);
this.variables = variables;
@@ -111,34 +109,10 @@ public final class CompileCommandLine {
return commandLine;
}
- private boolean isObjcCompile(String actionName) {
- return (actionName.equals(CppCompileAction.OBJC_COMPILE)
- || actionName.equals(CppCompileAction.OBJCPP_COMPILE));
- }
-
public List<String> getCompilerOptions(
@Nullable CcToolchainFeatures.Variables overwrittenVariables) {
List<String> options = new ArrayList<>();
- CppConfiguration toolchain = cppConfiguration;
-
- addFilteredOptions(options, toolchain.getCompilerOptions(features));
- String sourceFilename = sourceFile.getExecPathString();
- if (CppFileTypes.C_SOURCE.matches(sourceFilename)) {
- addFilteredOptions(options, toolchain.getCOptions());
- }
- if (CppFileTypes.CPP_SOURCE.matches(sourceFilename)
- || CppFileTypes.CPP_HEADER.matches(sourceFilename)
- || CppFileTypes.CPP_MODULE_MAP.matches(sourceFilename)
- || CppFileTypes.CLIF_INPUT_PROTO.matches(sourceFilename)) {
- addFilteredOptions(options, toolchain.getCxxOptions(features));
- }
-
- // TODO(bazel-team): This needs to be before adding getUnfilteredCompilerOptions() and after
- // adding the warning flags until all toolchains are migrated; currently toolchains use the
- // unfiltered compiler options to inject include paths, which is superseded by the feature
- // configuration; on the other hand toolchains switch off warnings for the layering check
- // that will be re-added by the feature flags.
CcToolchainFeatures.Variables updatedVariables = variables;
if (variables != null && overwrittenVariables != null) {
CcToolchainFeatures.Variables.Builder variablesBuilder =
@@ -147,18 +121,14 @@ public final class CompileCommandLine {
variablesBuilder.addAndOverwriteAll(overwrittenVariables);
updatedVariables = variablesBuilder.build();
}
- addFilteredOptions(options, featureConfiguration.getCommandLine(actionName, updatedVariables));
+ addFilteredOptions(
+ options, featureConfiguration.getPerFeatureExpansions(actionName, updatedVariables));
- // 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.
if (isObjcCompile(actionName)) {
PathFragment sysroot = cppProvider.getSysroot();
if (sysroot != null) {
- options.add(toolchain.getSysrootCompilerOption(sysroot));
+ options.add(cppConfiguration.getSysrootCompilerOption(sysroot));
}
- } else {
- options.addAll(cppProvider.getUnfilteredCompilerOptions(features));
}
// Add the options of --per_file_copt, if the label or the base name of the source file
@@ -186,9 +156,22 @@ public final class CompileCommandLine {
return options;
}
+ private boolean isObjcCompile(String actionName) {
+ return (actionName.equals(CppCompileAction.OBJC_COMPILE)
+ || actionName.equals(CppCompileAction.OBJCPP_COMPILE));
+ }
+
// For each option in 'in', add it to 'out' unless it is matched by the 'coptsFilter' regexp.
- private void addFilteredOptions(List<String> out, List<String> in) {
- in.stream().filter(coptsFilter).forEachOrdered(out::add);
+ private void addFilteredOptions(
+ List<String> out, List<Pair<String, List<String>>> expandedFeatures) {
+ for (Pair<String, List<String>> pair : expandedFeatures) {
+ if (pair.getFirst().equals(CppRuleClasses.UNFILTERED_COMPILE_FLAGS_FEATURE_NAME)) {
+ out.addAll(pair.getSecond());
+ continue;
+ }
+
+ pair.getSecond().stream().filter(coptsFilter).forEachOrdered(out::add);
+ }
}
public Artifact getSourceFile() {
@@ -211,8 +194,8 @@ public final class CompileCommandLine {
* 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);
+ if (variables.isAvailable(CppModel.USER_COMPILE_FLAGS_VARIABLE_NAME)) {
+ return Variables.toStringList(variables, CppModel.USER_COMPILE_FLAGS_VARIABLE_NAME);
} else {
return ImmutableList.of();
}
@@ -223,7 +206,6 @@ public final class CompileCommandLine {
Artifact outputFile,
Label sourceLabel,
Predicate<String> coptsFilter,
- ImmutableList<String> features,
String actionName,
CppConfiguration cppConfiguration,
DotdFile dotdFile,
@@ -233,7 +215,6 @@ public final class CompileCommandLine {
outputFile,
sourceLabel,
coptsFilter,
- features,
actionName,
cppConfiguration,
dotdFile,
@@ -245,8 +226,7 @@ public final class CompileCommandLine {
private final Artifact sourceFile;
private final Artifact outputFile;
private final Label sourceLabel;
- private final Predicate<String> coptsFilter;
- private final Collection<String> features;
+ private Predicate<String> coptsFilter;
private FeatureConfiguration featureConfiguration;
private CcToolchainFeatures.Variables variables = Variables.EMPTY;
private final String actionName;
@@ -260,7 +240,6 @@ public final class CompileCommandLine {
Preconditions.checkNotNull(outputFile),
Preconditions.checkNotNull(sourceLabel),
Preconditions.checkNotNull(coptsFilter),
- Preconditions.checkNotNull(features),
Preconditions.checkNotNull(featureConfiguration),
Preconditions.checkNotNull(cppConfiguration),
Preconditions.checkNotNull(variables),
@@ -274,7 +253,6 @@ public final class CompileCommandLine {
Artifact outputFile,
Label sourceLabel,
Predicate<String> coptsFilter,
- Collection<String> features,
String actionName,
CppConfiguration cppConfiguration,
DotdFile dotdFile,
@@ -283,7 +261,6 @@ public final class CompileCommandLine {
this.outputFile = outputFile;
this.sourceLabel = sourceLabel;
this.coptsFilter = coptsFilter;
- this.features = features;
this.actionName = actionName;
this.cppConfiguration = cppConfiguration;
this.dotdFile = dotdFile;
@@ -300,5 +277,11 @@ public final class CompileCommandLine {
this.variables = variables;
return this;
}
+
+ @VisibleForTesting
+ Builder setCoptsFilter(Predicate<String> filter) {
+ this.coptsFilter = Preconditions.checkNotNull(filter);
+ return this;
+ }
}
}
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 8ed59a2fe0..958b26f7f6 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
@@ -15,7 +15,7 @@ package com.google.devtools.build.lib.rules.cpp;
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
-import java.util.Set;
+import com.google.common.collect.ImmutableSet;
/**
* A helper class for creating action_configs for the c++ actions.
@@ -33,7 +33,7 @@ public class CppActionConfigs {
public static String getCppActionConfigs(
CppPlatform platform,
- Set<String> features,
+ ImmutableSet<String> existingFeatureNames,
String gccToolPath,
String cppLinkDynamicLibraryToolPath,
String arToolPath,
@@ -41,7 +41,8 @@ public class CppActionConfigs {
boolean supportsEmbeddedRuntimes,
boolean supportsInterfaceSharedLibraries) {
String cppDynamicLibraryLinkerTool = "";
- if (!features.contains("dynamic_library_linker_tool") && supportsInterfaceSharedLibraries) {
+ if (!existingFeatureNames.contains("dynamic_library_linker_tool")
+ && supportsInterfaceSharedLibraries) {
cppDynamicLibraryLinkerTool =
""
+ "feature {"
@@ -65,7 +66,9 @@ public class CppActionConfigs {
" tool {",
" tool_path: '" + gccToolPath + "'",
" }",
- " implies: 'copts'",
+ " implies: 'legacy_compile_flags'",
+ " implies: 'user_compile_flags'",
+ " implies: 'unfiltered_compile_flags'",
"}",
"action_config {",
" config_name: 'preprocess-assemble'",
@@ -73,7 +76,9 @@ public class CppActionConfigs {
" tool {",
" tool_path: '" + gccToolPath + "'",
" }",
- " implies: 'copts'",
+ " implies: 'legacy_compile_flags'",
+ " implies: 'user_compile_flags'",
+ " implies: 'unfiltered_compile_flags'",
"}",
"action_config {",
" config_name: 'c-compile'",
@@ -81,7 +86,9 @@ public class CppActionConfigs {
" tool {",
" tool_path: '" + gccToolPath + "'",
" }",
- " implies: 'copts'",
+ " implies: 'legacy_compile_flags'",
+ " implies: 'user_compile_flags'",
+ " implies: 'unfiltered_compile_flags'",
"}",
"action_config {",
" config_name: 'c++-compile'",
@@ -89,7 +96,9 @@ public class CppActionConfigs {
" tool {",
" tool_path: '" + gccToolPath + "'",
" }",
- " implies: 'copts'",
+ " implies: 'legacy_compile_flags'",
+ " implies: 'user_compile_flags'",
+ " implies: 'unfiltered_compile_flags'",
"}",
"action_config {",
" config_name: 'c++-header-parsing'",
@@ -97,7 +106,9 @@ public class CppActionConfigs {
" tool {",
" tool_path: '" + gccToolPath + "'",
" }",
- " implies: 'copts'",
+ " implies: 'legacy_compile_flags'",
+ " implies: 'user_compile_flags'",
+ " implies: 'unfiltered_compile_flags'",
"}",
"action_config {",
" config_name: 'c++-header-preprocessing'",
@@ -105,7 +116,9 @@ public class CppActionConfigs {
" tool {",
" tool_path: '" + gccToolPath + "'",
" }",
- " implies: 'copts'",
+ " implies: 'legacy_compile_flags'",
+ " implies: 'user_compile_flags'",
+ " implies: 'unfiltered_compile_flags'",
"}",
"action_config {",
" config_name: 'c++-module-compile'",
@@ -113,7 +126,9 @@ public class CppActionConfigs {
" tool {",
" tool_path: '" + gccToolPath + "'",
" }",
- " implies: 'copts'",
+ " implies: 'legacy_compile_flags'",
+ " implies: 'user_compile_flags'",
+ " implies: 'unfiltered_compile_flags'",
"}",
"action_config {",
" config_name: 'c++-module-codegen'",
@@ -121,8 +136,33 @@ public class CppActionConfigs {
" tool {",
" tool_path: '" + gccToolPath + "'",
" }",
- " implies: 'copts'",
+ " implies: 'legacy_compile_flags'",
+ " implies: 'user_compile_flags'",
+ " implies: 'unfiltered_compile_flags'",
"}",
+ ifTrue(
+ !existingFeatureNames.contains(CppRuleClasses.LEGACY_COMPILE_FLAGS),
+ "feature {",
+ " name: 'legacy_compile_flags'",
+ " enabled: true",
+ " flag_set {",
+ " expand_if_all_available: 'legacy_compile_flags'",
+ " 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'",
+ " action: 'clif-match'",
+ " flag_group {",
+ " iterate_over: 'legacy_compile_flags'",
+ " flag: '%{legacy_compile_flags}'",
+ " }",
+ " }",
+ "}"),
// 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
@@ -133,7 +173,7 @@ public class CppActionConfigs {
// This combination gets user and system includes with specified name:
// -MD -MF <name>
ifTrue(
- !features.contains(CppRuleClasses.DEPENDENCY_FILE),
+ !existingFeatureNames.contains(CppRuleClasses.DEPENDENCY_FILE),
"feature {",
" name: 'dependency_file'",
" flag_set {",
@@ -162,7 +202,7 @@ public class CppActionConfigs {
// any value which differs for all translation units; we use the
// path to the object file.
ifTrue(
- !features.contains(CppRuleClasses.RANDOM_SEED),
+ !existingFeatureNames.contains(CppRuleClasses.RANDOM_SEED),
"feature {",
" name: 'random_seed'",
" flag_set {",
@@ -175,7 +215,7 @@ public class CppActionConfigs {
" }",
"}"),
ifTrue(
- !features.contains(CppRuleClasses.PIC),
+ !existingFeatureNames.contains(CppRuleClasses.PIC),
"feature {",
" name: 'pic'",
" flag_set {",
@@ -192,7 +232,7 @@ public class CppActionConfigs {
" }",
"}"),
ifTrue(
- !features.contains(CppRuleClasses.PER_OBJECT_DEBUG_INFO),
+ !existingFeatureNames.contains(CppRuleClasses.PER_OBJECT_DEBUG_INFO),
"feature {",
" name: 'per_object_debug_info'",
" flag_set {",
@@ -208,7 +248,7 @@ public class CppActionConfigs {
" }",
"}"),
ifTrue(
- !features.contains(CppRuleClasses.PREPROCESSOR_DEFINES),
+ !existingFeatureNames.contains(CppRuleClasses.PREPROCESSOR_DEFINES),
"feature {",
" name: 'preprocessor_defines'",
" flag_set {",
@@ -226,7 +266,7 @@ public class CppActionConfigs {
" }",
"}"),
ifTrue(
- !features.contains(CppRuleClasses.INCLUDE_PATHS),
+ !existingFeatureNames.contains(CppRuleClasses.INCLUDE_PATHS),
"feature {",
" name: 'include_paths'",
" flag_set {",
@@ -256,7 +296,7 @@ public class CppActionConfigs {
" }",
"}"),
ifTrue(
- !features.contains(CppRuleClasses.FDO_INSTRUMENT),
+ !existingFeatureNames.contains(CppRuleClasses.FDO_INSTRUMENT),
"feature {",
" name: 'fdo_instrument'",
" provides: 'profile'",
@@ -273,7 +313,7 @@ public class CppActionConfigs {
" }",
"}"),
ifTrue(
- !features.contains(CppRuleClasses.FDO_OPTIMIZE),
+ !existingFeatureNames.contains(CppRuleClasses.FDO_OPTIMIZE),
"feature {",
" name: 'fdo_optimize'",
" provides: 'profile'",
@@ -290,7 +330,7 @@ public class CppActionConfigs {
" }",
"}"),
ifTrue(
- !features.contains(CppRuleClasses.AUTOFDO),
+ !existingFeatureNames.contains(CppRuleClasses.AUTOFDO),
"feature {",
" name: 'autofdo'",
" provides: 'profile'",
@@ -305,7 +345,7 @@ public class CppActionConfigs {
" }",
"}"),
ifTrue(
- !features.contains(CppRuleClasses.LIPO),
+ !existingFeatureNames.contains(CppRuleClasses.LIPO),
"feature {",
" name: 'lipo'",
" requires { feature: 'autofdo' }",
@@ -418,7 +458,7 @@ public class CppActionConfigs {
// follow right after build_interface_libraries.
cppDynamicLibraryLinkerTool),
ifTrue(
- !features.contains("symbol_counts"),
+ !existingFeatureNames.contains("symbol_counts"),
"feature {",
" name: 'symbol_counts'",
" flag_set {",
@@ -431,7 +471,7 @@ public class CppActionConfigs {
" }",
"}"),
ifTrue(
- !features.contains("shared_flag"),
+ !existingFeatureNames.contains("shared_flag"),
"feature {",
" name: 'shared_flag'",
" flag_set {",
@@ -442,7 +482,7 @@ public class CppActionConfigs {
" }",
"}"),
ifTrue(
- !features.contains("linkstamps"),
+ !existingFeatureNames.contains("linkstamps"),
"feature {",
" name: 'linkstamps'",
" flag_set {",
@@ -456,7 +496,7 @@ public class CppActionConfigs {
" }",
"}"),
ifTrue(
- !features.contains("output_execpath_flags"),
+ !existingFeatureNames.contains("output_execpath_flags"),
"feature {",
" name: 'output_execpath_flags'",
" flag_set {",
@@ -469,7 +509,7 @@ public class CppActionConfigs {
" }",
"}"),
ifTrue(
- !features.contains("output_execpath_flags_executable"),
+ !existingFeatureNames.contains("output_execpath_flags_executable"),
"feature {",
" name: 'output_execpath_flags_executable'",
" flag_set {",
@@ -498,7 +538,7 @@ public class CppActionConfigs {
" }",
"}"),
ifTrue(
- !features.contains("runtime_library_search_directories"),
+ !existingFeatureNames.contains("runtime_library_search_directories"),
"feature {",
" name: 'runtime_library_search_directories',",
" flag_set {",
@@ -523,7 +563,7 @@ public class CppActionConfigs {
" }",
"}"),
ifTrue(
- !features.contains("library_search_directories"),
+ !existingFeatureNames.contains("library_search_directories"),
"feature {",
" name: 'library_search_directories'",
" flag_set {",
@@ -537,7 +577,7 @@ public class CppActionConfigs {
" }",
"}"),
ifTrue(
- !features.contains("archiver_flags"),
+ !existingFeatureNames.contains("archiver_flags"),
"feature {",
" name: 'archiver_flags'",
" flag_set {",
@@ -558,7 +598,7 @@ public class CppActionConfigs {
" }",
"}"),
ifTrue(
- !features.contains("libraries_to_link"),
+ !existingFeatureNames.contains("libraries_to_link"),
"feature {",
" name: 'libraries_to_link'",
" flag_set {",
@@ -719,7 +759,7 @@ public class CppActionConfigs {
" }",
"}"),
ifTrue(
- !features.contains("force_pic_flags"),
+ !existingFeatureNames.contains("force_pic_flags"),
"feature {",
" name: 'force_pic_flags'",
" flag_set {",
@@ -731,7 +771,7 @@ public class CppActionConfigs {
" }",
"}"),
ifTrue(
- !features.contains("legacy_link_flags"),
+ !existingFeatureNames.contains("legacy_link_flags"),
"feature {",
" name: 'legacy_link_flags'",
" flag_set {",
@@ -745,7 +785,7 @@ public class CppActionConfigs {
" }",
"}"),
ifTrue(
- !features.contains("fission_support"),
+ !existingFeatureNames.contains("fission_support"),
"feature {",
" name: 'fission_support'",
" flag_set {",
@@ -759,7 +799,7 @@ public class CppActionConfigs {
" }",
"}"),
ifTrue(
- !features.contains("strip_debug_symbols"),
+ !existingFeatureNames.contains("strip_debug_symbols"),
"feature {",
" name: 'strip_debug_symbols'",
" flag_set {",
@@ -773,7 +813,7 @@ public class CppActionConfigs {
" }",
"}"),
ifTrue(
- !features.contains("linker_param_file"),
+ !existingFeatureNames.contains("linker_param_file"),
"feature {",
" name: 'linker_param_file'",
" flag_set {",
@@ -796,7 +836,7 @@ public class CppActionConfigs {
" }",
"}"),
ifTrue(
- !features.contains(CppRuleClasses.COVERAGE),
+ !existingFeatureNames.contains(CppRuleClasses.COVERAGE),
"feature {",
" name: 'coverage'",
"}",
@@ -859,28 +899,6 @@ public class CppActionConfigs {
" feature: 'coverage'",
" }",
"}"),
- ifTrue(
- !features.contains(CppRuleClasses.COPTS),
- "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'",
- " action: 'clif-match'",
- " flag_group {",
- " iterate_over: 'copts'",
- " flag: '%{copts}'",
- " }",
- " }",
- "}"),
"action_config {",
" config_name: 'strip'",
" action_name: 'strip'",
@@ -923,6 +941,63 @@ public class CppActionConfigs {
"}"));
}
+ public static String getFeaturesToAppearLastInToolchain(
+ ImmutableSet<String> existingFeatureNames) {
+ return Joiner.on("\n")
+ .join(
+ ImmutableList.of(
+ ifTrue(
+ !existingFeatureNames.contains("user_compile_flags"),
+ "feature {",
+ " name: 'user_compile_flags'",
+ " enabled: true",
+ " flag_set {",
+ " expand_if_all_available: 'user_compile_flags'",
+ " 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'",
+ " action: 'clif-match'",
+ " flag_group {",
+ " iterate_over: 'user_compile_flags'",
+ " flag: '%{user_compile_flags}'",
+ " }",
+ " }",
+ "}"),
+ // unfiltered_compile_flags contain system include paths. These must be added
+ // after the user provided options (present in legacy_compile_flags build
+ // variable above), otherwise users adding include paths will not pick up their own
+ // include paths first.
+ ifTrue(
+ !existingFeatureNames.contains("unfiltered_compile_flags"),
+ "feature {",
+ " name: 'unfiltered_compile_flags'",
+ " enabled: true",
+ " flag_set {",
+ " expand_if_all_available: 'unfiltered_compile_flags'",
+ " 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'",
+ " action: 'clif-match'",
+ " flag_group {",
+ " iterate_over: 'unfiltered_compile_flags'",
+ " flag: '%{unfiltered_compile_flags}'",
+ " }",
+ " }",
+ "}")));
+ }
+
private static String ifLinux(CppPlatform platform, String... lines) {
return ifTrue(platform == CppPlatform.LINUX, 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 ed46336d53..a8d3a38bbb 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
@@ -250,7 +250,6 @@ public class CppCompileAction extends AbstractAction
*
* @param owner the owner of the action, usually the configured target that emitted it
* @param allInputs the list of all action inputs.
- * @param features TODO(bazel-team): Add parameter description.
* @param featureConfiguration TODO(bazel-team): Add parameter description.
* @param variables TODO(bazel-team): Add parameter description.
* @param sourceFile the source file that should be compiled. {@code mandatoryInputs} must contain
@@ -285,9 +284,6 @@ public class CppCompileAction extends AbstractAction
protected CppCompileAction(
ActionOwner owner,
NestedSet<Artifact> allInputs,
- // TODO(bazel-team): Eventually we will remove 'features'; all functionality in 'features'
- // will be provided by 'featureConfiguration'.
- ImmutableList<String> features,
FeatureConfiguration featureConfiguration,
CcToolchainFeatures.Variables variables,
Artifact sourceFile,
@@ -352,7 +348,6 @@ public class CppCompileAction extends AbstractAction
outputFile,
sourceLabel,
coptsFilter,
- features,
actionName,
cppConfiguration,
dotdFile,
@@ -802,7 +797,7 @@ public class CppCompileAction extends AbstractAction
*/
@VisibleForTesting
public List<String> getCompilerOptions() {
- return compileCommandLine.getCompilerOptions(/*updatedVariables=*/ null);
+ return compileCommandLine.getCompilerOptions(/* overwrittenVariables= */ null);
}
@Override
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 6020ae906b..1db66cabc7 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
@@ -38,14 +38,11 @@ import com.google.devtools.build.lib.util.Preconditions;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.ArrayList;
-import java.util.Collection;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
-import java.util.Set;
import java.util.UUID;
import java.util.function.Consumer;
-import java.util.regex.Pattern;
/**
* Builder class to construct C++ compile actions.
@@ -55,7 +52,6 @@ public class CppCompileActionBuilder {
private final ActionOwner owner;
private final BuildConfiguration configuration;
- private final List<String> features = new ArrayList<>();
private CcToolchainFeatures.FeatureConfiguration featureConfiguration;
private CcToolchainFeatures.Variables variables = Variables.EMPTY;
private Artifact sourceFile;
@@ -70,7 +66,7 @@ public class CppCompileActionBuilder {
private Artifact gcnoFile;
private CppCompilationContext context = CppCompilationContext.EMPTY;
private final List<String> pluginOpts = new ArrayList<>();
- private final List<Pattern> nocopts = new ArrayList<>();
+ private Predicate<String> coptsFilter = Predicates.alwaysTrue();
private ImmutableList<PathFragment> extraSystemIncludePrefixes = ImmutableList.of();
private boolean usePic;
private boolean allowUsingHeaderModules;
@@ -101,7 +97,6 @@ public class CppCompileActionBuilder {
sourceLabel,
ruleContext.getConfiguration(),
getLipoScannableMap(ruleContext),
- ruleContext.getFeatures(),
ccToolchain);
}
@@ -116,7 +111,6 @@ public class CppCompileActionBuilder {
sourceLabel,
configuration,
getLipoScannableMap(ruleContext),
- ruleContext.getFeatures(),
ccToolchain);
}
@@ -126,14 +120,12 @@ public class CppCompileActionBuilder {
Label sourceLabel,
BuildConfiguration configuration,
Map<Artifact, IncludeScannable> lipoScannableMap,
- Set<String> features,
CcToolchainProvider ccToolchain) {
this.owner = actionOwner;
this.sourceLabel = sourceLabel;
this.configuration = configuration;
this.cppConfiguration = configuration.getFragment(CppConfiguration.class);
this.lipoScannableMap = ImmutableMap.copyOf(lipoScannableMap);
- this.features.addAll(features);
this.mandatoryInputsBuilder = NestedSetBuilder.stableOrder();
this.allowUsingHeaderModules = true;
this.localShellEnvironment = configuration.getLocalShellEnvironment();
@@ -161,7 +153,6 @@ public class CppCompileActionBuilder {
*/
public CppCompileActionBuilder(CppCompileActionBuilder other) {
this.owner = other.owner;
- this.features.addAll(other.features);
this.featureConfiguration = other.featureConfiguration;
this.sourceFile = other.sourceFile;
this.sourceLabel = other.sourceLabel;
@@ -176,7 +167,7 @@ public class CppCompileActionBuilder {
this.gcnoFile = other.gcnoFile;
this.context = other.context;
this.pluginOpts.addAll(other.pluginOpts);
- this.nocopts.addAll(other.nocopts);
+ this.coptsFilter = other.coptsFilter;
this.extraSystemIncludePrefixes = ImmutableList.copyOf(other.extraSystemIncludePrefixes);
this.specialInputsHandler = other.specialInputsHandler;
this.actionClassId = other.actionClassId;
@@ -216,23 +207,6 @@ public class CppCompileActionBuilder {
return mandatoryInputsBuilder.build();
}
- private static Predicate<String> getNocoptPredicate(Collection<Pattern> patterns) {
- final ImmutableList<Pattern> finalPatterns = ImmutableList.copyOf(patterns);
- if (finalPatterns.isEmpty()) {
- return Predicates.alwaysTrue();
- } else {
- return option -> {
- for (Pattern pattern : finalPatterns) {
- if (pattern.matcher(option).matches()) {
- return false;
- }
- }
-
- return true;
- };
- }
- }
-
private Iterable<IncludeScannable> getLipoScannables(NestedSet<Artifact> realMandatoryInputs) {
boolean fake = tempOutputFile != null;
@@ -385,7 +359,6 @@ public class CppCompileActionBuilder {
new FakeCppCompileAction(
owner,
allInputs,
- ImmutableList.copyOf(features),
featureConfiguration,
variables,
sourceFile,
@@ -403,7 +376,7 @@ public class CppCompileActionBuilder {
cppConfiguration,
context,
actionContext,
- getNocoptPredicate(nocopts),
+ coptsFilter,
getLipoScannables(realMandatoryInputs),
cppSemantics,
ccToolchain,
@@ -413,7 +386,6 @@ public class CppCompileActionBuilder {
new CppCompileAction(
owner,
allInputs,
- ImmutableList.copyOf(features),
featureConfiguration,
variables,
sourceFile,
@@ -434,7 +406,7 @@ public class CppCompileActionBuilder {
cppConfiguration,
context,
actionContext,
- getNocoptPredicate(nocopts),
+ coptsFilter,
specialInputsHandler,
getLipoScannables(realMandatoryInputs),
additionalIncludeFiles.build(),
@@ -535,10 +507,8 @@ public class CppCompileActionBuilder {
return this;
}
- /**
- * Returns the build variables to be used for the action.
- */
- CcToolchainFeatures.Variables getVariables() {
+ /** Returns the build variables to be used for the action. */
+ public CcToolchainFeatures.Variables getVariables() {
return variables;
}
@@ -674,11 +644,6 @@ public class CppCompileActionBuilder {
return this;
}
- public CppCompileActionBuilder addNocopts(Pattern nocopts) {
- this.nocopts.add(nocopts);
- return this;
- }
-
public CppCompileActionBuilder setContext(CppCompilationContext context) {
this.context = context;
return this;
@@ -714,4 +679,7 @@ public class CppCompileActionBuilder {
return ccToolchain;
}
+ public void setCoptsFilter(Predicate<String> coptsFilter) {
+ this.coptsFilter = Preconditions.checkNotNull(coptsFilter);
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
index a36781773f..539883bb72 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
@@ -55,6 +55,7 @@ import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CTool
import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.LinkingModeFlags;
import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.LipoMode;
import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.ToolPath;
+import com.google.protobuf.Descriptors.FieldDescriptor;
import com.google.protobuf.TextFormat;
import com.google.protobuf.TextFormat.ParseException;
import java.io.Serializable;
@@ -631,12 +632,13 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
}
}
- ImmutableSet.Builder<String> featuresBuilder = ImmutableSet.builder();
- for (CToolchain.Feature feature : toolchain.getFeatureList()) {
- featuresBuilder.add(feature.getName());
- }
- Set<String> features = featuresBuilder.build();
- if (!features.contains(CppRuleClasses.NO_LEGACY_FEATURES)) {
+ ImmutableSet<String> featureNames =
+ toolchain
+ .getFeatureList()
+ .stream()
+ .map(feature -> feature.getName())
+ .collect(ImmutableSet.toImmutableSet());
+ if (!featureNames.contains(CppRuleClasses.NO_LEGACY_FEATURES)) {
try {
String gccToolPath = "DUMMY_GCC_TOOL";
String linkerToolPath = "DUMMY_LINKER_TOOL";
@@ -657,10 +659,28 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
stripToolPath = tool.getPath();
}
}
+
+ // TODO(b/30109612): Remove fragile legacyCompileFlags shuffle once there are no legacy
+ // crosstools.
+ // Existing projects depend on flags from legacy toolchain fields appearing first on the
+ // compile command line. 'legacy_compile_flags' feature contains all these flags, and so it
+ // needs to appear before other features from {@link CppActionConfigs}.
+ CToolchain.Feature legacyCompileFlagsFeature =
+ toolchain
+ .getFeatureList()
+ .stream()
+ .filter(feature -> feature.getName().equals(CppRuleClasses.LEGACY_COMPILE_FLAGS))
+ .findFirst()
+ .orElse(null);
+ if (legacyCompileFlagsFeature != null) {
+ toolchainBuilder.addFeature(legacyCompileFlagsFeature);
+ toolchain = removeLegacyCompileFlagsFeatureFromToolchain(toolchain);
+ }
+
TextFormat.merge(
CppActionConfigs.getCppActionConfigs(
getTargetLibc().equals("macosx") ? CppPlatform.MAC : CppPlatform.LINUX,
- features,
+ featureNames,
gccToolPath,
linkerToolPath,
arToolPath,
@@ -676,9 +696,34 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
}
toolchainBuilder.mergeFrom(toolchain);
+
+ if (!featureNames.contains(CppRuleClasses.NO_LEGACY_FEATURES)) {
+ try {
+ TextFormat.merge(
+ CppActionConfigs.getFeaturesToAppearLastInToolchain(featureNames), toolchainBuilder);
+ } catch (ParseException e) {
+ // Can only happen if we change the proto definition without changing our
+ // configuration above.
+ throw new RuntimeException(e);
+ }
+ }
return toolchainBuilder.build();
}
+ private CToolchain removeLegacyCompileFlagsFeatureFromToolchain(CToolchain toolchain) {
+ FieldDescriptor featuresFieldDescriptor = CToolchain.getDescriptor().findFieldByName("feature");
+ return toolchain
+ .toBuilder()
+ .setField(
+ featuresFieldDescriptor,
+ toolchain
+ .getFeatureList()
+ .stream()
+ .filter(feature -> !feature.getName().equals(CppRuleClasses.LEGACY_COMPILE_FLAGS))
+ .collect(ImmutableList.toImmutableList()))
+ .build();
+ }
+
@VisibleForTesting
static CompilationMode importCompilationMode(CrosstoolConfig.CompilationMode mode) {
return CompilationMode.valueOf(mode.name());
@@ -1963,6 +2008,22 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
return requestedFeatures.build();
}
+ public ImmutableList<String> collectLegacyCompileFlags(
+ String sourceFilename, ImmutableSet<String> features) {
+ ImmutableList.Builder<String> legacyCompileFlags = ImmutableList.builder();
+ legacyCompileFlags.addAll(getCompilerOptions(features));
+ if (CppFileTypes.C_SOURCE.matches(sourceFilename)) {
+ legacyCompileFlags.addAll(getCOptions());
+ }
+ if (CppFileTypes.CPP_SOURCE.matches(sourceFilename)
+ || CppFileTypes.CPP_HEADER.matches(sourceFilename)
+ || CppFileTypes.CPP_MODULE_MAP.matches(sourceFilename)
+ || CppFileTypes.CLIF_INPUT_PROTO.matches(sourceFilename)) {
+ legacyCompileFlags.addAll(getCxxOptions(features));
+ }
+ return legacyCompileFlags.build();
+ }
+
public static PathFragment computeDefaultSysroot(CToolchain toolchain) {
PathFragment defaultSysroot =
toolchain.getBuiltinSysroot().length() == 0
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 8e1d578f66..07a3f41cce 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
@@ -14,6 +14,9 @@
package com.google.devtools.build.lib.rules.cpp;
+import com.google.common.base.Predicate;
+import com.google.common.base.Predicates;
+import com.google.common.base.Supplier;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
@@ -50,8 +53,6 @@ import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
-import java.util.regex.Pattern;
-import javax.annotation.Nullable;
/**
* Representation of a C/C++ compilation. Its purpose is to share the code that creates compilation
@@ -121,6 +122,9 @@ public final class CppModel {
*/
public static final String SYSTEM_INCLUDE_PATHS_VARIABLE_NAME = "system_include_paths";
+ /** Name of the build variable for the dependency file path */
+ public static final String DEPENDENCY_FILE_VARIABLE_NAME = "dependency_file";
+
/** Name of the build variable for the collection of macros defined for preprocessor. */
public static final String PREPROCESSOR_DEFINES_VARIABLE_NAME = "preprocessor_defines";
@@ -137,12 +141,24 @@ 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";
-
/** Name of the build variable for stripopts for the strip action. */
public static final String STRIPOPTS_VARIABLE_NAME = "stripopts";
+ /**
+ * Build variable for all flags coming from legacy crosstool fields, such as compiler_flag,
+ * optional_compiler_flag, cxx_flag, optional_cxx_flag.
+ */
+ public static final String LEGACY_COMPILE_FLAGS_VARIABLE_NAME = "legacy_compile_flags";
+
+ /**
+ * Build variable for all flags coming from copt rule attribute, and from --copt, --cxxopt, or
+ * --conlyopt options.
+ */
+ public static final String USER_COMPILE_FLAGS_VARIABLE_NAME = "user_compile_flags";
+
+ /** Build variable for flags coming from unfiltered_cxx_flag CROSSTOOL fields. */
+ public static final String UNFILTERED_COMPILE_FLAGS_VARIABLE_NAME = "unfiltered_compile_flags";
+
private final CppSemantics semantics;
private final RuleContext ruleContext;
private final BuildConfiguration configuration;
@@ -153,7 +169,7 @@ public final class CppModel {
private final Set<CppSource> sourceFiles = new LinkedHashSet<>();
private final List<Artifact> mandatoryInputs = new ArrayList<>();
private final ImmutableList<String> copts;
- @Nullable private Pattern nocopts;
+ private final Predicate<String> coptsFilter;
private boolean fake;
private boolean maySaveTemps;
private boolean onlySingleOutput;
@@ -172,6 +188,7 @@ public final class CppModel {
private final CcToolchainProvider ccToolchain;
private final FdoSupportProvider fdoSupport;
private String linkedArtifactNameSuffix = "";
+ private final ImmutableSet<String> features;
public CppModel(
RuleContext ruleContext,
@@ -179,7 +196,31 @@ public final class CppModel {
CcToolchainProvider ccToolchain,
FdoSupportProvider fdoSupport,
ImmutableList<String> copts) {
- this(ruleContext, semantics, ccToolchain, fdoSupport, ruleContext.getConfiguration(), copts);
+ this(
+ ruleContext,
+ semantics,
+ ccToolchain,
+ fdoSupport,
+ ruleContext.getConfiguration(),
+ copts,
+ Predicates.alwaysTrue());
+ }
+
+ public CppModel(
+ RuleContext ruleContext,
+ CppSemantics semantics,
+ CcToolchainProvider ccToolchain,
+ FdoSupportProvider fdoSupport,
+ ImmutableList<String> copts,
+ Predicate<String> coptsFilter) {
+ this(
+ ruleContext,
+ semantics,
+ ccToolchain,
+ fdoSupport,
+ ruleContext.getConfiguration(),
+ copts,
+ coptsFilter);
}
public CppModel(
@@ -188,14 +229,17 @@ public final class CppModel {
CcToolchainProvider ccToolchain,
FdoSupportProvider fdoSupport,
BuildConfiguration configuration,
- ImmutableList<String> copts) {
+ ImmutableList<String> copts,
+ Predicate<String> coptsFilter) {
this.ruleContext = Preconditions.checkNotNull(ruleContext);
this.semantics = semantics;
this.ccToolchain = Preconditions.checkNotNull(ccToolchain);
this.fdoSupport = Preconditions.checkNotNull(fdoSupport);
this.configuration = configuration;
this.copts = copts;
+ this.coptsFilter = Preconditions.checkNotNull(coptsFilter);
cppConfiguration = ruleContext.getFragment(CppConfiguration.class);
+ features = ruleContext.getFeatures();
}
private Artifact getDwoFile(Artifact outputFile) {
@@ -273,15 +317,6 @@ public final class CppModel {
}
/**
- * Sets the nocopts pattern. This is used to filter out flags from the system defined set of
- * flags. By default no filter is applied.
- */
- public CppModel setNoCopts(@Nullable Pattern nocopts) {
- this.nocopts = nocopts;
- return this;
- }
-
- /**
* Adds the given linkopts to the optional dynamic library link command.
*/
public CppModel addLinkopts(Collection<String> linkopts) {
@@ -432,10 +467,6 @@ public final class CppModel {
private CppCompileActionBuilder initializeCompileAction(
Artifact sourceArtifact, Label sourceLabel) {
CppCompileActionBuilder builder = createCompileActionBuilder(sourceArtifact, sourceLabel);
- if (nocopts != null) {
- builder.addNocopts(nocopts);
- }
-
builder.setFeatureConfiguration(featureConfiguration);
return builder;
@@ -450,6 +481,30 @@ public final class CppModel {
return result.build();
}
+ /**
+ * Supplier that computes unfiltered_compile_flags lazily at the execution phase.
+ *
+ * <p>Dear friends of the lambda, this method exists to limit the scope of captured variables
+ * only to arguments (to prevent accidental capture of enclosing instance which could regress
+ * memory).
+ */
+ public static Supplier<ImmutableList<String>> getUnfilteredCompileFlagsSupplier(
+ CcToolchainProvider ccToolchain, ImmutableSet<String> features) {
+ return () -> ccToolchain.getUnfilteredCompilerOptionsWithSysroot(features);
+ }
+
+ /**
+ * Supplier that computes legacy_compile_flags lazily at the execution phase.
+ *
+ * <p>Dear friends of the lambda, this method exists to limit the scope of captured variables
+ * only to arguments (to prevent accidental capture of enclosing instance which could regress
+ * memory).
+ */
+ public static Supplier<ImmutableList<String>> getLegacyCompileFlagsSupplier(
+ CppConfiguration cppConfiguration, String sourceFilename, ImmutableSet<String> features) {
+ return () -> cppConfiguration.collectLegacyCompileFlags(sourceFilename, features);
+ }
+
private void setupCompileBuildVariables(
CppCompileActionBuilder builder,
boolean usePic,
@@ -463,8 +518,6 @@ public final class CppModel {
CcToolchainFeatures.Variables.Builder buildVariables =
new CcToolchainFeatures.Variables.Builder();
- // TODO(bazel-team): Pull out string constants for all build variables.
-
CppCompilationContext builderContext = builder.getContext();
Artifact sourceFile = builder.getSourceFile();
Artifact outputFile = builder.getOutputFile();
@@ -472,7 +525,19 @@ 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);
+ buildVariables.addStringSequenceVariable(USER_COMPILE_FLAGS_VARIABLE_NAME, copts);
+
+ String sourceFilename = sourceFile.getExecPathString();
+ buildVariables.addLazyStringSequenceVariable(
+ LEGACY_COMPILE_FLAGS_VARIABLE_NAME,
+ getLegacyCompileFlagsSupplier(cppConfiguration, sourceFilename, features));
+
+ if (!CppFileTypes.OBJC_SOURCE.matches(sourceFilename)
+ && !CppFileTypes.OBJCPP_SOURCE.matches(sourceFilename)) {
+ buildVariables.addLazyStringSequenceVariable(
+ UNFILTERED_COMPILE_FLAGS_VARIABLE_NAME,
+ getUnfilteredCompileFlagsSupplier(ccToolchain, features));
+ }
if (builder.getTempOutputFile() != null) {
realOutputFilePath = builder.getTempOutputFile().getPathString();
@@ -495,7 +560,7 @@ public final class CppModel {
// Set dependency_file to enable <object>.d file generation.
if (dotdFile != null) {
buildVariables.addStringVariable(
- "dependency_file", dotdFile.getSafeExecPath().getPathString());
+ DEPENDENCY_FILE_VARIABLE_NAME, dotdFile.getSafeExecPath().getPathString());
}
if (featureConfiguration.isEnabled(CppRuleClasses.MODULE_MAPS) && cppModuleMap != null) {
@@ -706,7 +771,11 @@ public final class CppModel {
builder.getContext().getCppModuleMap(),
ImmutableMap.of());
semantics.finalizeCompileActionBuilder(
- ruleContext, builder, featureConfiguration.getFeatureSpecification());
+ ruleContext,
+ builder,
+ featureConfiguration.getFeatureSpecification(),
+ coptsFilter,
+ features);
CppCompileAction compileAction = builder.buildOrThrowRuleError(ruleContext);
env.registerAction(compileAction);
Artifact tokenFile = compileAction.getOutputFile();
@@ -768,7 +837,11 @@ public final class CppModel {
builder.setDwoFile(dwoFile);
semantics.finalizeCompileActionBuilder(
- ruleContext, builder, featureConfiguration.getFeatureSpecification());
+ ruleContext,
+ builder,
+ featureConfiguration.getFeatureSpecification(),
+ coptsFilter,
+ features);
CppCompileAction compileAction = builder.buildOrThrowRuleError(ruleContext);
AnalysisEnvironment env = ruleContext.getAnalysisEnvironment();
env.registerAction(compileAction);
@@ -830,7 +903,11 @@ public final class CppModel {
builder.getContext().getCppModuleMap(),
/* sourceSpecificBuildVariables= */ ImmutableMap.of());
semantics.finalizeCompileActionBuilder(
- ruleContext, builder, featureConfiguration.getFeatureSpecification());
+ ruleContext,
+ builder,
+ featureConfiguration.getFeatureSpecification(),
+ coptsFilter,
+ features);
CppCompileAction compileAction = builder.buildOrThrowRuleError(ruleContext);
env.registerAction(compileAction);
Artifact tokenFile = compileAction.getOutputFile();
@@ -916,7 +993,11 @@ public final class CppModel {
picBuilder.setLtoIndexingFile(ltoIndexingFile);
semantics.finalizeCompileActionBuilder(
- ruleContext, picBuilder, featureConfiguration.getFeatureSpecification());
+ ruleContext,
+ picBuilder,
+ featureConfiguration.getFeatureSpecification(),
+ coptsFilter,
+ features);
CppCompileAction picAction = picBuilder.buildOrThrowRuleError(ruleContext);
env.registerAction(picAction);
directOutputs.add(picAction.getOutputFile());
@@ -983,7 +1064,11 @@ public final class CppModel {
builder.setLtoIndexingFile(ltoIndexingFile);
semantics.finalizeCompileActionBuilder(
- ruleContext, builder, featureConfiguration.getFeatureSpecification());
+ ruleContext,
+ builder,
+ featureConfiguration.getFeatureSpecification(),
+ coptsFilter,
+ features);
CppCompileAction compileAction = builder.buildOrThrowRuleError(ruleContext);
env.registerAction(compileAction);
Artifact objectFile = compileAction.getOutputFile();
@@ -1024,7 +1109,11 @@ public final class CppModel {
builder.getContext().getCppModuleMap(),
source.getBuildVariables());
semantics.finalizeCompileActionBuilder(
- ruleContext, builder, featureConfiguration.getFeatureSpecification());
+ ruleContext,
+ builder,
+ featureConfiguration.getFeatureSpecification(),
+ coptsFilter,
+ features);
// Make sure this builder doesn't reference ruleContext outside of analysis phase.
CppCompileActionTemplate actionTemplate = new CppCompileActionTemplate(
sourceArtifact,
@@ -1080,7 +1169,11 @@ public final class CppModel {
builder.getContext().getCppModuleMap(),
ImmutableMap.of());
semantics.finalizeCompileActionBuilder(
- ruleContext, builder, featureConfiguration.getFeatureSpecification());
+ ruleContext,
+ builder,
+ featureConfiguration.getFeatureSpecification(),
+ coptsFilter,
+ features);
CppCompileAction action = builder.buildOrThrowRuleError(ruleContext);
env.registerAction(action);
if (addObject) {
@@ -1421,7 +1514,11 @@ public final class CppModel {
builder.getContext().getCppModuleMap(),
ImmutableMap.of());
semantics.finalizeCompileActionBuilder(
- ruleContext, dBuilder, featureConfiguration.getFeatureSpecification());
+ ruleContext,
+ dBuilder,
+ featureConfiguration.getFeatureSpecification(),
+ coptsFilter,
+ features);
CppCompileAction dAction = dBuilder.buildOrThrowRuleError(ruleContext);
ruleContext.registerAction(dAction);
@@ -1439,7 +1536,11 @@ public final class CppModel {
builder.getContext().getCppModuleMap(),
ImmutableMap.of());
semantics.finalizeCompileActionBuilder(
- ruleContext, sdBuilder, featureConfiguration.getFeatureSpecification());
+ ruleContext,
+ sdBuilder,
+ featureConfiguration.getFeatureSpecification(),
+ coptsFilter,
+ features);
CppCompileAction sdAction = sdBuilder.buildOrThrowRuleError(ruleContext);
ruleContext.registerAction(sdAction);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java
index 06e4c9b687..b099854b14 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java
@@ -176,8 +176,8 @@ public class CppRuleClasses {
public static final SafeImplicitOutputsFunction CC_BINARY_DEBUG_PACKAGE =
fromTemplates("%{name}.dwp");
- /** A string constant for the copts feature. */
- public static final String COPTS = "copts";
+ /** Name of the feature that will be exempt from flag filtering when nocopts are used */
+ public static final String UNFILTERED_COMPILE_FLAGS_FEATURE_NAME = "unfiltered_compile_flags";
/**
* A string constant for the parse_headers feature.
@@ -289,6 +289,13 @@ public class CppRuleClasses {
public static final String NO_LEGACY_FEATURES = "no_legacy_features";
/**
+ * A string constant for the legacy_compile_flags feature. If this feature is present in the
+ * toolchain, and the toolchain doesn't specify no_legacy_features, bazel will move
+ * legacy_compile_flags before other features from {@link CppActionConfigs}.
+ */
+ public static final String LEGACY_COMPILE_FLAGS = "legacy_compile_flags";
+
+ /**
* A string constant for the feature that makes us build per-object debug info files.
*/
public static final String PER_OBJECT_DEBUG_INFO = "per_object_debug_info";
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppSemantics.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppSemantics.java
index dfdafb6ded..3331cb1e0f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppSemantics.java
@@ -14,6 +14,8 @@
package com.google.devtools.build.lib.rules.cpp;
+import com.google.common.base.Predicate;
+import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
@@ -40,7 +42,9 @@ public interface CppSemantics {
void finalizeCompileActionBuilder(
RuleContext ruleContext,
CppCompileActionBuilder actionBuilder,
- FeatureSpecification featureSpecification);
+ FeatureSpecification featureSpecification,
+ Predicate<String> coptsFilter,
+ ImmutableSet<String> features);
/**
* Called before {@link CppCompilationContext}s are finalized.
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 bf6b9fdc70..3ca810051b 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
@@ -56,7 +56,6 @@ public class FakeCppCompileAction extends CppCompileAction {
FakeCppCompileAction(
ActionOwner owner,
NestedSet<Artifact> allInputs,
- ImmutableList<String> features,
FeatureConfiguration featureConfiguration,
CcToolchainFeatures.Variables variables,
Artifact sourceFile,
@@ -82,7 +81,6 @@ public class FakeCppCompileAction extends CppCompileAction {
super(
owner,
allInputs,
- features,
featureConfiguration,
variables,
sourceFile,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCppSemantics.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCppSemantics.java
index 27f920b103..54aa46e80b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCppSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCppSemantics.java
@@ -18,6 +18,7 @@ import static com.google.devtools.build.lib.rules.objc.ObjcProvider.DYNAMIC_FRAM
import static com.google.devtools.build.lib.rules.objc.ObjcProvider.HEADER;
import static com.google.devtools.build.lib.rules.objc.ObjcProvider.STATIC_FRAMEWORK_FILE;
+import com.google.common.base.Predicate;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.Artifact;
@@ -97,13 +98,16 @@ public class ObjcCppSemantics implements CppSemantics {
public void finalizeCompileActionBuilder(
RuleContext ruleContext,
CppCompileActionBuilder actionBuilder,
- FeatureSpecification featureSpecification) {
+ FeatureSpecification featureSpecification,
+ Predicate<String> coptsFilter,
+ ImmutableSet<String> features) {
actionBuilder.setCppConfiguration(ruleContext.getFragment(CppConfiguration.class));
actionBuilder.setActionContext(CppCompileActionContext.class);
// Because Bazel does not support include scanning, we need the entire crosstool filegroup,
// including header files, as opposed to just the "compile" filegroup.
actionBuilder.addTransitiveMandatoryInputs(actionBuilder.getToolchain().getCrosstool());
actionBuilder.setShouldScanIncludes(false);
+ actionBuilder.setCoptsFilter(coptsFilter);
actionBuilder.addTransitiveMandatoryInputs(objcProvider.get(STATIC_FRAMEWORK_FILE));
actionBuilder.addTransitiveMandatoryInputs(objcProvider.get(DYNAMIC_FRAMEWORK_FILE));