You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2020/06/25 13:47:17 UTC

[groovy] 01/02: GROOVY-9588: groovyCompile 6x slower in 3.0.4 than 2.5.6

This is an automated email from the ASF dual-hosted git repository.

sunlan pushed a commit to branch GROOVY-9588
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit b3a951cb10fbaadb31e856e40e41b78362483908
Author: Daniel Sun <su...@apache.org>
AuthorDate: Thu Jun 25 16:56:07 2020 +0800

    GROOVY-9588: groovyCompile 6x slower in 3.0.4 than 2.5.6
    
    Antlr4 can not report missing token smartly as antlr2 does, so it have to use error alternatives, but which may impact the parsing performance a lot if the error alternative matches nothing.
---
 src/antlr/GroovyParser.g4                          |  7 +++---
 src/test/gls/generics/GenericsUsageTest.groovy     |  2 +-
 .../apache/groovy/parser/antlr4/PositionInfo.java  |  1 +
 .../parser/antlr4/SyntaxErrorReportable.java       | 25 ++++++++++++++++++++--
 .../parser/antlr4/util/PositionConfigureUtils.java | 17 +++++++++++++++
 .../groovy/parser/antlr4/SyntaxErrorTest.groovy    | 20 +++++++++++++++--
 6 files changed, 63 insertions(+), 9 deletions(-)

