diff options
author | hlopko <hlopko@google.com> | 2017-07-04 04:31:11 -0400 |
---|---|---|
committer | John Cater <jcater@google.com> | 2017-07-05 10:57:30 -0400 |
commit | ec41b8cea579e135eca18f808a710abb397994f6 (patch) | |
tree | f187ce5dd8cf767fb058cfb7e4761dad100c7322 /src/main/java/com | |
parent | 6446ffa1ec61f8cfd73edc540ac54e42d15169f9 (diff) |
Remove implicit iteration from Crosstool
Up until now we allowed implicit iteration, e.g.:
flag_group { flag: '%{some_sequence_variable}' }
From now on, snippet above will raise an error. We require explicit
'iterate_over' message, e.g.:
flag_group {
iterate_over: 'some_sequence_variable'
flag: '%{some_sequence_variable}'
}
RELNOTES: Implicit iteration in the CROSSTOOL has been removed, use explicit 'iterate_over' message.
PiperOrigin-RevId: 160871888
Diffstat (limited to 'src/main/java/com')
3 files changed, 22 insertions, 96 deletions
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 1eee0802c4..db8774d118 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 @@ -360,7 +360,6 @@ public class CcToolchainFeatures implements Serializable { @Immutable private static class FlagGroup implements Serializable, Expandable { private final ImmutableList<Expandable> expandables; - private final ImmutableSet<String> usedVariables; private String iterateOverVariable; private final ImmutableSet<String> expandIfAllAvailable; private final ImmutableSet<String> expandIfNoneAvailable; @@ -368,15 +367,8 @@ public class CcToolchainFeatures implements Serializable { private final String expandIfFalse; private final VariableWithValue expandIfEqual; - /** - * TODO(b/32655571): Cleanup and get rid of usedVariables field once implicit iteration is not - * needed. - * - * @throws InvalidConfigurationException - */ private FlagGroup(CToolchain.FlagGroup flagGroup) throws InvalidConfigurationException { ImmutableList.Builder<Expandable> expandables = ImmutableList.builder(); - ImmutableSet.Builder<String> usedVariables = ImmutableSet.builder(); Collection<String> flags = flagGroup.getFlagList(); Collection<CToolchain.FlagGroup> groups = flagGroup.getFlagGroupList(); if (!flags.isEmpty() && !groups.isEmpty()) { @@ -388,18 +380,14 @@ public class CcToolchainFeatures implements Serializable { for (String flag : flags) { StringValueParser parser = new StringValueParser(flag); expandables.add(new Flag(parser.getChunks())); - usedVariables.addAll(parser.getUsedVariables()); } for (CToolchain.FlagGroup group : groups) { FlagGroup subgroup = new FlagGroup(group); expandables.add(subgroup); - usedVariables.addAll(subgroup.getUsedVariables()); } if (flagGroup.hasIterateOver()) { this.iterateOverVariable = flagGroup.getIterateOver(); - usedVariables.add(this.iterateOverVariable); } - this.usedVariables = usedVariables.build(); this.expandables = expandables.build(); this.expandIfAllAvailable = ImmutableSet.copyOf(flagGroup.getExpandIfAllAvailableList()); this.expandIfNoneAvailable = ImmutableSet.copyOf(flagGroup.getExpandIfNoneAvailableList()); @@ -419,10 +407,6 @@ public class CcToolchainFeatures implements Serializable { if (!canBeExpanded(variables)) { return; } - if (iterateOverVariable == null) { - // TODO(b/32655571): Remove branch once implicit iteration is not needed anymore. - iterateOverVariable = variables.guessIteratedOverVariable(usedVariables); - } if (iterateOverVariable != null) { for (VariableValue variableValue : variables.getSequenceVariable(iterateOverVariable)) { Variables nestedVariables = new Variables(variables, iterateOverVariable, variableValue); @@ -469,10 +453,6 @@ public class CcToolchainFeatures implements Serializable { return true; } - private Set<String> getUsedVariables() { - return usedVariables; - } - /** * Expands all flags in this group and adds them to {@code commandLine}. * @@ -870,9 +850,6 @@ public class CcToolchainFeatures implements Serializable { */ Iterable<? extends VariableValue> getSequenceValue(String variableName); - // TODO(b/32655571): Remove once implicit iteration is not needed - boolean isSequence(); - /** * Return value of the field, if the variable is of struct type or throw exception if it is * not or no such field exists. @@ -1084,11 +1061,6 @@ public class CcToolchainFeatures implements Serializable { } @Override - public boolean isSequence() { - return false; - } - - @Override public VariableValue getFieldValue(String variableName, String field) { Preconditions.checkNotNull(field); if (NAME_FIELD_NAME.equals(field) && !type.equals(Type.OBJECT_FILE_GROUP)) { @@ -1143,11 +1115,6 @@ public class CcToolchainFeatures implements Serializable { } @Override - public boolean isSequence() { - return true; - } - - @Override public VariableValue getFieldValue(String variableName, String field) { throw new ExpansionException( String.format( @@ -1196,11 +1163,6 @@ public class CcToolchainFeatures implements Serializable { } @Override - public boolean isSequence() { - return true; - } - - @Override public VariableValue getFieldValue(String variableName, String field) { throw new ExpansionException( String.format( @@ -1240,11 +1202,6 @@ public class CcToolchainFeatures implements Serializable { } @Override - public boolean isSequence() { - return true; - } - - @Override public VariableValue getFieldValue(String variableName, String field) { throw new ExpansionException( String.format( @@ -1309,11 +1266,6 @@ public class CcToolchainFeatures implements Serializable { } @Override - public boolean isSequence() { - return false; - } - - @Override public boolean isTruthy() { return !value.isEmpty(); } @@ -1357,11 +1309,6 @@ public class CcToolchainFeatures implements Serializable { } @Override - public boolean isSequence() { - return false; - } - - @Override public boolean isTruthy() { return !value.isEmpty(); } @@ -1405,11 +1352,6 @@ public class CcToolchainFeatures implements Serializable { } @Override - public boolean isSequence() { - return false; - } - - @Override public boolean isTruthy() { return value != 0; } @@ -1685,27 +1627,6 @@ public class CcToolchainFeatures implements Serializable { return getVariable(variableName).getSequenceValue(variableName); } - private String guessIteratedOverVariable(ImmutableSet<String> usedVariables) { - String sequenceName = null; - for (String usedVariable : usedVariables) { - VariableValue variableValue = lookupVariable(usedVariable, false); - if (variableValue != null && variableValue.isSequence()) { - if (sequenceName != null) { - throw new ExpansionException( - "Invalid toolchain configuration: trying to expand two variable list in one " - + "flag group: '" - + sequenceName - + "' and '" - + usedVariable - + "'"); - } else { - sequenceName = usedVariable; - } - } - } - return sequenceName; - } - /** Returns whether {@code variable} is set. */ boolean isAvailable(String variable) { return lookupVariable(variable, false) != null; 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 36dfc9eb93..5229ddf19f 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 @@ -18,15 +18,15 @@ import com.google.common.collect.ImmutableList; import java.util.Set; /** - * A helper class for creating action_configs for the c++ link action. + * A helper class for creating action_configs for the c++ actions. * * <p>TODO(b/30109612): Replace this with action_configs in the crosstool instead of putting it in * legacy features. */ public class CppActionConfigs { - /** A platform for linker invocations. */ - public static enum CppPlatform { + /** A platform for C++ tool invocations. */ + public enum CppPlatform { LINUX, MAC } @@ -44,15 +44,15 @@ public class CppActionConfigs { cppDynamicLibraryLinkerTool = "" + "feature {" - + " name: 'dynamic_library_linker_tool'" - + " flag_set {" - + " action: 'c++-link-dynamic-library'" - + " flag_group {" - + " flag: '" + + " name: 'dynamic_library_linker_tool'" + + " flag_set {" + + " action: 'c++-link-dynamic-library'" + + " flag_group {" + + " flag: '" + cppLinkDynamicLibraryToolPath + "'" - + " }" - + " }" + + " }" + + " }" + "}"; } @@ -240,6 +240,7 @@ public class CppActionConfigs { " action: 'c++-link-dynamic-library'", " expand_if_all_available: 'linkstamp_paths'", " flag_group {", + " iterate_over: 'linkstamp_paths'", " flag: '%{linkstamp_paths}'", " }", " }", @@ -328,7 +329,10 @@ 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'", 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 a8e6233d02..3e0c7446c1 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 @@ -773,6 +773,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment { + " action: 'c++-module-compile'" + " action: 'clif-match'" + " flag_group {" + + " iterate_over: 'preprocessor_defines'" + " flag: '-D%{preprocessor_defines}'" + " }" + " }" @@ -795,13 +796,16 @@ public class CppConfiguration extends BuildConfiguration.Fragment { + " action: 'objc-compile'" + " action: 'objc++-compile'" + " flag_group {" + + " iterate_over: 'quote_include_paths'" + " flag: '-iquote'" + " flag: '%{quote_include_paths}'" + " }" + " flag_group {" + + " iterate_over: 'include_paths'" + " flag: '-I%{include_paths}'" + " }" + " flag_group {" + + " iterate_over: 'system_include_paths'" + " flag: '-isystem'" + " flag: '%{system_include_paths}'" + " }" @@ -890,11 +894,8 @@ public class CppConfiguration extends BuildConfiguration.Fragment { String linkerFlags; if (useLLVMCoverageMap) { compileFlags = - "flag_group {" - + " flag: '-fprofile-instr-generate'" - + " flag: '-fcoverage-mapping'" - + "}"; - linkerFlags = " flag_group {" + " flag: '-fprofile-instr-generate'" + "}"; + "flag_group { flag: '-fprofile-instr-generate' flag: '-fcoverage-mapping' }"; + linkerFlags = "flag_group { flag: '-fprofile-instr-generate' }"; } else { compileFlags = " expand_if_all_available: 'gcov_gcno_file'" @@ -902,7 +903,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment { + " flag: '-fprofile-arcs'" + " flag: '-ftest-coverage'" + "}"; - linkerFlags = " flag_group {" + " flag: '-lgcov'" + "}"; + linkerFlags = " flag_group { flag: '-lgcov' }"; } TextFormat.merge( "" |