starlark: move MethodLibrary.parseInt to StarlarkInt

Also: - simplify prefix logic, avoiding hash table. - fix bug that allowed two signs ("-0x-10"), and test. PiperOrigin-RevId: 339374232
philsc · Oct 28, 2020 · 9bdb4b1 · 9bdb4b1
1 parent 3ce5531
commit 9bdb4b1
Show file tree

Hide file tree

Showing 3 changed files with 90 additions and 86 deletions.
diff --git a/src/main/java/net/starlark/java/eval/MethodLibrary.java b/src/main/java/net/starlark/java/eval/MethodLibrary.java
@@ -15,17 +15,14 @@
 package net.starlark.java.eval;
 
 import com.google.common.base.Ascii;
-import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Ordering;
-import java.math.BigInteger;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Comparator;
 import java.util.Iterator;
 import java.util.List;
 import java.util.NoSuchElementException;
-import javax.annotation.Nullable;
 import net.starlark.java.annot.Param;
 import net.starlark.java.annot.ParamType;
 import net.starlark.java.annot.StarlarkBuiltin;
@@ -396,9 +393,6 @@ public StarlarkFloat floatForStarlark(Object x) throws EvalException {
     }
   }
 
-  private static final ImmutableMap<String, Integer> INT_PREFIXES =
-      ImmutableMap.of("0b", 2, "0o", 8, "0x", 16);
-
   @StarlarkMethod(
       name = "int",
       doc =
@@ -480,80 +474,6 @@ public StarlarkInt intForStarlark(Object x, Object baseO) throws EvalException {
     throw Starlark.errorf("got %s, want string, int, float, or bool", Starlark.type(x));
   }
 
-  // TODO(adonovan): move into StarlarkInt.parse in a follow-up.
-  static StarlarkInt parseInt(String string, int base) throws NumberFormatException {
-    String stringForErrors = string;
-
-    boolean isNegative = false;
-    if (string.isEmpty()) {
-      throw new NumberFormatException("empty string");
-    }
-    char c = string.charAt(0);
-    if (c == '+') {
-      string = string.substring(1);
-    } else if (c == '-') {
-      string = string.substring(1);
-      isNegative = true;
-    }
-
-    String prefix = getIntegerPrefix(string);
-    String digits;
-    if (prefix == null) {
-      // Nothing to strip. Infer base 10 if autodetection was requested (base == 0).
-      digits = string;
-      if (base == 0) {
-        if (string.length() > 1 && string.startsWith("0")) {
-          // We don't infer the base when input starts with '0' (due
-          // to confusion between octal and decimal).
-          throw new NumberFormatException(
-              "cannot infer base when string begins with a 0: " + Starlark.repr(stringForErrors));
-        }
-        base = 10;
-      }
-    } else {
-      // Strip prefix. Infer base from prefix if unknown (base == 0), or else verify its
-      // consistency.
-      digits = string.substring(prefix.length());
-      int expectedBase = INT_PREFIXES.get(prefix);
-      if (base == 0) {
-        base = expectedBase;
-      } else if (base != expectedBase) {
-        throw new NumberFormatException(
-            String.format(
-                "invalid base-%d literal: %s (%s prefix wants base %d)",
-                base, Starlark.repr(stringForErrors), prefix, expectedBase));
-      }
-    }
-
-    if (base < 2 || base > 36) {
-      throw new NumberFormatException(
-          String.format("invalid base %d (want 2 <= base <= 36)", base));
-    }
-    StarlarkInt result;
-    try {
-      result = StarlarkInt.of(Long.parseLong(digits, base));
-    } catch (NumberFormatException unused1) {
-      try {
-        result = StarlarkInt.of(new BigInteger(digits, base));
-      } catch (NumberFormatException unused2) {
-        throw new NumberFormatException(
-            String.format("invalid base-%d literal: %s", base, Starlark.repr(stringForErrors)));
-      }
-    }
-    return isNegative ? StarlarkInt.uminus(result) : result;
-  }
-
-  @Nullable
-  private static String getIntegerPrefix(String value) {
-    value = Ascii.toLowerCase(value);
-    for (String prefix : INT_PREFIXES.keySet()) {
-      if (value.startsWith(prefix)) {
-        return prefix;
-      }
-    }
-    return null;
-  }
-
   @StarlarkMethod(
       name = "dict",
       doc =

diff --git a/src/main/java/net/starlark/java/eval/StarlarkInt.java b/src/main/java/net/starlark/java/eval/StarlarkInt.java
@@ -81,10 +81,85 @@ static StarlarkInt ofFiniteDouble(double x) {
 
   /**
    * Returns the int denoted by a literal string in the specified base, as if by the Starlark
-   * expression {@code int(str, base)}.
+   * expression {@code int(s, base)}.
+   *
+   * @throws NumberFormatException if the input is invalid.
    */
-  public static StarlarkInt parse(String str, int base) throws NumberFormatException {
-    return MethodLibrary.parseInt(str, base);
+  public static StarlarkInt parse(String s, int base) {
+    String stringForErrors = s;
+
+    if (s.isEmpty()) {
+      throw new NumberFormatException("empty string");
+    }
+
+    // +/- prefix?
+    boolean isNegative = false;
+    char c = s.charAt(0);
+    if (c == '+') {
+      s = s.substring(1);
+    } else if (c == '-') {
+      s = s.substring(1);
+      isNegative = true;
+    }
+
+    String digits = s;
+
+    // 0b 0o 0x prefix?
+    if (s.length() > 1 && s.charAt(0) == '0') {
+      int prefixBase = 0;
+      c = s.charAt(1);
+      if (c == 'b' || c == 'B') {
+        prefixBase = 2;
+      } else if (c == 'o' || c == 'O') {
+        prefixBase = 8;
+      } else if (c == 'x' || c == 'X') {
+        prefixBase = 16;
+      }
+      if (prefixBase != 0) {
+        digits = s.substring(2); // strip prefix
+        if (base == 0) {
+          base = prefixBase;
+        } else if (base != prefixBase) {
+          throw new NumberFormatException(
+              String.format(
+                  "invalid base-%d literal: %s (%s prefix wants base %d)",
+                  base, Starlark.repr(stringForErrors), s.substring(0, 2), prefixBase));
+        }
+      }
+    }
+
+    // No prefix, no base? Use decimal.
+    if (digits == s && base == 0) {
+      // Don't infer base when input starts with '0' due to octal/decimal ambiguity.
+      if (s.length() > 1 && s.charAt(0) == '0') {
+        throw new NumberFormatException(
+            "cannot infer base when string begins with a 0: " + Starlark.repr(stringForErrors));
+      }
+      base = 10;
+    }
+    if (base < 2 || base > 36) {
+      throw new NumberFormatException(
+          String.format("invalid base %d (want 2 <= base <= 36)", base));
+    }
+
+    // Do not allow Long.parseLong and new BigInteger to accept another +/- sign.
+    if (digits.startsWith("+") || digits.startsWith("-")) {
+      throw new NumberFormatException(
+          String.format("invalid base-%d literal: %s", base, Starlark.repr(stringForErrors)));
+    }
+
+    StarlarkInt result;
+    try {
+      result = StarlarkInt.of(Long.parseLong(digits, base));
+    } catch (NumberFormatException unused1) {
+      try {
+        result = StarlarkInt.of(new BigInteger(digits, base));
+      } catch (NumberFormatException unused2) {
+        throw new NumberFormatException(
+            String.format("invalid base-%d literal: %s", base, Starlark.repr(stringForErrors)));
+      }
+    }
+    return isNegative ? StarlarkInt.uminus(result) : result;
   }
 
   // Subclass for values exactly representable in a Java int.
@@ -216,13 +291,17 @@ public boolean equals(Object that) {
   /** Returns this StarlarkInt as a string of decimal digits. */
   @Override
   public String toString() {
-    // TODO(adonovan): opt: avoid Number allocation
-    return toNumber().toString();
+    if (this instanceof Int32) {
+      return Integer.toString(((Int32) this).v);
+    } else if (this instanceof Int64) {
+      return Long.toString(((Int64) this).v);
+    } else {
+      return toBigInteger().toString();
+    }
   }
 
   @Override
   public void repr(Printer printer) {
-    // TODO(adonovan): opt: avoid Number and String allocations.
     printer.append(toString());
   }
 

diff --git a/src/test/java/net/starlark/java/eval/testdata/int_constructor.star b/src/test/java/net/starlark/java/eval/testdata/int_constructor.star
@@ -64,6 +64,7 @@ assert_eq(int('-11', 2), -3)
 assert_eq(int('016', 8), 14)
 assert_eq(int('016', 16), 22)
 assert_eq(int('0', 0), 0)
+assert_eq(int('0x0b10', 16), 0x0b10)
 int('0xFF', 8) ### invalid base-8 literal: "0xFF"
 ---
 int('016', 0) ### cannot infer base when string begins with a 0: "016"
@@ -103,3 +104,7 @@ int('0x') ### invalid base-10 literal: "0x"
 int('1.5') ### invalid base-10 literal: "1.5"
 ---
 int('ab') ### invalid base-10 literal: "ab"
+---
+int('--1') ### invalid base-10 literal: "--1"
+---
+int('-0x-10', 16) ### invalid base-16 literal: "-0x-10"