Remove the --incompatible_list_plus_equals_inplace flag

RELNOTES[INC]: The flag --incompatible_list_plus_equals_inplace is removed, its
default behavior is preserved. += on lists now always mutates the left hand
side.

PiperOrigin-RevId: 178359047

site/docs/skylark/backward-compatibility.md

@@ -32,7 +32,6 @@ guarded behind flags in the current release:
 
 *   [Set constructor](#set-constructor)
 *   [Keyword-only arguments](#keyword-only-arguments)
-*   [Mutating `+=`](#mutating)
 *   [Dictionary concatenation](#dictionary-concatenation)
 *   [Load must appear at top of file](#load-must-appear-at-top-of-file)
 *   [Load argument is a label](#load-argument-is-a-label)
@@ -85,29 +84,6 @@ documented).
 *   Default: `true`
 
 
-### Mutating `+=`
-
-We are changing `left += right` when `left` is a list. The old behavior is
-equivalent to `left = left + right`, which creates a new list and assigns it to
-`left`. The new behavior does not rebind `left`, but instead just mutates the
-list in-place.
-
-``` python
-def fct():
-  li = [1]
-  alias = li
-  li += [2]
-  # Old behavior: alias == [1]
-  # New behavior: alias == [1, 2]
-```
-
-This change makes Skylark more compatible with Python and avoids performance
-issues. The `+=` operator for tuples is unaffected.
-
-*   Flag: `--incompatible_list_plus_equals_inplace`
-*   Default: `true`
-
-
 ### Dictionary concatenation
 
 We are removing the `+` operator on dictionaries. This includes the `+=` form

src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsCodec.java

@@ -50,7 +50,6 @@ public void serialize(SkylarkSemantics semantics, CodedOutputStream codedOut)
     codedOut.writeBoolNoTag(semantics.incompatibleDisallowKeywordOnlyArgs());
     codedOut.writeBoolNoTag(semantics.incompatibleDisallowToplevelIfStatement());
     codedOut.writeBoolNoTag(semantics.incompatibleDisallowUncalledSetConstructor());
-    codedOut.writeBoolNoTag(semantics.incompatibleListPlusEqualsInplace());
     codedOut.writeBoolNoTag(semantics.incompatibleLoadArgumentIsLabel());
     codedOut.writeBoolNoTag(semantics.incompatibleNewActionsApi());
     codedOut.writeBoolNoTag(semantics.incompatibleShowAllPrintMessages());
@@ -75,7 +74,6 @@ public SkylarkSemantics deserialize(CodedInputStream codedIn)
     builder.incompatibleDisallowKeywordOnlyArgs(codedIn.readBool());
     builder.incompatibleDisallowToplevelIfStatement(codedIn.readBool());
     builder.incompatibleDisallowUncalledSetConstructor(codedIn.readBool());
-    builder.incompatibleListPlusEqualsInplace(codedIn.readBool());
     builder.incompatibleLoadArgumentIsLabel(codedIn.readBool());
     builder.incompatibleNewActionsApi(codedIn.readBool());
     builder.incompatibleShowAllPrintMessages(codedIn.readBool());

src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java

@@ -182,19 +182,6 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable
   )
   public boolean incompatibleDisallowUncalledSetConstructor;
 
-  @Option(
-    name = "incompatible_list_plus_equals_inplace",
-    defaultValue = "true",
-    category = "incompatible changes",
-    documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
-    effectTags = {OptionEffectTag.UNKNOWN},
-    metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
-    help =
-        "If set to true, `+=` on lists works like the `extend` method mutating the original "
-            + "list. Otherwise it copies the original list without mutating it."
-  )
-  public boolean incompatibleListPlusEqualsInplace;
-
   @Option(
     name = "incompatible_load_argument_is_label",
     defaultValue = "true",
@@ -284,7 +271,6 @@ public SkylarkSemantics toSkylarkSemantics() {
         .incompatibleDisallowKeywordOnlyArgs(incompatibleDisallowKeywordOnlyArgs)
         .incompatibleDisallowToplevelIfStatement(incompatibleDisallowToplevelIfStatement)
         .incompatibleDisallowUncalledSetConstructor(incompatibleDisallowUncalledSetConstructor)
-        .incompatibleListPlusEqualsInplace(incompatibleListPlusEqualsInplace)
         .incompatibleLoadArgumentIsLabel(incompatibleLoadArgumentIsLabel)
         .incompatibleNewActionsApi(incompatibleNewActionsApi)
         .incompatibleShowAllPrintMessages(incompatibleShowAllPrintMessages)

src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java

@@ -211,7 +211,7 @@ private static Object evaluate(
           return divide(lhs, rhs, location);
 
         case PERCENT:
-          return percent(lhs, rhs, env, location);
+          return percent(lhs, rhs, location);
 
         case EQUALS_EQUALS:
           return lhs.equals(rhs);
@@ -293,7 +293,7 @@ private static Object plus(
     }
 
     if ((lval instanceof MutableList) && (rval instanceof MutableList)) {
-      if (isAugmented && env.getSemantics().incompatibleListPlusEqualsInplace()) {
+      if (isAugmented) {
         @SuppressWarnings("unchecked")
         MutableList<Object> list = (MutableList) lval;
         list.addAll((MutableList<?>) rval, location, env.mutability());
@@ -420,7 +420,7 @@ private static Object divide(Object lval, Object rval, Location location) throws
   }
 
   /** Implements Operator.PERCENT. */
-  private static Object percent(Object lval, Object rval, Environment env, Location location)
+  private static Object percent(Object lval, Object rval, Location location)
       throws EvalException {
     // int % int
     if (lval instanceof Integer && rval instanceof Integer) {

src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java

@@ -51,7 +51,6 @@ public abstract class SkylarkSemantics {
   public abstract boolean incompatibleDisallowKeywordOnlyArgs();
   public abstract boolean incompatibleDisallowToplevelIfStatement();
   public abstract boolean incompatibleDisallowUncalledSetConstructor();
-  public abstract boolean incompatibleListPlusEqualsInplace();
   public abstract boolean incompatibleLoadArgumentIsLabel();
   public abstract boolean incompatibleNewActionsApi();
   public abstract boolean incompatibleShowAllPrintMessages();
@@ -84,7 +83,6 @@ public static Builder builderWithDefaults() {
           .incompatibleDisallowKeywordOnlyArgs(true)
           .incompatibleDisallowToplevelIfStatement(true)
           .incompatibleDisallowUncalledSetConstructor(true)
-          .incompatibleListPlusEqualsInplace(true)
           .incompatibleLoadArgumentIsLabel(true)
           .incompatibleNewActionsApi(false)
           .incompatibleShowAllPrintMessages(true)
@@ -110,7 +108,6 @@ public abstract static class Builder {
     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);
     public abstract Builder incompatibleShowAllPrintMessages(boolean value);

src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java

@@ -123,7 +123,6 @@ private static SkylarkSemanticsOptions buildRandomOptions(Random rand) throws Ex
         "--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_show_all_print_messages=" + rand.nextBoolean(),
@@ -149,7 +148,6 @@ private static SkylarkSemantics buildRandomSemantics(Random rand) {
         .incompatibleDisallowKeywordOnlyArgs(rand.nextBoolean())
         .incompatibleDisallowToplevelIfStatement(rand.nextBoolean())
         .incompatibleDisallowUncalledSetConstructor(rand.nextBoolean())
-        .incompatibleListPlusEqualsInplace(rand.nextBoolean())
         .incompatibleLoadArgumentIsLabel(rand.nextBoolean())
         .incompatibleNewActionsApi(rand.nextBoolean())
         .incompatibleShowAllPrintMessages(rand.nextBoolean())

src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java

@@ -1073,22 +1073,9 @@ public void testDotExpressionOnNonStructObject() throws Exception {
         .testIfExactError("object of type 'string' has no field 'field'", "x = 'a'.field");
   }
 
-  @Test
-  public void testPlusEqualsOnListCopying() throws Exception {
-    new SkylarkTest("--incompatible_list_plus_equals_inplace=false")
-        .setUp(
-            "def func():",
-            "  l1 = [1, 2]",
-            "  l2 = l1",
-            "  l2 += [3, 4]",
-            "  return l1, l2",
-            "lists = str(func())")
-        .testLookup("lists", "([1, 2], [1, 2, 3, 4])");
-  }
-
   @Test
   public void testPlusEqualsOnListMutating() throws Exception {
-    new SkylarkTest("--incompatible_list_plus_equals_inplace=true")
+    new SkylarkTest()
         .setUp(
             "def func():",
             "  l1 = [1, 2]",
@@ -1099,7 +1086,7 @@ public void testPlusEqualsOnListMutating() throws Exception {
         .testLookup("lists", "([1, 2, 3, 4], [1, 2, 3, 4])");
 
     // The same but with += after an IndexExpression
-    new SkylarkTest("--incompatible_list_plus_equals_inplace=true")
+    new SkylarkTest()
         .setUp(
             "def func():",
             "  l = [1, 2]",
@@ -1112,22 +1099,7 @@ public void testPlusEqualsOnListMutating() throws Exception {
 
   @Test
   public void testPlusEqualsOnTuple() throws Exception {
-    new SkylarkTest("--incompatible_list_plus_equals_inplace=false")
-        .setUp(
-            "def func():",
-            "  t1 = (1, 2)",
-            "  t2 = t1",
-            "  t2 += (3, 4)",
-            "  return t1, t2",
-            "tuples = func()")
-        .testLookup("tuples", SkylarkList.Tuple.of(
-            SkylarkList.Tuple.of(1, 2),
-            SkylarkList.Tuple.of(1, 2, 3, 4)
-        ));
-
-    // This behavior should remain the same regardless of the
-    // --incompatible_list_plus_equals_inplace flag
-    new SkylarkTest("--incompatible_list_plus_equals_inplace=true")
+    new SkylarkTest()
         .setUp(
             "def func():",
             "  t1 = (1, 2)",