Skylark: Slice operations now accept a step argument.

--
MOS_MIGRATED_REVID=110446625
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 3c272ac..3cf6113 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
@@ -33,6 +33,7 @@
 import com.google.devtools.build.lib.syntax.Type.ConversionException;
 
 import java.util.ArrayList;
+import java.util.Comparator;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.LinkedList;
@@ -921,57 +922,162 @@
   };
 
   // slice operator
-  @SkylarkSignature(name = "$slice", objectType = String.class,
-      documented = false,
-      mandatoryPositionals = {
-        @Param(name = "self", type = String.class, doc = "This string."),
-        @Param(name = "start", type = Integer.class, doc = "start position of the slice."),
-        @Param(name = "end", type = Integer.class, doc = "end position of the slice.")},
-      doc = "x[<code>start</code>:<code>end</code>] returns a slice or a list slice.")
-  private static BuiltinFunction stringSlice = new BuiltinFunction("$slice") {
-    public Object invoke(String self, Integer left, Integer right)
-        throws EvalException, ConversionException {
-      return pythonSubstring(self, left, right, "");
-    }
-  };
+  @SkylarkSignature(
+    name = "$slice",
+    objectType = String.class,
+    documented = false,
+    mandatoryPositionals = {
+      @Param(name = "self", type = String.class, doc = "This string."),
+      @Param(name = "start", type = Object.class, doc = "start position of the slice."),
+      @Param(name = "end", type = Object.class, doc = "end position of the slice.")
+    },
+    optionalPositionals = {
+      @Param(name = "step", type = Integer.class, defaultValue = "1", doc = "step value.")
+    },
+    doc =
+        "x[<code>start</code>:<code>end</code>:<code>step</code>] returns a slice or a list slice. "
+            + "Values may be negative and can be omitted.",
+    useLocation = true
+  )
+  private static BuiltinFunction stringSlice =
+      new BuiltinFunction("$slice") {
+        @SuppressWarnings("unused") // Accessed via Reflection.
+        public Object invoke(String self, Object start, Object end, Integer step, Location loc)
+            throws EvalException, ConversionException {
+          List<Integer> indices = getSliceIndices(start, end, step, self.length(), loc);
+          char[] result = new char[indices.size()];
+          char[] original = self.toCharArray();
+          int resultIndex = 0;
+          for (int originalIndex : indices) {
+            result[resultIndex] = original[originalIndex];
+            ++resultIndex;
+          }
+          return new String(result);
+        }
+      };
 
-  @SkylarkSignature(name = "$slice", objectType = MutableList.class, returnType = MutableList.class,
-      documented = false,
-      mandatoryPositionals = {
-        @Param(name = "self", type = MutableList.class, doc = "This list."),
-        @Param(name = "start", type = Integer.class, doc = "start position of the slice."),
-        @Param(name = "end", type = Integer.class, doc = "end position of the slice.")},
-      doc = "x[<code>start</code>:<code>end</code>] returns a slice or a list slice.",
-      useEnvironment = true)
-  private static BuiltinFunction mutableListSlice = new BuiltinFunction("$slice") {
-    public MutableList invoke(MutableList self, Integer left, Integer right,
-        Environment env) throws EvalException, ConversionException {
-      return new MutableList(sliceList(self.getList(), left, right), env);
-    }
-  };
+  @SkylarkSignature(
+    name = "$slice",
+    objectType = MutableList.class,
+    returnType = MutableList.class,
+    documented = false,
+    mandatoryPositionals = {
+      @Param(name = "self", type = MutableList.class, doc = "This list."),
+      @Param(name = "start", type = Object.class, doc = "start position of the slice."),
+      @Param(name = "end", type = Object.class, doc = "end position of the slice.")
+    },
+    optionalPositionals = {
+      @Param(name = "step", type = Integer.class, defaultValue = "1", doc = "step value.")
+    },
+    doc =
+        "x[<code>start</code>:<code>end</code>:<code>step</code>] returns a slice or a list slice."
+            + "Values may be negative and can be omitted.",
+    useLocation = true,
+    useEnvironment = true
+  )
+  private static BuiltinFunction mutableListSlice =
+      new BuiltinFunction("$slice") {
+        @SuppressWarnings("unused") // Accessed via Reflection.
+        public MutableList invoke(
+            MutableList self, Object start, Object end, Integer step, Location loc,
+            Environment env)
+            throws EvalException, ConversionException {
+          return new MutableList(sliceList(self.getList(), start, end, step, loc), env);
+        }
+      };
 
