From 4eeae88ff723054b061b8a0a42a3a8dbc18aa57b Mon Sep 17 00:00:00 2001 From: laurentlb Date: Wed, 6 Nov 2019 10:14:58 -0800 Subject: [PATCH] Delete flag incompatible_disallow_hashing_frozen_mutables https://github.com/bazelbuild/bazel/issues/7800 RELNOTES: None. PiperOrigin-RevId: 278886996 --- .../lib/packages/StarlarkSemanticsOptions.java | 13 ------------- .../google/devtools/build/lib/syntax/EvalUtils.java | 10 +++------- .../build/lib/syntax/StarlarkSemantics.java | 5 ----- .../packages/SkylarkSemanticsConsistencyTest.java | 2 -- .../build/lib/skylark/SkylarkIntegrationTest.java | 13 ------------- 5 files changed, 3 insertions(+), 40 deletions(-) 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 890c9c1ed3f891..97f5ace13fb75e 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 @@ -343,18 +343,6 @@ public class StarlarkSemanticsOptions extends OptionsBase implements Serializabl help = "If set to true, the default value of the `allow_empty` argument of glob() is False.") public boolean incompatibleDisallowEmptyGlob; - @Option( - name = "incompatible_disallow_hashing_frozen_mutables", - 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, freezing a mutable object will not make it hashable.") - public boolean incompatibleDisallowHashingFrozenMutables; - @Option( name = "incompatible_disallow_legacy_javainfo", defaultValue = "true", @@ -741,7 +729,6 @@ public StarlarkSemantics toSkylarkSemantics() { .incompatibleRestrictStringEscapes(incompatibleRestrictStringEscapes) .incompatibleDisallowDictLookupUnhashableKeys( incompatibleDisallowDictLookupUnhashableKeys) - .incompatibleDisallowHashingFrozenMutables(incompatibleDisallowHashingFrozenMutables) .build(); return INTERNER.intern(semantics); } 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 370ccd616f2eff..676d0bee5569b8 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 @@ -123,11 +123,7 @@ public int compare(Object o1, Object o2) { */ public static void checkValidDictKey(Object o, StarlarkThread thread) throws EvalException { // TODO(bazel-team): check that all recursive elements are both Immutable AND Comparable. - if (thread != null && thread.getSemantics().incompatibleDisallowHashingFrozenMutables()) { - if (isHashable(o)) { - return; - } - } else if (isImmutable(o)) { + if (isHashable(o)) { return; } // Same error message as Python (that makes it a TypeError). @@ -149,11 +145,11 @@ public static boolean isHashable(Object o) { /** * Is this object known or assumed to be recursively immutable by Skylark? + * * @param o an Object * @return true if the object is known to be an immutable value. */ - // NB: This is used as the basis for accepting objects in SkylarkNestedSet-s, - // as well as for accepting objects as keys for Skylark dict-s. + // NB: This is used as the basis for accepting objects in SkylarkNestedSet-s. public static boolean isImmutable(Object o) { if (o instanceof SkylarkValue) { return ((SkylarkValue) o).isImmutable(); 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 ef5e224c88707d..e6acacafaa7d3b 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 @@ -220,8 +220,6 @@ public boolean flagValue(FlagIdentifier flagIdentifier) { public abstract boolean experimentalAllowTagsPropagation(); - public abstract boolean incompatibleDisallowHashingFrozenMutables(); - @Memoized @Override public abstract int hashCode(); @@ -300,7 +298,6 @@ public static Builder builderWithDefaults() { .incompatibleDepsetForLibrariesToLinkGetter(true) .incompatibleRestrictStringEscapes(false) .incompatibleDisallowDictLookupUnhashableKeys(false) - .incompatibleDisallowHashingFrozenMutables(true) .build(); /** Builder for {@link StarlarkSemantics}. All fields are mandatory. */ @@ -399,8 +396,6 @@ public abstract Builder incompatibleDisallowRuleExecutionPlatformConstraintsAllo public abstract Builder incompatibleDisallowDictLookupUnhashableKeys(boolean value); - public abstract Builder incompatibleDisallowHashingFrozenMutables(boolean value); - public abstract StarlarkSemantics build(); } } 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 98359fa0c7dfc2..8edf8905ba4e3a 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 @@ -165,7 +165,6 @@ private static StarlarkSemanticsOptions buildRandomOptions(Random rand) throws E "--incompatible_visibility_private_attributes_at_definition=" + rand.nextBoolean(), "--incompatible_restrict_string_escapes=" + rand.nextBoolean(), "--incompatible_disallow_dict_lookup_unhashable_keys=" + rand.nextBoolean(), - "--incompatible_disallow_hashing_frozen_mutables=" + rand.nextBoolean(), "--internal_skylark_flag_test_canary=" + rand.nextBoolean()); } @@ -221,7 +220,6 @@ private static StarlarkSemantics buildRandomSemantics(Random rand) { .incompatibleVisibilityPrivateAttributesAtDefinition(rand.nextBoolean()) .incompatibleRestrictStringEscapes(rand.nextBoolean()) .incompatibleDisallowDictLookupUnhashableKeys(rand.nextBoolean()) - .incompatibleDisallowHashingFrozenMutables(rand.nextBoolean()) .internalSkylarkFlagTestCanary(rand.nextBoolean()) .build(); } diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java index 4e0dbaf87e62ba..a09fc86c6b44f4 100644 --- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java @@ -3239,20 +3239,8 @@ public void testIdentifierAssignmentFromOuterScopeForbidden() throws Exception { assertContainsEvent("local variable 'a' is referenced before assignment"); } - @Test - public void testHashFrozenList() throws Exception { - setSkylarkSemanticsOptions("--incompatible_disallow_hashing_frozen_mutables=false"); - scratch.file("test/extension.bzl", "y = []"); - - scratch.file( - "test/BUILD", "load('//test:extension.bzl', 'y')", "{y: 1}", "cc_library(name = 'r')"); - - getConfiguredTarget("//test:r"); - } - @Test public void testHashFrozenListForbidden() throws Exception { - setSkylarkSemanticsOptions("--incompatible_disallow_hashing_frozen_mutables=true"); scratch.file("test/extension.bzl", "y = []"); scratch.file( @@ -3265,7 +3253,6 @@ public void testHashFrozenListForbidden() throws Exception { @Test public void testHashFrozenDeepMutableForbidden() throws Exception { - setSkylarkSemanticsOptions("--incompatible_disallow_hashing_frozen_mutables=true"); scratch.file("test/extension.bzl", "y = {}"); scratch.file(