Skip to content

Commit

Permalink
Internally, use PatternCompiler from Doubles.tryParse.
Browse files Browse the repository at this point in the history 
(Redo of CL 202132002, which was partially rolled back in CL 202139691.)

This time including tests to cover the RE2J approach.
To avoid the errors of last time, I had to remove the possessive quantifiers under RE2J and replace \p{XDigit} with [0-9a-fA-F] (which is equivalent; see the Pattern Javadoc).

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=204301262
  • Loading branch information
@cpovirk @ronshapiro
cpovirk authored and ronshapiro committed Jul 13, 2018
1 parent 7975a2f commit 08adcec
Show file tree
Hide file tree
Showing 11 changed files with 88 additions and 28 deletions.
4 changes: 2 additions & 2 deletions android/guava-tests/test/com/google/common/base/SplitterTest.java
View file
Expand Up @@ -354,7 +354,7 @@ public void testPatternSplitWithDoubleDelimiterOmitEmptyStrings() {
@GwtIncompatible // java.util.regex.Pattern
@AndroidIncompatible // Bug in older versions of Android we test against, since fixed.
public void testPatternSplitLookBehind() {
if (!Platform.usingJdkPatternCompiler()) {
if (!CommonPattern.isPcreLike()) {
return;
}
String toSplit = ":foo::barbaz:";
Expand Down Expand Up @@ -491,7 +491,7 @@ public void testSplitterIterableIsLazy_string() {
@GwtIncompatible // java.util.regex.Pattern
@AndroidIncompatible // not clear that j.u.r.Matcher promises to handle mutations during use
public void testSplitterIterableIsLazy_pattern() {
if (!Platform.usingJdkPatternCompiler()) {
if (!CommonPattern.isPcreLike()) {
return;
}
assertSplitterIterableIsLazy(Splitter.onPattern(","));
Expand Down
6 changes: 5 additions & 1 deletion android/guava/src/com/google/common/base/CommonPattern.java
View file
Expand Up @@ -33,7 +33,11 @@ abstract class CommonPattern {
@Override
public abstract String toString();

static CommonPattern compile(String pattern) {
public static CommonPattern compile(String pattern) {
return Platform.compilePattern(pattern);
}

public static boolean isPcreLike() {
return Platform.patternCompilerIsPcreLike();
}
}
6 changes: 6 additions & 0 deletions android/guava/src/com/google/common/base/PatternCompiler.java
View file
Expand Up @@ -29,4 +29,10 @@ interface PatternCompiler {
* @throws IllegalArgumentException if the pattern is invalid
*/
CommonPattern compile(String pattern);

/**
* Returns {@code true} if the regex implementation behaves like Perl -- notably, by supporting
* possessive quantifiers but also being susceptible to catastrophic backtracking.
*/
boolean isPcreLike();
}
9 changes: 7 additions & 2 deletions android/guava/src/com/google/common/base/Platform.java
View file
Expand Up @@ -70,8 +70,8 @@ static CommonPattern compilePattern(String pattern) {
return patternCompiler.compile(pattern);
}

static boolean usingJdkPatternCompiler() {
return patternCompiler instanceof JdkPatternCompiler;
static boolean patternCompilerIsPcreLike() {
return patternCompiler.isPcreLike();
}

private static PatternCompiler loadPatternCompiler() {
Expand All @@ -92,5 +92,10 @@ private static final class JdkPatternCompiler implements PatternCompiler {
public CommonPattern compile(String pattern) {
return new JdkPattern(Pattern.compile(pattern));
}

@Override
public boolean isPcreLike() {
return true;
}
}
}
31 changes: 23 additions & 8 deletions android/guava/src/com/google/common/primitives/Doubles.java
View file
Expand Up @@ -33,7 +33,6 @@
import java.util.Comparator;
import java.util.List;
import java.util.RandomAccess;
import java.util.regex.Pattern;
import org.checkerframework.checker.nullness.compatqual.NullableDecl;

/**
Expand Down Expand Up @@ -646,16 +645,32 @@ public String toString() {
* that pass this regex are valid -- only a performance hit is incurred, not a semantics bug.
*/
@GwtIncompatible // regular expressions
static final Pattern FLOATING_POINT_PATTERN = fpPattern();
static final
java.util.regex.Pattern
FLOATING_POINT_PATTERN = fpPattern();

@GwtIncompatible // regular expressions
private static Pattern fpPattern() {
String decimal = "(?:\\d++(?:\\.\\d*+)?|\\.\\d++)";
String completeDec = decimal + "(?:[eE][+-]?\\d++)?[fFdD]?";
String hex = "(?:\\p{XDigit}++(?:\\.\\p{XDigit}*+)?|\\.\\p{XDigit}++)";
String completeHex = "0[xX]" + hex + "[pP][+-]?\\d++[fFdD]?";
private static
java.util.regex.Pattern
fpPattern() {
/*
* We use # instead of * for possessive quantifiers. This lets us strip them out when building
* the regex for RE2 (which doesn't support them) but leave them in when building it for
* java.util.regex (where we want them in order to avoid catastrophic backtracking).
*/
String decimal = "(?:\\d+#(?:\\.\\d*#)?|\\.\\d+#)";
String completeDec = decimal + "(?:[eE][+-]?\\d+#)?[fFdD]?";
String hex = "(?:[0-9a-fA-F]+#(?:\\.[0-9a-fA-F]*#)?|\\.[0-9a-fA-F]+#)";
String completeHex = "0[xX]" + hex + "[pP][+-]?\\d+#[fFdD]?";
String fpPattern = "[+-]?(?:NaN|Infinity|" + completeDec + "|" + completeHex + ")";
return Pattern.compile(fpPattern);
fpPattern =
fpPattern.replace(
"#",
"+"
);
return
java.util.regex.Pattern
.compile(fpPattern);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions guava-gwt/src-super/com/google/common/base/super/com/google/common/base/Platform.java
View file
Expand Up @@ -75,8 +75,8 @@ static CommonPattern compilePattern(String pattern) {
throw new UnsupportedOperationException();
}

static boolean usingJdkPatternCompiler() {
return false;
static boolean patternCompilerIsPcreLike() {
throw new UnsupportedOperationException();
}

private Platform() {}
Expand Down
4 changes: 2 additions & 2 deletions guava-tests/test/com/google/common/base/SplitterTest.java
View file
Expand Up @@ -354,7 +354,7 @@ public void testPatternSplitWithDoubleDelimiterOmitEmptyStrings() {
@GwtIncompatible // java.util.regex.Pattern
@AndroidIncompatible // Bug in older versions of Android we test against, since fixed.
public void testPatternSplitLookBehind() {
if (!Platform.usingJdkPatternCompiler()) {
if (!CommonPattern.isPcreLike()) {
return;
}
String toSplit = ":foo::barbaz:";
Expand Down Expand Up @@ -491,7 +491,7 @@ public void testSplitterIterableIsLazy_string() {
@GwtIncompatible // java.util.regex.Pattern
@AndroidIncompatible // not clear that j.u.r.Matcher promises to handle mutations during use
public void testSplitterIterableIsLazy_pattern() {
if (!Platform.usingJdkPatternCompiler()) {
if (!CommonPattern.isPcreLike()) {
return;
}
assertSplitterIterableIsLazy(Splitter.onPattern(","));
Expand Down
6 changes: 5 additions & 1 deletion guava/src/com/google/common/base/CommonPattern.java
View file
Expand Up @@ -33,7 +33,11 @@ abstract class CommonPattern {
@Override
public abstract String toString();

static CommonPattern compile(String pattern) {
public static CommonPattern compile(String pattern) {
return Platform.compilePattern(pattern);
}

public static boolean isPcreLike() {
return Platform.patternCompilerIsPcreLike();
}
}
6 changes: 6 additions & 0 deletions guava/src/com/google/common/base/PatternCompiler.java
View file
Expand Up @@ -29,4 +29,10 @@ interface PatternCompiler {
* @throws IllegalArgumentException if the pattern is invalid
*/
CommonPattern compile(String pattern);

/**
* Returns {@code true} if the regex implementation behaves like Perl -- notably, by supporting
* possessive quantifiers but also being susceptible to catastrophic backtracking.
*/
boolean isPcreLike();
}
9 changes: 7 additions & 2 deletions guava/src/com/google/common/base/Platform.java
View file
Expand Up @@ -70,8 +70,8 @@ static CommonPattern compilePattern(String pattern) {
return patternCompiler.compile(pattern);
}

static boolean usingJdkPatternCompiler() {
return patternCompiler instanceof JdkPatternCompiler;
static boolean patternCompilerIsPcreLike() {
return patternCompiler.isPcreLike();
}

private static PatternCompiler loadPatternCompiler() {
Expand All @@ -87,5 +87,10 @@ private static final class JdkPatternCompiler implements PatternCompiler {
public CommonPattern compile(String pattern) {
return new JdkPattern(Pattern.compile(pattern));
}

@Override
public boolean isPcreLike() {
return true;
}
}
}
31 changes: 23 additions & 8 deletions guava/src/com/google/common/primitives/Doubles.java
View file
Expand Up @@ -35,7 +35,6 @@
import java.util.RandomAccess;
import java.util.Spliterator;
import java.util.Spliterators;
import java.util.regex.Pattern;
import org.checkerframework.checker.nullness.qual.Nullable;

/**
Expand Down Expand Up @@ -653,16 +652,32 @@ public String toString() {
* that pass this regex are valid -- only a performance hit is incurred, not a semantics bug.
*/
@GwtIncompatible // regular expressions
static final Pattern FLOATING_POINT_PATTERN = fpPattern();
static final
java.util.regex.Pattern
FLOATING_POINT_PATTERN = fpPattern();

@GwtIncompatible // regular expressions
private static Pattern fpPattern() {
String decimal = "(?:\\d++(?:\\.\\d*+)?|\\.\\d++)";
String completeDec = decimal + "(?:[eE][+-]?\\d++)?[fFdD]?";
String hex = "(?:\\p{XDigit}++(?:\\.\\p{XDigit}*+)?|\\.\\p{XDigit}++)";
String completeHex = "0[xX]" + hex + "[pP][+-]?\\d++[fFdD]?";
private static
java.util.regex.Pattern
fpPattern() {
/*
* We use # instead of * for possessive quantifiers. This lets us strip them out when building
* the regex for RE2 (which doesn't support them) but leave them in when building it for
* java.util.regex (where we want them in order to avoid catastrophic backtracking).
*/
String decimal = "(?:\\d+#(?:\\.\\d*#)?|\\.\\d+#)";
String completeDec = decimal + "(?:[eE][+-]?\\d+#)?[fFdD]?";
String hex = "(?:[0-9a-fA-F]+#(?:\\.[0-9a-fA-F]*#)?|\\.[0-9a-fA-F]+#)";
String completeHex = "0[xX]" + hex + "[pP][+-]?\\d+#[fFdD]?";
String fpPattern = "[+-]?(?:NaN|Infinity|" + completeDec + "|" + completeHex + ")";
return Pattern.compile(fpPattern);
fpPattern =
fpPattern.replace(
"#",
"+"
);
return
java.util.regex.Pattern
.compile(fpPattern);
}

/**
Expand Down

0 comments on commit 08adcec

Please sign in to comment.