Remove the deprecated set constructor from Skylark
The `set` constructor used to be deprecated, but it was still possible to use
it by providing --incompatible_disallow_set_constructor=false.
It's still allowed to have `set` in parts of the code that are not executed, this will be deprecated later. You can check if your code is compatible with this future change by using the flag --incompatible_disallow_uncalled_set_constructor (currently the default is "false").
RELNOTES[INC]: The flag --incompatible_disallow_set_constructor is no longer
available, the deprecated `set` constructor is not available anymore.
PiperOrigin-RevId: 176491641
diff --git a/site/docs/skylark/backward-compatibility.md b/site/docs/skylark/backward-compatibility.md
index 61eabd1..4a67bf4 100644
--- a/site/docs/skylark/backward-compatibility.md
+++ b/site/docs/skylark/backward-compatibility.md
@@ -30,6 +30,7 @@
The following are the backward incompatible changes that are implemented and
guarded behind flags in the current release:
+* [Set constructor](#set-constructor)
* [Keyword-only arguments](#keyword-only-arguments)
* [Mutating `+=`](#mutating)
* [Dictionary concatenation](#dictionary-concatenation)
@@ -43,6 +44,22 @@
* [New actions API](#new-actions-api)
* [Checked arithmetic](#checked-arithmetic)
+### Set constructor
+
+To maintain a clear distinction between the specialized [`depset`](depsets.md)
+data structure and Python's native `set` datatype (which does not currently
+exist in Skylark), the `set` constructor has been superseded by `depset`. It is
+no longer allowed to run code that calls the old `set` constructor.
+
+However, for a limited time, it will not be an error to reference the `set`
+constructor from code that is not executed (e.g. a function that is never
+called). Enable this flag to confirm that your code does not still refer to the
+old `set` constructor from unexecuted code.
+
+* Flag: `--incompatible_disallow_uncalled_set_constructor`
+* Default: `false`
+
+
### Keyword-only arguments
Keyword-only parameters are parameters that can be called only using their name.
diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsCodec.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsCodec.java
index 52db1ce..1ffb7e2 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsCodec.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsCodec.java
@@ -48,6 +48,7 @@
codedOut.writeBoolNoTag(semantics.incompatibleDisallowDictPlus());
codedOut.writeBoolNoTag(semantics.incompatibleDisallowKeywordOnlyArgs());
codedOut.writeBoolNoTag(semantics.incompatibleDisallowToplevelIfStatement());
+ codedOut.writeBoolNoTag(semantics.incompatibleDisallowUncalledSetConstructor());
codedOut.writeBoolNoTag(semantics.incompatibleListPlusEqualsInplace());
codedOut.writeBoolNoTag(semantics.incompatibleLoadArgumentIsLabel());
codedOut.writeBoolNoTag(semantics.incompatibleNewActionsApi());
@@ -70,6 +71,7 @@
builder.incompatibleDisallowDictPlus(codedIn.readBool());
builder.incompatibleDisallowKeywordOnlyArgs(codedIn.readBool());
builder.incompatibleDisallowToplevelIfStatement(codedIn.readBool());
+ builder.incompatibleDisallowUncalledSetConstructor(codedIn.readBool());
builder.incompatibleListPlusEqualsInplace(codedIn.readBool());
builder.incompatibleLoadArgumentIsLabel(codedIn.readBool());
builder.incompatibleNewActionsApi(codedIn.readBool());
diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java
index 0f8e0c4..6545d06 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java
@@ -159,6 +159,17 @@
public boolean incompatibleDisallowToplevelIfStatement;
@Option(
+ name = "incompatible_disallow_uncalled_set_constructor",
+ defaultValue = "false",
+ category = "incompatible changes",
+ documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+ effectTags = {OptionEffectTag.UNKNOWN},
+ metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
+ help = "If set to true, it's not allowed to use `set()` even if that code is never executed."
+ )
+ public boolean incompatibleDisallowUncalledSetConstructor;
+
+ @Option(
name = "incompatible_list_plus_equals_inplace",
defaultValue = "true",
category = "incompatible changes",
@@ -244,6 +255,7 @@
.incompatibleDisallowDictPlus(incompatibleDisallowDictPlus)
.incompatibleDisallowKeywordOnlyArgs(incompatibleDisallowKeywordOnlyArgs)
.incompatibleDisallowToplevelIfStatement(incompatibleDisallowToplevelIfStatement)
+ .incompatibleDisallowUncalledSetConstructor(incompatibleDisallowUncalledSetConstructor)
.incompatibleListPlusEqualsInplace(incompatibleListPlusEqualsInplace)
.incompatibleLoadArgumentIsLabel(incompatibleLoadArgumentIsLabel)
.incompatibleNewActionsApi(incompatibleNewActionsApi)
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java b/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java
index a60ec53..110c9e1 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java
@@ -175,36 +175,6 @@
}
@SkylarkSignature(
- name = "set",
- returnType = SkylarkNestedSet.class,
- doc =
- "A temporary alias for <a href=\"#depset\">depset</a>. "
- + "Deprecated in favor of <code>depset</code>.",
- parameters = {
- @Param(
- name = "items",
- type = Object.class,
- defaultValue = "[]",
- doc = "Same as for <a href=\"#depset\">depset</a>."
- ),
- @Param(
- name = "order",
- type = String.class,
- defaultValue = "\"default\"",
- doc = "Same as for <a href=\"#depset\">depset</a>."
- )
- },
- useLocation = true
- )
- private static final BuiltinFunction set =
- new BuiltinFunction("set") {
- public SkylarkNestedSet invoke(Object items, String order, Location loc)
- throws EvalException {
- throw new EvalException(loc, "The function 'set' has been removed in favor of 'depset'.");
- }
- };
-
- @SkylarkSignature(
name = "union",
objectType = SkylarkNestedSet.class,
returnType = SkylarkNestedSet.class,
@@ -285,7 +255,7 @@
};
private static Environment.Frame createGlobals() {
- List<BaseFunction> bazelGlobalFunctions = ImmutableList.of(select, depset, set, type);
+ List<BaseFunction> bazelGlobalFunctions = ImmutableList.of(select, depset, type);
try (Mutability mutability = Mutability.create("BUILD")) {
Environment env = Environment.builder(mutability)
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java b/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java
index a0dc773..1bbf78c 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java
@@ -94,6 +94,13 @@
if (name.equals("$error$")) {
return new EvalException(getLocation(), "contains syntax error(s)", true);
}
+ if (name.equals("set")) {
+ //TODO(vladmos): Remove as soon as the flag is removed
+ return new EvalException(getLocation(),
+ "The function 'set' has been removed in favor of 'depset', please use the latter. "
+ + "You can temporarily refer to the old 'set' constructor from unexecuted code "
+ + "by using --incompatible_disallow_uncalled_set_constructor=false");
+ }
String suggestion = SpellChecker.didYouMean(name, symbols);
return new EvalException(getLocation(), "name '" + name + "' is not defined" + suggestion);
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java
index 105768f..39a7673 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java
@@ -47,6 +47,7 @@
public abstract boolean incompatibleDisallowDictPlus();
public abstract boolean incompatibleDisallowKeywordOnlyArgs();
public abstract boolean incompatibleDisallowToplevelIfStatement();
+ public abstract boolean incompatibleDisallowUncalledSetConstructor();
public abstract boolean incompatibleListPlusEqualsInplace();
public abstract boolean incompatibleLoadArgumentIsLabel();
public abstract boolean incompatibleNewActionsApi();
@@ -68,6 +69,7 @@
.incompatibleDisallowDictPlus(false)
.incompatibleDisallowKeywordOnlyArgs(true)
.incompatibleDisallowToplevelIfStatement(true)
+ .incompatibleDisallowUncalledSetConstructor(false)
.incompatibleListPlusEqualsInplace(true)
.incompatibleLoadArgumentIsLabel(false)
.incompatibleNewActionsApi(false)
@@ -89,6 +91,7 @@
public abstract Builder incompatibleDisallowDictPlus(boolean value);
public abstract Builder incompatibleDisallowKeywordOnlyArgs(boolean value);
public abstract Builder incompatibleDisallowToplevelIfStatement(boolean value);
+ public abstract Builder incompatibleDisallowUncalledSetConstructor(boolean value);
public abstract Builder incompatibleListPlusEqualsInplace(boolean value);
public abstract Builder incompatibleLoadArgumentIsLabel(boolean value);
public abstract Builder incompatibleNewActionsApi(boolean value);
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java b/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java
index 451b0d5..cbe6094d 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java
@@ -68,6 +68,12 @@
block.variables.addAll(builtinVariables);
block.readOnlyVariables.addAll(builtinVariables);
semantics = env.getSemantics();
+
+ // If the flag is set to false, it should be allowed to have `set`
+ // in non-executable parts of the code.
+ if (!env.getSemantics().incompatibleDisallowUncalledSetConstructor()) {
+ block.variables.add("set");
+ }
}
@Override
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 f0f203e..2d388ca 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
@@ -113,13 +113,13 @@
"--incompatible_disallow_dict_plus=" + rand.nextBoolean(),
"--incompatible_disallow_keyword_only_args=" + rand.nextBoolean(),
"--incompatible_disallow_toplevel_if_statement=" + rand.nextBoolean(),
+ "--incompatible_disallow_uncalled_set_constructor=" + rand.nextBoolean(),
"--incompatible_list_plus_equals_inplace=" + rand.nextBoolean(),
"--incompatible_load_argument_is_label=" + rand.nextBoolean(),
"--incompatible_new_actions_api=" + rand.nextBoolean(),
"--incompatible_string_is_not_iterable=" + rand.nextBoolean(),
"--internal_do_not_export_builtins=" + rand.nextBoolean(),
- "--internal_skylark_flag_test_canary=" + rand.nextBoolean()
- );
+ "--internal_skylark_flag_test_canary=" + rand.nextBoolean());
}
/**
@@ -137,6 +137,7 @@
.incompatibleDisallowDictPlus(rand.nextBoolean())
.incompatibleDisallowKeywordOnlyArgs(rand.nextBoolean())
.incompatibleDisallowToplevelIfStatement(rand.nextBoolean())
+ .incompatibleDisallowUncalledSetConstructor(rand.nextBoolean())
.incompatibleListPlusEqualsInplace(rand.nextBoolean())
.incompatibleLoadArgumentIsLabel(rand.nextBoolean())
.incompatibleNewActionsApi(rand.nextBoolean())
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 c8fdba5..3ba8563 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
@@ -37,15 +37,27 @@
@Test
public void testLegacyConstructorNotCalled() throws Exception {
- env = newEnvironmentWithSkylarkOptions();
+ env =
+ newEnvironmentWithSkylarkOptions("--incompatible_disallow_uncalled_set_constructor=false");
eval("s = set([1, 2]) if False else depset([3, 4])");
SkylarkNestedSet s = get("s");
assertThat(s.getSet(Object.class)).containsExactly(3, 4);
+
+ // Static check are only enabled in .bzl files
+ new SkylarkTest("--incompatible_disallow_uncalled_set_constructor=true")
+ .testIfErrorContains("The function 'set' has been removed in favor of 'depset'",
+ "s = set([1, 2]) if False else depset([3, 4])");
+ new BuildTest("--incompatible_disallow_uncalled_set_constructor=true")
+ .testEval("s = set([1, 2]) if False else depset([3, 4]); s.to_list()", "[3, 4]");
}
@Test
public void testLegacyConstructorCalled() throws Exception {
- new BothModesTest()
+ new BothModesTest("--incompatible_disallow_uncalled_set_constructor=false")
+ .testIfErrorContains("The function 'set' has been removed in favor of 'depset'",
+ "s = set([1, 2])");
+
+ new BothModesTest("--incompatible_disallow_uncalled_set_constructor=true")
.testIfErrorContains("The function 'set' has been removed in favor of 'depset'",
"s = set([1, 2])");
}
