Remove the flag --incompatible_depset_is_not_iterable
https://github.com/bazelbuild/bazel/issues/5816
RELNOTES: None.
PiperOrigin-RevId: 280688754
diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
index 96fe21e..dc4d68b 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
@@ -225,21 +225,6 @@
public boolean incompatibleDepsetUnion;
@Option(
- name = "incompatible_depset_is_not_iterable",
- defaultValue = "true",
- documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
- effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS},
- metadataTags = {
- OptionMetadataTag.INCOMPATIBLE_CHANGE,
- OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
- },
- help =
- "If set to true, depset type is not iterable. For loops and functions expecting an "
- + "iterable will reject depset objects. Use the `.to_list` method to explicitly "
- + "convert to a list.")
- public boolean incompatibleDepsetIsNotIterable;
-
- @Option(
name = "incompatible_disable_target_provider_fields",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
@@ -640,7 +625,6 @@
.experimentalStarlarkUnusedInputsList(experimentalStarlarkUnusedInputsList)
.experimentalCcSharedLibrary(experimentalCcSharedLibrary)
.incompatibleBzlDisallowLoadAfterStatement(incompatibleBzlDisallowLoadAfterStatement)
- .incompatibleDepsetIsNotIterable(incompatibleDepsetIsNotIterable)
.incompatibleDepsetUnion(incompatibleDepsetUnion)
.incompatibleDisableTargetProviderFields(incompatibleDisableTargetProviderFields)
.incompatibleDisableThirdPartyLicenseChecking(
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java b/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java
index 363a3a9..c3b8a94 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java
@@ -324,17 +324,16 @@
} catch (ComparisonException e) {
throw new EvalException(loc, e);
}
- } else if (o instanceof SkylarkNestedSet) {
- return nestedSetToCollection((SkylarkNestedSet) o, loc, thread);
} else {
throw new EvalException(loc,
"type '" + getDataTypeName(o) + "' is not a collection");
}
}
+ // TODO(laurentlb): Get rid of this function.
private static Collection<?> nestedSetToCollection(
SkylarkNestedSet set, Location loc, @Nullable StarlarkThread thread) throws EvalException {
- if (thread != null && thread.getSemantics().incompatibleDepsetIsNotIterable()) {
+ if (thread != null) {
throw new EvalException(
loc,
"type 'depset' is not iterable. Use the `to_list()` method to get a list. Use "
@@ -356,9 +355,7 @@
public static Iterable<?> toIterable(Object o, Location loc, @Nullable StarlarkThread thread)
throws EvalException {
- if (o instanceof SkylarkNestedSet) {
- return nestedSetToCollection((SkylarkNestedSet) o, loc, thread);
- } else if (o instanceof Iterable) {
+ if (o instanceof Iterable) {
return (Iterable<?>) o;
} else if (o instanceof Map) {
return toCollection(o, loc, thread);
@@ -961,16 +958,7 @@
/** Implements 'x in y'. */
private static boolean in(Object x, Object y, StarlarkThread thread, Location location)
throws EvalException {
- if (thread.getSemantics().incompatibleDepsetIsNotIterable() && y instanceof SkylarkNestedSet) {
- throw new EvalException(
- location,
- "argument of type '"
- + getDataTypeName(y)
- + "' is not iterable. "
- + "in operator only works on lists, tuples, dicts and strings. "
- + "Use --incompatible_depset_is_not_iterable=false to temporarily disable "
- + "this check.");
- } else if (y instanceof SkylarkQueryable) {
+ if (y instanceof SkylarkQueryable) {
return ((SkylarkQueryable) y).containsKey(x, location, thread);
} else if (y instanceof String) {
if (x instanceof String) {
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java b/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
index 93acac3..9b8cbe9 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
@@ -348,16 +348,6 @@
return ((Map<?, ?>) x).size();
} else if (x instanceof Sequence) {
return ((Sequence<?>) x).size();
- } else if (x instanceof SkylarkNestedSet) {
- if (thread.getSemantics().incompatibleDepsetIsNotIterable()) {
- throw new EvalException(
- loc,
- EvalUtils.getDataTypeName(x)
- + " is not iterable. You may use `len(<depset>.to_list())` instead. Use "
- + "--incompatible_depset_is_not_iterable=false to temporarily disable this "
- + "check.");
- }
- return ((SkylarkNestedSet) x).toCollection().size();
} else if (x instanceof Iterable) {
// Iterables.size() checks if x is a Collection so it's efficient in that sense.
return Iterables.size((Iterable<?>) x);
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkNestedSet.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkNestedSet.java
index 8b05f4d..755a328 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkNestedSet.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkNestedSet.java
@@ -44,48 +44,49 @@
* <p>TODO(bazel-team): Decide whether this restriction is still useful.
*/
@SkylarkModule(
- name = "depset",
- category = SkylarkModuleCategory.BUILTIN,
- doc =
- "<p>A specialized data structure that supports efficient merge operations and has a defined "
- + "traversal order. Commonly used for accumulating data from transitive dependencies in "
- + "rules and aspects. For more information see <a href=\"../depsets.md\">here</a>."
- + "<p>"
- + "Depsets are not implemented as hash sets and do not support fast membership tests. If "
- + "you need a general set datatype, you can simulate one using a dictionary where all "
- + "keys map to <code>True</code>."
- + "<p>"
- + "Depsets are immutable. They should be created using their "
- + "<a href=\"globals.html#depset\">constructor function</a> and merged or augmented with "
- + "other depsets via the <code>transitive</code> argument. There are other deprecated "
- + "methods (<code>|</code> and <code>+</code> operators, <code>union</code> method) that "
- + "will eventually go away."
- + "<p>"
- + "The <code>order</code> parameter determines the kind of traversal that is done to "
- + "convert the depset to an iterable. There are four possible values:"
- + "<ul>"
- + "<li><code>\"default\"</code> (formerly <code>\"stable\"</code>): Order is unspecified "
- + "(but deterministic).</li>"
- + "<li><code>\"postorder\"</code> (formerly <code>\"compile\"</code>): A left-to-right "
- + "post-ordering. Precisely, this recursively traverses all children leftmost-first, "
- + "then the direct elements leftmost-first.</li>"
- + "<li><code>\"preorder\"</code> (formerly <code>\"naive_link\"</code>): A left-to-right "
- + "pre-ordering. Precisely, this traverses the direct elements leftmost-first, then "
- + "recursively traverses the children leftmost-first.</li>"
- + "<li><code>\"topological\"</code> (formerly <code>\"link\"</code>): A topological "
- + "ordering from the root down to the leaves. There is no left-to-right guarantee.</li>"
- + "</ul>"
- + "<p>"
- + "Two depsets may only be merged if either both depsets have the same order, or one of "
- + "them has <code>\"default\"</code> order. In the latter case the resulting depset's "
- + "order will be the same as the other's order."
- + "<p>"
- + "Depsets may contain duplicate values but these will be suppressed when iterating "
- + "(using <code>to_list()</code>). Duplicates may interfere with the ordering semantics."
-)
+ name = "depset",
+ category = SkylarkModuleCategory.BUILTIN,
+ doc =
+ "<p>A specialized data structure that supports efficient merge operations and has a"
+ + " defined traversal order. Commonly used for accumulating data from transitive"
+ + " dependencies in rules and aspects. For more information see <a"
+ + " href=\"../depsets.md\">here</a>."
+ + " <p>Depsets are not implemented as hash sets and do"
+ + " not support fast membership tests. If you need a general set datatype, you can"
+ + " simulate one using a dictionary where all keys map to <code>True</code>."
+ + "<p>Depsets are immutable. They should be created using their <a"
+ + " href=\"globals.html#depset\">constructor function</a> and merged or augmented with"
+ + " other depsets via the <code>transitive</code> argument. There are other deprecated"
+ + " methods (<code>|</code> and <code>+</code> operators, <code>union</code> method)"
+ + " that will eventually go away."
+ + "<p>The <code>order</code> parameter determines the"
+ + " kind of traversal that is done to convert the depset to an iterable. There are"
+ + " four possible values:"
+ + "<ul><li><code>\"default\"</code> (formerly"
+ + " <code>\"stable\"</code>): Order is unspecified (but"
+ + " deterministic).</li>"
+ + "<li><code>\"postorder\"</code> (formerly"
+ + " <code>\"compile\"</code>): A left-to-right post-ordering. Precisely, this"
+ + " recursively traverses all children leftmost-first, then the direct elements"
+ + " leftmost-first.</li>"
+ + "<li><code>\"preorder\"</code> (formerly"
+ + " <code>\"naive_link\"</code>): A left-to-right pre-ordering. Precisely, this"
+ + " traverses the direct elements leftmost-first, then recursively traverses the"
+ + " children leftmost-first.</li>"
+ + "<li><code>\"topological\"</code> (formerly"
+ + " <code>\"link\"</code>): A topological ordering from the root down to the leaves."
+ + " There is no left-to-right guarantee.</li>"
+ + "</ul>"
+ + "<p>Two depsets may only be merged if"
+ + " either both depsets have the same order, or one of them has"
+ + " <code>\"default\"</code> order. In the latter case the resulting depset's order"
+ + " will be the same as the other's order."
+ + "<p>Depsets may contain duplicate values but"
+ + " these will be suppressed when iterating (using <code>to_list()</code>). Duplicates"
+ + " may interfere with the ordering semantics.")
@Immutable
@AutoCodec
-public final class SkylarkNestedSet implements SkylarkValue, SkylarkQueryable {
+public final class SkylarkNestedSet implements SkylarkValue {
private final SkylarkType contentType;
private final NestedSet<?> set;
@Nullable private final ImmutableList<Object> items;
@@ -397,11 +398,6 @@
printer.append(")");
}
- @Override
- public final boolean containsKey(Object key, Location loc) throws EvalException {
- return set.toList().contains(key);
- }
-
@SkylarkCallable(
name = "union",
doc =
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
index 1554f3a..5dfed56 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
@@ -152,8 +152,6 @@
public abstract boolean incompatibleBzlDisallowLoadAfterStatement();
- public abstract boolean incompatibleDepsetIsNotIterable();
-
public abstract boolean incompatibleDepsetUnion();
public abstract boolean incompatibleDisableTargetProviderFields();
@@ -256,7 +254,6 @@
.experimentalStarlarkUnusedInputsList(true)
.experimentalCcSharedLibrary(false)
.incompatibleBzlDisallowLoadAfterStatement(true)
- .incompatibleDepsetIsNotIterable(true)
.incompatibleDepsetUnion(true)
.incompatibleDisableTargetProviderFields(false)
.incompatibleDisableThirdPartyLicenseChecking(true)
@@ -318,8 +315,6 @@
public abstract Builder incompatibleBzlDisallowLoadAfterStatement(boolean value);
- public abstract Builder incompatibleDepsetIsNotIterable(boolean value);
-
public abstract Builder incompatibleDepsetUnion(boolean value);
public abstract Builder incompatibleDisableTargetProviderFields(boolean value);
diff --git a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
index bfcca73..7a31a81 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
@@ -135,7 +135,6 @@
"--experimental_cc_shared_library=" + rand.nextBoolean(),
"--incompatible_bzl_disallow_load_after_statement=" + rand.nextBoolean(),
"--incompatible_depset_for_libraries_to_link_getter=" + rand.nextBoolean(),
- "--incompatible_depset_is_not_iterable=" + rand.nextBoolean(),
"--incompatible_depset_union=" + rand.nextBoolean(),
"--incompatible_disable_target_provider_fields=" + rand.nextBoolean(),
"--incompatible_disable_deprecated_attr_params=" + rand.nextBoolean(),
@@ -186,7 +185,6 @@
.experimentalCcSharedLibrary(rand.nextBoolean())
.incompatibleBzlDisallowLoadAfterStatement(rand.nextBoolean())
.incompatibleDepsetForLibrariesToLinkGetter(rand.nextBoolean())
- .incompatibleDepsetIsNotIterable(rand.nextBoolean())
.incompatibleDepsetUnion(rand.nextBoolean())
.incompatibleDisableTargetProviderFields(rand.nextBoolean())
.incompatibleDisableDeprecatedAttrParams(rand.nextBoolean())
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
index aad2c31..902e964 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
@@ -1481,14 +1481,6 @@
}
@Test
- public void testInSetDeprecated() throws Exception {
- new SkylarkTest("--incompatible_depset_is_not_iterable=false")
- .testExpression("'b' in depset(['a', 'b'])", Boolean.TRUE)
- .testExpression("'c' in depset(['a', 'b'])", Boolean.FALSE)
- .testExpression("1 in depset(['a', 'b'])", Boolean.FALSE);
- }
-
- @Test
public void testUnionSet() throws Exception {
new SkylarkTest("--incompatible_depset_union=false")
.testExpression("str(depset([1, 3]) | depset([1, 2]))", "depset([1, 2, 3])")
@@ -1498,29 +1490,17 @@
@Test
public void testSetIsNotIterable() throws Exception {
- new SkylarkTest("--incompatible_depset_is_not_iterable=true")
- .testIfErrorContains("not iterable", "list(depset(['a', 'b']))")
+ new SkylarkTest()
+ .testIfErrorContains("not a collection", "list(depset(['a', 'b']))")
.testIfErrorContains("not iterable", "max(depset([1, 2, 3]))")
.testIfErrorContains("not iterable", "1 in depset([1, 2, 3])")
- .testIfErrorContains("not iterable", "sorted(depset(['a', 'b']))")
- .testIfErrorContains("not iterable", "tuple(depset(['a', 'b']))")
+ .testIfErrorContains("not a collection", "sorted(depset(['a', 'b']))")
+ .testIfErrorContains("not a collection", "tuple(depset(['a', 'b']))")
.testIfErrorContains("not iterable", "[x for x in depset()]")
.testIfErrorContains("not iterable", "len(depset(['a']))");
}
@Test
- public void testSetIsIterable() throws Exception {
- new SkylarkTest("--incompatible_depset_is_not_iterable=false")
- .testExpression("str(list(depset(['a', 'b'])))", "[\"a\", \"b\"]")
- .testExpression("max(depset([1, 2, 3]))", 3)
- .testExpression("1 in depset([1, 2, 3])", true)
- .testExpression("str(sorted(depset(['b', 'a'])))", "[\"a\", \"b\"]")
- .testExpression("str(tuple(depset(['a', 'b'])))", "(\"a\", \"b\")")
- .testExpression("str([x for x in depset()])", "[]")
- .testExpression("len(depset(['a']))", 1);
- }
-
- @Test
public void testClassObjectCannotAccessNestedSet() throws Exception {
new SkylarkTest()
.update("mock", new MockClassObject())
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkNestedSetTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkNestedSetTest.java
index 3445388..f4abf4d 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkNestedSetTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkNestedSetTest.java
@@ -591,25 +591,19 @@
@Test
public void testDepthExceedsLimitDuringIteration() throws Exception {
NestedSet.setApplicationDepthLimit(2000);
- new SkylarkTest("--incompatible_depset_is_not_iterable=false")
+ new SkylarkTest()
.setUp(
"def create_depset(depth):",
" x = depset([0])",
" for i in range(1, depth):",
" x = depset([i], transitive = [x])",
- " for element in x:",
+ " for element in x.to_list():",
" str(x)",
" return None")
.testEval("create_depset(1000)", "None")
.testIfErrorContains("depset exceeded maximum depth 2000", "create_depset(3000)");
}
- @Test
- public void testListComprehensionsWithNestedSet() throws Exception {
- new SkylarkTest("--incompatible_depset_is_not_iterable=false")
- .testEval("[x + x for x in depset([1, 2, 3])]", "[2, 4, 6]");
- }
-
private interface MergeStrategy {
SkylarkNestedSet merge(SkylarkNestedSet[] sets) throws Exception;
}