-  @SkylarkSignature(name = "$slice", objectType = Tuple.class, returnType = Tuple.class,
-      documented = false,
-      mandatoryPositionals = {
-        @Param(name = "self", type = Tuple.class, doc = "This tuple."),
-        @Param(name = "start", type = Integer.class, doc = "start position of the slice."),
-        @Param(name = "end", type = Integer.class, doc = "end position of the slice.")},
-      doc = "x[<code>start</code>:<code>end</code>] returns a slice or a list slice.",
-      useEnvironment = true)
-  private static BuiltinFunction tupleSlice = new BuiltinFunction("$slice") {
-      public Tuple invoke(Tuple self, Integer left, Integer right,
-          Environment env) throws EvalException, ConversionException {
-        return Tuple.copyOf(sliceList(self.getList(), left, right));
+  @SkylarkSignature(
+    name = "$slice",
+    objectType = Tuple.class,
+    returnType = Tuple.class,
+    documented = false,
+    mandatoryPositionals = {
+      @Param(name = "self", type = Tuple.class, doc = "This tuple."),
+      @Param(name = "start", type = Object.class, doc = "start position of the slice."),
+      @Param(name = "end", type = Object.class, doc = "end position of the slice.")
+    },
+    optionalPositionals = {
+      @Param(name = "step", type = Integer.class, defaultValue = "1", doc = "step value.")
+    },
+    doc =
+        "x[<code>start</code>:<code>end</code>:<code>step</code>] returns a slice or a list slice. "
+            + "Values may be negative and can be omitted.",
+    useLocation = true
+  )
+  private static BuiltinFunction tupleSlice =
+      new BuiltinFunction("$slice") {
+        @SuppressWarnings("unused") // Accessed via Reflection.
+        public Tuple invoke(Tuple self, Object start, Object end, Integer step, Location loc)
+            throws EvalException, ConversionException {
+          return Tuple.copyOf(sliceList(self.getList(), start, end, step, loc));
+        }
+      };
+
+  private static List<Object> sliceList(
+      List<Object> original, Object startObj, Object endObj, int step, Location loc)
+      throws EvalException {
+    int length = original.size();
+    ImmutableList.Builder<Object> slice = ImmutableList.builder();
+    for (int pos : getSliceIndices(startObj, endObj, step, length, loc)) {
+      slice.add(original.get(pos));
+    }
+    return slice.build();
+  }
+
+  /**
+   *  Calculates the indices of the elements that should be included in the slice [start:end:step]
+   * of a sequence with the given length.
+   */
+  private static List<Integer> getSliceIndices(
+      Object startObj, Object endObj, int step, int length, Location loc) throws EvalException {
+    if (step == 0) {
+      throw new EvalException(loc, "slice step cannot be zero");
+    }
+    int start = getIndex(startObj,
+        step,
+        /*positiveStepDefault=*/ 0,
+        /*negativeStepDefault=*/ length - 1,
+        /*length=*/ length,
+        loc);
+    int end = getIndex(endObj,
+        step,
+        /*positiveStepDefault=*/ length,
+        /*negativeStepDefault=*/ -1,
+        /*length=*/ length,
+        loc);
+    Comparator<Integer> comparator = getOrderingForStep(step);
+    ImmutableList.Builder<Integer> indices = ImmutableList.builder();
+    for (int current = start; comparator.compare(current, end) < 0; current += step) {
+      indices.add(current);
+    }
+    return indices.build();
+  }
+
+  /**
+   * Converts the given value into an integer index.
+   *
+   * <p>If the value is {@code None}, the return value of this methods depends on the sign of the
+   * slice step.
+   */
+  private static int getIndex(Object value, int step, int positiveStepDefault,
+      int negativeStepDefault, int length, Location loc) throws EvalException {
+    if (value == Runtime.NONE) {
+      return step < 0 ? negativeStepDefault : positiveStepDefault;
+    } else {
+      try {
+        return clampIndex(Type.INTEGER.cast(value), length);
+      } catch (ClassCastException ex) {
+        throw new EvalException(loc, String.format("'%s' is not a valid int", value));
       }
-    };
-
-  private static List<Object> sliceList(List<Object> list, Integer left, Integer right) {
-    left = clampIndex(left, list.size());
-    right = clampIndex(right, list.size());
-    if (left > right) {
-      left = right;
     }
-    return list.subList(left, right);
+  }
+
+  private static Ordering<Integer> getOrderingForStep(int step) {
+    Ordering<Integer> ordering = Ordering.<Integer>natural();
+    if (step < 0) {
+      ordering = ordering.reverse();
+    }
+    return ordering;
   }
 
   @SkylarkSignature(
@@ -1871,22 +1977,28 @@
   /**
    * Skylark String module.
    */
-  @SkylarkModule(name = "string", doc =
-      "A language built-in type to support strings. "
-      + "Examples of string literals:<br>"
-      + "<pre class=\"language-python\">a = 'abc\\ndef'\n"
-      + "b = \"ab'cd\"\n"
-      + "c = \"\"\"multiline string\"\"\"\n"
-      + "\n"
-      + "# Strings support slicing (negative index starts from the end):\n"
-      + "x = \"hello\"[2:4]  # \"ll\"\n"
-      + "y = \"hello\"[1:-1]  # \"ell\"\n"
-      + "z = \"hello\"[:4]  # \"hell\"</pre>"
-      + "Strings are iterable and support the <code>in</code> operator. Examples:<br>"
-      + "<pre class=\"language-python\">\"bc\" in \"abcd\"   # evaluates to True\n"
-      + "x = [s for s in \"abc\"]  # x == [\"a\", \"b\", \"c\"]</pre>\n"
-      + "Implicit concatenation of strings is not allowed; use the <code>+</code> "
-      + "operator instead.")
+  @SkylarkModule(
+    name = "string",
+    doc =
+        "A language built-in type to support strings. "
+            + "Examples of string literals:<br>"
+            + "<pre class=\"language-python\">a = 'abc\\ndef'\n"
+            + "b = \"ab'cd\"\n"
+            + "c = \"\"\"multiline string\"\"\"\n"
+            + "\n"
+            + "# Strings support slicing (negative index starts from the end):\n"
+            + "x = \"hello\"[2:4]  # \"ll\"\n"
+            + "y = \"hello\"[1:-1]  # \"ell\"\n"
+            + "z = \"hello\"[:4]  # \"hell\""
+            + "# Slice steps can be used, too:\n"
+            + "s = \"hello\"[::2] # \"hlo\"\n"
+            + "t = \"hello\"[3:0:-1] # \"lle\"\n</pre>"
+            + "Strings are iterable and support the <code>in</code> operator. Examples:<br>"
+            + "<pre class=\"language-python\">\"bc\" in \"abcd\"   # evaluates to True\n"
+            + "x = [s for s in \"abc\"]  # x == [\"a\", \"b\", \"c\"]</pre>\n"
+            + "Implicit concatenation of strings is not allowed; use the <code>+</code> "
+            + "operator instead."
+  )
   static final class StringModule {}
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
index 1214d8d..c5f612f 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
@@ -713,16 +713,15 @@
     return receiver;
   }
 
-  // substring_suffix ::= '[' expression? ':' expression? ']'
+  // substring_suffix ::= '[' expression? ':' expression?  ':' expression? ']'
   private Expression parseSubstringSuffix(int start, Expression receiver) {
     List<Argument.Passed> args = new ArrayList<>();
     Expression startExpr;
-    Expression endExpr;
 
     expect(TokenKind.LBRACKET);
     int loc1 = token.left;
     if (token.kind == TokenKind.COLON) {
-      startExpr = setLocation(new IntegerLiteral(0), token.left, token.right);
+      startExpr = setLocation(new Identifier("None"), token.left, token.right);
     } else {
       startExpr = parseExpression();
     }
@@ -734,20 +733,38 @@
                                    start, token.right);
     }
     // This is a slice (or substring)
-    expect(TokenKind.COLON);
-    int loc2 = token.left;
-    if (token.kind == TokenKind.RBRACKET) {
-      endExpr = setLocation(new IntegerLiteral(Integer.MAX_VALUE), token.left, token.right);
-    } else {
-      endExpr = parseNonTupleExpression();
-    }
+    args.add(parseSliceArgument(new Identifier("None")));
+    args.add(parseSliceArgument(new IntegerLiteral(1)));
     expect(TokenKind.RBRACKET);
-
-    args.add(setLocation(new Argument.Positional(endExpr), loc2, endExpr));
     return makeFuncallExpression(receiver, new Identifier("$slice"), args,
                                  start, token.right);
   }
 
+  /**
+   * Parses {@code [':' [expr]]} which can either be the end or the step argument of a slice
+   * operation. If no such expression is found, this method returns an argument that represents
+   * {@code defaultValue}.
+   */
+  private Argument.Positional parseSliceArgument(Expression defaultValue) {
+    Expression explicitArg = getSliceEndOrStepExpression();
+    Expression argValue =
+        (explicitArg == null) ? setLocation(defaultValue, token.left, token.right) : explicitArg;
+    return setLocation(new Argument.Positional(argValue), token.left, argValue);
+  }
+
+  private Expression getSliceEndOrStepExpression() {
+    // There has to be a colon before any end or slice argument.
+    // However, if the next token thereafter is another colon or a right bracket, no argument value
+    // was specified.
+    if (token.kind == TokenKind.COLON) {
+      expect(TokenKind.COLON);
+      if (token.kind != TokenKind.COLON && token.kind != TokenKind.RBRACKET) {
+        return parseNonTupleExpression();
+      }
+    }
+    return null;
+  }
+
   // Equivalent to 'exprlist' rule in Python grammar.
   // loop_variables ::= primary_with_suffix ( ',' primary_with_suffix )* ','?
   private Expression parseForLoopVariables() {
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkList.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkList.java
index 97cdd4e..1f88c4b 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkList.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkList.java
@@ -168,16 +168,23 @@
   /**
    * A class for mutable lists.
    */
-  @SkylarkModule(name = "list",
-      doc = "A language built-in type to support lists. Example of list literal:<br>"
-      + "<pre class=language-python>x = [1, 2, 3]</pre>"
-      + "Accessing elements is possible using indexing (starts from <code>0</code>):<br>"
-      + "<pre class=language-python>e = x[1]   # e == 2</pre>"
-      + "Lists support the <code>+</code> operator to concatenate two lists. Example:<br>"
-      + "<pre class=language-python>x = [1, 2] + [3, 4]   # x == [1, 2, 3, 4]\n"
-      + "x = [\"a\", \"b\"]\n"
-      + "x += [\"c\"]            # x == [\"a\", \"b\", \"c\"]</pre>"
-      + "Lists are mutable, as in Python.")
+  @SkylarkModule(
+    name = "list",
+    doc =
+        "A language built-in type to support lists. Example of list literal:<br>"
+            + "<pre class=language-python>x = [1, 2, 3]</pre>"
+            + "Accessing elements is possible using indexing (starts from <code>0</code>):<br>"
+            + "<pre class=language-python>e = x[1]   # e == 2</pre>"
+            + "Lists support the <code>+</code> operator to concatenate two lists. Example:<br>"
+            + "<pre class=language-python>x = [1, 2] + [3, 4]   # x == [1, 2, 3, 4]\n"
+            + "x = [\"a\", \"b\"]\n"
+            + "x += [\"c\"]            # x == [\"a\", \"b\", \"c\"]</pre>"
+            + "Similar to strings, lists support slice operations:"
+            + "<pre class=language-python>['a', 'b', 'c', 'd'][1:3]   # ['b', 'c']\n"
+            + "['a', 'b', 'c', 'd'][::2]  # ['a', 'c']\n"
+            + "['a', 'b', 'c', 'd'][3:0:-1]  # ['d', 'c', 'b']</pre>"
+            + "Lists are mutable, as in Python."
+  )
   public static final class MutableList extends SkylarkList implements Freezable {
 
     private final ArrayList<Object> contents = new ArrayList<>();
@@ -371,16 +378,23 @@
   /**
    * An immutable tuple, e.g. in (1, 2, 3)
    */
-  @SkylarkModule(name = "tuple",
-      doc = "A language built-in type to support tuples. Example of tuple literal:<br>"
-      + "<pre class=language-python>x = (1, 2, 3)</pre>"
-      + "Accessing elements is possible using indexing (starts from <code>0</code>):<br>"
-      + "<pre class=language-python>e = x[1]   # e == 2</pre>"
-      + "Lists support the <code>+</code> operator to concatenate two tuples. Example:<br>"
-      + "<pre class=language-python>x = (1, 2) + (3, 4)   # x == (1, 2, 3, 4)\n"
-      + "x = (\"a\", \"b\")\n"
-      + "x += (\"c\",)            # x == (\"a\", \"b\", \"c\")</pre>"
-      + "Tuples are immutable, therefore <code>x[1] = \"a\"</code> is not supported.")
+  @SkylarkModule(
+    name = "tuple",
+    doc =
+        "A language built-in type to support tuples. Example of tuple literal:<br>"
+            + "<pre class=language-python>x = (1, 2, 3)</pre>"
+            + "Accessing elements is possible using indexing (starts from <code>0</code>):<br>"
+            + "<pre class=language-python>e = x[1]   # e == 2</pre>"
+            + "Lists support the <code>+</code> operator to concatenate two tuples. Example:<br>"
+            + "<pre class=language-python>x = (1, 2) + (3, 4)   # x == (1, 2, 3, 4)\n"
+            + "x = (\"a\", \"b\")\n"
+            + "x += (\"c\",)            # x == (\"a\", \"b\", \"c\")</pre>"
+            + "Similar to lists, tuples support slice operations:"
+            + "<pre class=language-python>('a', 'b', 'c', 'd')[1:3]   # ('b', 'c')\n"
+            + "('a', 'b', 'c', 'd')[::2]  # ('a', 'c')\n"
+            + "('a', 'b', 'c', 'd')[3:0:-1]  # ('d', 'c', 'b')</pre>"
+            + "Tuples are immutable, therefore <code>x[1] = \"a\"</code> is not supported."
+  )
   @Immutable
   public static final class Tuple extends SkylarkList {
 
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java b/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java
index 772d875..acc3923 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java
@@ -453,7 +453,9 @@
 
   @Test
   public void testBoolean() throws Exception {
-    new BothModesTest().testStatement("False", Boolean.FALSE).testStatement("True", Boolean.TRUE);
+    new BothModesTest()
+        .testStatement("False", Boolean.FALSE)
+        .testStatement("True", Boolean.TRUE);
   }
 
   @Test
@@ -973,14 +975,98 @@
   }
 
   @Test
+  public void testEquivalenceOfReversedAndSlice() throws Exception {
+    String[] data = new String[] {"[]", "[1]", "[1, 2, 3]"};
+    for (String toBeReversed : data) {
+      new SkylarkTest().testEval(
+          String.format("reversed(%s)", toBeReversed), String.format("%s[::-1]", toBeReversed));
+    }
+  }
+
+  @Test
   public void testListSlice() throws Exception {
     new BothModesTest()
-        .testEval("[0,1,2,3][0:-1]", "[0, 1, 2]")
-        .testEval("[0,1,2,3,4,5][2:4]", "[2, 3]")
-        .testEval("[0,1,2,3,4,5][-2:-1]", "[4]")
+        .testEval("[0, 1, 2, 3][0:-1]", "[0, 1, 2]")
+        .testEval("[0, 1, 2, 3, 4, 5][2:4]", "[2, 3]")
+        .testEval("[0, 1, 2, 3, 4, 5][-2:-1]", "[4]")
         .testEval("[][1:2]", "[]")
-        .testEval("[1,2,3][1:0]", "[]")
-        .testEval("[0,1,2,3][-10:10]", "[0, 1, 2, 3]");
+        .testEval("[0, 1, 2, 3][-10:10]", "[0, 1, 2, 3]");
+  }
+
+  @Test
+  public void testListSlice_WrongType() throws Exception {
+    new BothModesTest()
+        .testIfExactError("'a' is not a valid int", "'123'['a'::]")
+        .testIfExactError("'b' is not a valid int", "'123'[:'b':]");
+  }
+
+  @Test
+  public void testListSliceStep() throws Exception {
+    new BothModesTest()
+        .testEval("[1, 2, 3, 4, 5][::1]", "[1, 2, 3, 4, 5]")
+        .testEval("[1, 2, 3, 4, 5][1::1]", "[2, 3, 4, 5]")
+        .testEval("[1, 2, 3, 4, 5][:2:1]", "[1, 2]")
+        .testEval("[1, 2, 3, 4, 5][1:3:1]", "[2, 3]")
+        .testEval("[1, 2, 3, 4, 5][-4:-2:1]", "[2, 3]")
+        .testEval("[1, 2, 3, 4, 5][-10:10:1]", "[1, 2, 3, 4, 5]")
+        .testEval("[1, 2, 3, 4, 5][::42]", "[1]");
+  }
+
+  @Test
+  public void testListSliceStep_EmptyList() throws Exception {
+    new BothModesTest().testEval("[][::1]", "[]").testEval("[][::-1]", "[]");
+  }
+
+  @Test
+  public void testListSliceStep_SkipValues() throws Exception {
+    new BothModesTest()
+        .testEval("[1, 2, 3, 4, 5, 6, 7][::3]", "[1, 4, 7]")
+        .testEval("[1, 2, 3, 4, 5, 6, 7, 8, 9][1:7:3]", "[2, 5]");
+  }
+
+  @Test
+  public void testListSliceStep_Negative() throws Exception {
+    new BothModesTest()
+        .testEval("[1, 2, 3, 4, 5][::-1]", "[5, 4, 3, 2, 1]")
+        .testEval("[1, 2, 3, 4, 5][4::-1]", "[5, 4, 3, 2, 1]")
+        .testEval("[1, 2, 3, 4, 5][:0:-1]", "[5, 4, 3, 2]")
+        .testEval("[1, 2, 3, 4, 5][3:1:-1]", "[4, 3]")
+        .testEval("[1, 2, 3, 4, 5][::-2]", "[5, 3, 1]")
+        .testEval("[1, 2, 3, 4, 5][::-10]", "[5]");
+  }
+
+  @Test
+  public void testListSlice_WrongOrder() throws Exception {
+    new BothModesTest().testEval("[1, 2, 3][3:1:1]", "[]").testEval("[1, 2, 3][1:3:-1]", "[]");
+  }
+
+  @Test
+  public void testListSliceStep_InvalidStep() throws Exception {
+    String msg = "slice step cannot be zero";
+    new BothModesTest()
+        .testIfExactError(msg, "[1, 2, 3][::0]")
+        .testIfExactError(msg, "[1, 2, 3][1::0]")
+        .testIfExactError(msg, "[1, 2, 3][:3:0]")
+        .testIfExactError(msg, "[1, 2, 3][1:3:0]");
+  }
+
+  @Test
+  public void testTupleSlice() throws Exception {
+    // Not as comprehensive as the tests for slicing lists since the underlying mechanism is the
+    // same.
+    new BothModesTest()
+        .testEval("()[1:2]", "()")
+        .testEval("()[::1]", "()")
+        .testEval("(0, 1, 2, 3)[0:-1]", "(0, 1, 2)")
+        .testEval("(0, 1, 2, 3, 4, 5)[2:4]", "(2, 3)")
+        .testEval("(0, 1, 2, 3)[-10:10]", "(0, 1, 2, 3)")
+        .testEval("(1, 2, 3, 4, 5)[-10:10:1]", "(1, 2, 3, 4, 5)")
+        .testEval("(1, 2, 3, 4, 5, 6, 7, 8, 9)[1:7:3]", "(2, 5)")
+        .testEval("(1, 2, 3, 4, 5)[::-1]", "(5, 4, 3, 2, 1)")
+        .testEval("(1, 2, 3, 4, 5)[3:1:-1]", "(4, 3)")
+        .testEval("(1, 2, 3, 4, 5)[::-2]", "(5, 3, 1)")
+        .testEval("(1, 2, 3, 4, 5)[::-10]", "(5,)")
+        .testIfExactError("slice step cannot be zero", "(1, 2, 3)[1::0]");
   }
 
   @Test
@@ -1065,6 +1151,78 @@
   }
 
   @Test
+  public void testStringSlice() throws Exception {
+    new BothModesTest()
+        .testStatement("'0123'[0:-1]", "012")
+        .testStatement("'012345'[2:4]", "23")
+        .testStatement("'012345'[-2:-1]", "4")
+        .testStatement("''[1:2]", "")
+        .testStatement("'012'[1:0]", "")
+        .testStatement("'0123'[-10:10]", "0123");
+  }
+
+  @Test
+  public void testStringSlice_WrongType() throws Exception {
+    new BothModesTest()
+        .testIfExactError("'a' is not a valid int", "'123'['a'::]")
+        .testIfExactError("'b' is not a valid int", "'123'[:'b':]");
+  }
+
+  @Test
+  public void testStringSliceStep() throws Exception {
+    new BothModesTest()
+        .testStatement("'01234'[::1]", "01234")
+        .testStatement("'01234'[1::1]", "1234")
+        .testStatement("'01234'[:2:1]", "01")
+        .testStatement("'01234'[1:3:1]", "12")
+        .testStatement("'01234'[-4:-2:1]", "12")
+        .testStatement("'01234'[-10:10:1]", "01234")
+        .testStatement("'01234'[::42]", "0");
+  }
+
+  @Test
+  public void testStringSliceStep_EmptyString() throws Exception {
+    new BothModesTest()
+        .testStatement("''[::1]", "")
+        .testStatement("''[::-1]", "");
+  }
+
+  @Test
+  public void testStringSliceStep_SkipValues() throws Exception {
+    new BothModesTest()
+        .testStatement("'0123456'[::3]", "036")
+        .testStatement("'01234567'[1:7:3]", "14");
+  }
+
+  @Test
+  public void testStringSliceStep_Negative() throws Exception {
+    new BothModesTest()
+        .testStatement("'01234'[::-1]", "43210")
+        .testStatement("'01234'[4::-1]", "43210")
+        .testStatement("'01234'[:0:-1]", "4321")
+        .testStatement("'01234'[3:1:-1]", "32")
+        .testStatement("'01234'[::-2]", "420")
+        .testStatement("'01234'[::-10]", "4");
+  }
+
+  @Test
+  public void testStringSliceStep_WrongOrder() throws Exception {
+    new BothModesTest()
+        .testStatement("'123'[3:1:1]", "")
+        .testStatement("'123'[1:3:-1]", "");
+  }
+
+  @Test
+  public void testStringSliceStep_InvalidStep() throws Exception {
+    String msg = "slice step cannot be zero";
+    new BothModesTest()
+        .testIfExactError(msg, "'123'[::0]")
+        .testIfExactError(msg, "'123'[1::0]")
+        .testIfExactError(msg, "'123'[:3:0]")
+        .testIfExactError(msg, "'123'[1:3:0]");
+  }
+
+  @Test
   public void testDictionaryCreation() throws Exception {
     String expected = "{'a': 1, 'b': 2, 'c': 3}";
 
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java b/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
index fac46d4..de81e4e 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
@@ -23,6 +23,7 @@
 import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.syntax.Argument.Passed;
 import com.google.devtools.build.lib.syntax.DictionaryLiteral.DictionaryEntryLiteral;
 import com.google.devtools.build.lib.syntax.util.EvaluationTestCase;
 
@@ -257,7 +258,7 @@
   public void testSubstring() throws Exception {
     FuncallExpression e = (FuncallExpression) parseExpression("'FOO.CC'[:].lower()[1:]");
     assertEquals("$slice", e.getFunction().getName());
-    assertThat(e.getArguments()).hasSize(2);
+    assertThat(e.getArguments()).hasSize(3);
 
     e = (FuncallExpression) parseExpression("'FOO.CC'.lower()[1:].startswith('oo')");
     assertEquals("startswith", e.getFunction().getName());
@@ -265,7 +266,48 @@
 
     e = (FuncallExpression) parseExpression("'FOO.CC'[1:][:2]");
     assertEquals("$slice", e.getFunction().getName());
-    assertThat(e.getArguments()).hasSize(2);
+    assertThat(e.getArguments()).hasSize(3);
+  }
+
+  @Test
+  public void testSlice() throws Exception {
+    evalSlice("'0123'[:]", Runtime.NONE, Runtime.NONE, 1);
+    evalSlice("'0123'[1:]", 1, Runtime.NONE, 1);
+    evalSlice("'0123'[:3]", Runtime.NONE, 3, 1);
+    evalSlice("'0123'[::]", Runtime.NONE, Runtime.NONE, 1);
+    evalSlice("'0123'[1::]", 1, Runtime.NONE, 1);
+    evalSlice("'0123'[:3:]", Runtime.NONE, 3, 1);
+    evalSlice("'0123'[::-1]", Runtime.NONE, Runtime.NONE, -1);
+    evalSlice("'0123'[1:3:]", 1, 3, 1);
+    evalSlice("'0123'[1::-1]", 1, Runtime.NONE, -1);
+    evalSlice("'0123'[:3:-1]", Runtime.NONE, 3, -1);
+    evalSlice("'0123'[1:3:-1]", 1, 3, -1);
+  }
+
+  private void evalSlice(String statement, Object... expectedArgs) {
+    FuncallExpression e = (FuncallExpression) parseExpression(statement);
+    assertEquals("$slice", e.getFunction().getName());
+    List<Passed> actualArgs = e.getArguments();
+    assertThat(actualArgs).hasSize(expectedArgs.length);
+    int pos = 0;
+    for (Passed arg : actualArgs) {
+      // There is no way to evaluate the expression here, so we rely on string comparison.
+      String actualString = arg.getValue().toString();
+      String expectedString = printSliceArg(expectedArgs[pos]);
+      assertThat(actualString).isEqualTo(expectedString);
+      ++pos;
+    }
+  }
+
+  private String printSliceArg(Object arg) {
+    // The parser sees negative integer constants as FuncallExpressions instead of negative
+    // IntegerLiterals.
+    // Consequently, the string representation of -1 is "-(1)", not "-1".
+    if (arg instanceof Integer) {
+      int value = (int) arg;
+      return value < 0 ? String.format("-(%d)", -value) : String.valueOf(value);
+    }
+    return arg.toString();
   }
 
   private void assertLocation(int start, int end, Location location)