diff --git a/src/antlr/GroovyParser.g4 b/src/antlr/GroovyParser.g4
index 47b212c..c29212f 100644
--- a/src/antlr/GroovyParser.g4
+++ b/src/antlr/GroovyParser.g4
@@ -42,7 +42,7 @@ options {
 @header {
     import java.util.Map;
     import org.codehaus.groovy.ast.NodeMetaDataHandler;
-    import org.apache.groovy.parser.antlr4.SemanticPredicates;
+    import org.apache.groovy.parser.antlr4.util.PositionConfigureUtils;
 }
 
 @members {
@@ -1256,9 +1256,8 @@ keywords
 
 rparen
     :   RPAREN
-    |
-        // !!!Error Alternative, impact the performance of parsing
-        { require(false, "Missing ')'"); }
+    |   r=~RPAREN // !!!Error Alternative
+        { require(false, "Missing ')'", PositionConfigureUtils.calcMissingRparenOffset($r, _input)); }
     ;
 
 nls
diff --git a/src/test/gls/generics/GenericsUsageTest.groovy b/src/test/gls/generics/GenericsUsageTest.groovy
index 2305f8f..88a16d8 100644
--- a/src/test/gls/generics/GenericsUsageTest.groovy
+++ b/src/test/gls/generics/GenericsUsageTest.groovy
@@ -166,7 +166,7 @@ final class GenericsUsageTest extends CompilableTestSupport {
 
         shouldFailCompilationWithMessage """
             def m(Class<Integer someParam) {}
-        """, "Unexpected input: 'Class<Integer someParam'"
+        """, "Unexpected input: '<'"
 
         shouldFailCompilationWithMessage """
             abstract class ArrayList1<E extends AbstractList<E> implements List<E> {}
diff --git a/subprojects/parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/PositionInfo.java b/subprojects/parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/PositionInfo.java
index f4a752c..532df33 100644
--- a/subprojects/parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/PositionInfo.java
+++ b/subprojects/parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/PositionInfo.java
@@ -67,4 +67,5 @@ public class PositionInfo {
 
         return " @ line " + line + ", column " + column;
     }
+
 }
diff --git a/subprojects/parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/SyntaxErrorReportable.java b/subprojects/parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/SyntaxErrorReportable.java
index 021e2f5..92c927a 100644
--- a/subprojects/parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/SyntaxErrorReportable.java
+++ b/subprojects/parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/SyntaxErrorReportable.java
@@ -18,28 +18,45 @@
  */
 package org.apache.groovy.parser.antlr4;
 
+import groovy.lang.Tuple;
+import groovy.lang.Tuple2;
+
 /**
  * A SyntaxErrorReportable is a recognizer that can report syntax error
  */
 public interface SyntaxErrorReportable {
+    Tuple2<Integer, Integer> NO_OFFSET = Tuple.tuple(0, 0);
+
     default void require(boolean condition, String msg, int offset, boolean toAttachPositionInfo) {
+        require(condition, msg, Tuple.tuple(0, offset), toAttachPositionInfo);
+    }
+    default void require(boolean condition, String msg, Tuple2<Integer, Integer> offset, boolean toAttachPositionInfo) {
         if (condition) {
             return;
         }
 
         this.throwSyntaxError(msg, offset, toAttachPositionInfo);
     }
+
     default void require(boolean condition, String msg, boolean toAttachPositionInfo) {
-        require(condition, msg, 0, toAttachPositionInfo);
+        require(condition, msg, NO_OFFSET, toAttachPositionInfo);
     }
+
     default void require(boolean condition, String msg, int offset) {
+        require(condition, msg, Tuple.tuple(0, offset));
+    }
+    default void require(boolean condition, String msg, Tuple2<Integer, Integer> offset) {
         require(condition, msg, offset,false);
     }
+
     default void require(boolean condition, String msg) {
         require(condition, msg, false);
     }
 
     default void throwSyntaxError(String msg, int offset, boolean toAttachPositionInfo) {
+        throwSyntaxError(msg, Tuple.tuple(0, offset), toAttachPositionInfo);
+    }
+    default void throwSyntaxError(String msg, Tuple2<Integer, Integer> offset, boolean toAttachPositionInfo) {
         PositionInfo positionInfo = this.genPositionInfo(offset);
         throw new GroovySyntaxError(msg + (toAttachPositionInfo ? positionInfo.toString() : ""),
                 this.getSyntaxErrorSource(),
@@ -49,8 +66,12 @@ public interface SyntaxErrorReportable {
     }
 
     int getSyntaxErrorSource();
+
     default PositionInfo genPositionInfo(int offset) {
-        return new PositionInfo(getErrorLine(), getErrorColumn() + offset);
+        return genPositionInfo(Tuple.tuple(0, offset));
+    }
+    default PositionInfo genPositionInfo(Tuple2<Integer, Integer> offset) {
+        return new PositionInfo(getErrorLine() + offset.getV1(), getErrorColumn() + offset.getV2());
     }
 
     int getErrorLine();
diff --git a/subprojects/parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/util/PositionConfigureUtils.java b/subprojects/parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/util/PositionConfigureUtils.java
index a009468..de7a299 100644
--- a/subprojects/parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/util/PositionConfigureUtils.java
+++ b/subprojects/parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/util/PositionConfigureUtils.java
@@ -18,10 +18,13 @@
  */
 package org.apache.groovy.parser.antlr4.util;
 
+import groovy.lang.Tuple;
 import groovy.lang.Tuple2;
 import org.antlr.v4.runtime.Token;
+import org.antlr.v4.runtime.TokenStream;
 import org.antlr.v4.runtime.tree.TerminalNode;
 import org.apache.groovy.parser.antlr4.GroovyParser;
+import org.apache.groovy.parser.antlr4.SyntaxErrorReportable;
 import org.codehaus.groovy.ast.ASTNode;
 
 import static groovy.lang.Tuple.tuple;
@@ -31,6 +34,20 @@ import static org.codehaus.groovy.runtime.DefaultGroovyMethods.asBoolean;
  * Utilities for configuring node positions
  */
 public class PositionConfigureUtils {
+    public static Tuple2<Integer, Integer> calcMissingRparenOffset(Token errorToken, TokenStream tokenStream) {
+        Token token = tokenStream.LT(-2);
+        int tokenType = token.getType();
+
+        // DON'T adjust the offset for type casting, e.g. `println ((int) 6`
+        if (GroovyParser.RPAREN == tokenType) {
+            return SyntaxErrorReportable.NO_OFFSET;
+        }
+
+        final int errorTokenLength = errorToken.getText().length();
+
+        return Tuple.tuple(0, -errorTokenLength);
+    }
+
     /**
      * Sets location(lineNumber, colNumber, lastLineNumber, lastColumnNumber) for node using standard context information.
      * Note: this method is implemented to be closed over ASTNode. It returns same node as it received in arguments.
diff --git a/subprojects/parser-antlr4/src/test/groovy/org/apache/groovy/parser/antlr4/SyntaxErrorTest.groovy b/subprojects/parser-antlr4/src/test/groovy/org/apache/groovy/parser/antlr4/SyntaxErrorTest.groovy
index 0b789db..de3ad53 100644
--- a/subprojects/parser-antlr4/src/test/groovy/org/apache/groovy/parser/antlr4/SyntaxErrorTest.groovy
+++ b/subprojects/parser-antlr4/src/test/groovy/org/apache/groovy/parser/antlr4/SyntaxErrorTest.groovy
@@ -393,9 +393,9 @@ final class SyntaxErrorTest extends GroovyTestCase {
 
         assert err == '''\
             |startup failed:
-            |test.groovy: 1: Missing ')' @ line 1, column 14.
+            |test.groovy: 1: Missing ')' @ line 1, column 15.
             |   println ((int 123)
-            |                ^
+            |                 ^
             |
             |1 error
             |'''.stripMargin()
@@ -418,6 +418,22 @@ final class SyntaxErrorTest extends GroovyTestCase {
             |'''.stripMargin()
     }
 
+    void 'test error alternative - Missing ")" 3'() {
+        def err = expectParseError '''\
+            |def m( {
+            |}
+            |'''.stripMargin()
+
+        assert err == '''\
+            |startup failed:
+            |test.groovy: 1: Missing ')' @ line 1, column 8.
+            |   def m( {
+            |          ^
+            |
+            |1 error
+            |'''.stripMargin()
+    }
+
     @NotYetImplemented
     void 'test CompilerErrorTest_001'() {
         unzipScriptAndShouldFail('scripts/CompilerErrorTest_001.groovy', [])