Repository: groovy
Updated Branches:
  refs/heads/master f2bdd2d62 -> b9b69444c


Refine statementExpression rule for better performance

FYI, groovy-parser tests parsing groovy source code from:

1) groovy 2.5.0
2) grails 3.2.0
3) gradle 3.1
4) spock 1.1-rc-2
5) geb 1.0

the test build time of danielsun1106/groovy-parser is reduced from 32m to 27m.


Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/b9b69444
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/b9b69444
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/b9b69444

Branch: refs/heads/master
Commit: b9b69444c45edd063553a8d8191e32300a780afc
Parents: f2bdd2d
Author: Daniel Sun <sun...@apache.org>
Authored: Sat Dec 1 22:13:11 2018 +0800
Committer: Daniel Sun <sun...@apache.org>
Committed: Sat Dec 1 22:13:11 2018 +0800

----------------------------------------------------------------------
 src/antlr/GroovyParser.g4                       |  12 +--
 src/test/gls/generics/GenericsTest.groovy       |   6 +-
 src/test/groovy/bugs/Groovy5318Bug.groovy       |   2 +-
 .../apache/groovy/parser/antlr4/AstBuilder.java | 100 +++++++++++--------
 .../parser/antlr4/SemanticPredicates.java       |  47 ++++++++-
 5 files changed, 105 insertions(+), 62 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/b9b69444/src/antlr/GroovyParser.g4
----------------------------------------------------------------------
diff --git a/src/antlr/GroovyParser.g4 b/src/antlr/GroovyParser.g4
index 3f768a5..926fb5b 100644
--- a/src/antlr/GroovyParser.g4
+++ b/src/antlr/GroovyParser.g4
@@ -768,14 +768,8 @@ enhancedStatementExpression
     |   standardLambdaExpression
     ;
 
-/**
- *  In order to resolve the syntactic ambiguities, e.g. (String)'abc' can be 
parsed as a cast expression or a parentheses-less method call(method name: 
(String), arguments: 'abc')
- *      try to match expression first.
- *  If it is not a normal expression, then try to match the command expression
- */
 statementExpression
-    :   expression                          #normalExprAlt
-    |   commandExpression                   #commandExprAlt
+    :   commandExpression                   #commandExprAlt
     ;
 
 postfixExpression
@@ -886,9 +880,9 @@ enhancedExpression
 */
 
 commandExpression
-    :   pathExpression
+    :   expression
         (
-            { SemanticPredicates.isFollowingMethodName($pathExpression.t) }?
+            { 
!SemanticPredicates.isFollowingArgumentsOrClosure($expression.ctx) }?
             argumentList
         |
             /* if pathExpression is a method call, no need to have any more 
arguments */

http://git-wip-us.apache.org/repos/asf/groovy/blob/b9b69444/src/test/gls/generics/GenericsTest.groovy
----------------------------------------------------------------------
diff --git a/src/test/gls/generics/GenericsTest.groovy 
b/src/test/gls/generics/GenericsTest.groovy
index 8a68145..43025c0 100644
--- a/src/test/gls/generics/GenericsTest.groovy
+++ b/src/test/gls/generics/GenericsTest.groovy
@@ -398,11 +398,11 @@ import java.util.concurrent.atomic.AtomicInteger
         } else {
             shouldFailCompilationWithMessage """
                 def list1 = new ArrayList<Integer()
-            """, "Unexpected input: 'new ArrayList<Integer('"
+            """, "Unexpected input: '('"
 
             shouldFailCompilationWithMessage """
                 List<Integer list2 = new ArrayList<Integer>()
-            """, "Unexpected input: 'List<Integer list2'"
+            """, "Unexpected input: 'List<Integer'"
 
             shouldFailCompilationWithMessage """
                 def c = []
@@ -431,7 +431,7 @@ import java.util.concurrent.atomic.AtomicInteger
 
             shouldFailCompilationWithMessage """
                 def List<List<Integer>> history = new ArrayList<List<Integer>()
-            """, "Unexpected input: 'new ArrayList<List<Integer>('"
+            """, "Unexpected input: '('"
         }
     }
 

http://git-wip-us.apache.org/repos/asf/groovy/blob/b9b69444/src/test/groovy/bugs/Groovy5318Bug.groovy
----------------------------------------------------------------------
diff --git a/src/test/groovy/bugs/Groovy5318Bug.groovy 
b/src/test/groovy/bugs/Groovy5318Bug.groovy
index bf2610a..da34b04 100644
--- a/src/test/groovy/bugs/Groovy5318Bug.groovy
+++ b/src/test/groovy/bugs/Groovy5318Bug.groovy
@@ -31,7 +31,7 @@ class Groovy5318Bug extends CompilableTestSupport {
         if (ParserVersion.V_2 == CompilerConfiguration.DEFAULT.parserVersion) {
             assert message.contains('Unexpected type arguments found prior to: 
ArrayList')
         } else {
-            assert message.contains('Unexpected input: \'new 
java.util<Integer>.\'')
+            assert message.contains('Unexpected input: \'.\'')
         }
     }
 }

http://git-wip-us.apache.org/repos/asf/groovy/blob/b9b69444/subprojects/parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
----------------------------------------------------------------------
diff --git 
a/subprojects/parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
 
b/subprojects/parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
index ee6b625..1697088 100644
--- 
a/subprojects/parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
+++ 
b/subprojects/parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
@@ -277,7 +277,6 @@ import static 
org.apache.groovy.parser.antlr4.GroovyLangParser.NamePartContext;
 import static 
org.apache.groovy.parser.antlr4.GroovyLangParser.NamedPropertyArgsContext;
 import static 
org.apache.groovy.parser.antlr4.GroovyLangParser.NewPrmrAltContext;
 import static 
org.apache.groovy.parser.antlr4.GroovyLangParser.NonWildcardTypeArgumentsContext;
-import static 
org.apache.groovy.parser.antlr4.GroovyLangParser.NormalExprAltContext;
 import static 
org.apache.groovy.parser.antlr4.GroovyLangParser.NullLiteralAltContext;
 import static org.apache.groovy.parser.antlr4.GroovyLangParser.PRIVATE;
 import static 
org.apache.groovy.parser.antlr4.GroovyLangParser.PackageDeclarationContext;
@@ -2092,12 +2091,6 @@ public class AstBuilder extends 
GroovyParserBaseVisitor<Object> implements Groov
                 ctx);
     }
 
-
-    @Override
-    public ExpressionStatement visitNormalExprAlt(NormalExprAltContext ctx) {
-        return configureAST(new ExpressionStatement((Expression) 
this.visit(ctx.expression())), ctx);
-    }
-
 /*
     @Override
     public Expression visitEnhancedExpression(EnhancedExpressionContext ctx) {
@@ -2117,60 +2110,79 @@ public class AstBuilder extends 
GroovyParserBaseVisitor<Object> implements Groov
 
     @Override
     public ExpressionStatement visitCommandExprAlt(CommandExprAltContext ctx) {
-        if (visitingArrayInitializerCnt > 0) {
+        return configureAST(new 
ExpressionStatement(this.visitCommandExpression(ctx.commandExpression())), ctx);
+    }
+
+    @Override
+    public Expression visitCommandExpression(CommandExpressionContext ctx) {
+        boolean hasArgumentList = asBoolean(ctx.enhancedArgumentList());
+        boolean hasCommandArgument = asBoolean(ctx.commandArgument());
+
+        if (visitingArrayInitializerCnt > 0 && (hasArgumentList || 
hasCommandArgument)) {
             // To avoid ambiguities, command chain expression should not be 
used in array initializer
             // the old parser does not support either, so no breaking changes
             // SEE 
http://groovy.329449.n5.nabble.com/parrot-Command-expressions-in-array-initializer-tt5752273.html
             throw createParsingFailedException("Command chain expression can 
not be used in array initializer", ctx);
         }
 
-        return configureAST(new 
ExpressionStatement(this.visitCommandExpression(ctx.commandExpression())), ctx);
-    }
+        Expression baseExpr = (Expression) this.visit(ctx.expression());
 
-    @Override
-    public Expression visitCommandExpression(CommandExpressionContext ctx) {
-        Expression baseExpr = this.visitPathExpression(ctx.pathExpression());
-        Expression arguments = 
this.visitEnhancedArgumentList(ctx.enhancedArgumentList());
+        if (hasArgumentList || hasCommandArgument) {
+            if (baseExpr instanceof BinaryExpression) {
+                if (!"[".equals(((BinaryExpression) 
baseExpr).getOperation().getText()) && !isInsideParentheses(baseExpr)) {
+                    throw createParsingFailedException("Unexpected input: '" + 
ctx.expression().getText() + "'", ctx.expression());
+                }
+            }
+        }
 
-        MethodCallExpression methodCallExpression;
-        if (baseExpr instanceof PropertyExpression) { // e.g. obj.a 1, 2
-            methodCallExpression =
-                    configureAST(
-                            this.createMethodCallExpression(
-                                    (PropertyExpression) baseExpr, arguments),
-                            arguments);
-
-        } else if (baseExpr instanceof MethodCallExpression && 
!isInsideParentheses(baseExpr)) { // e.g. m {} a, b  OR  m(...) a, b
-            if (asBoolean(arguments)) {
-                // The error should never be thrown.
-                throw new GroovyBugError("When baseExpr is a instance of 
MethodCallExpression, which should follow NO argumentList");
+        MethodCallExpression methodCallExpression = null;
+
+        if (hasArgumentList) {
+            Expression arguments = 
this.visitEnhancedArgumentList(ctx.enhancedArgumentList());
+
+            if (baseExpr instanceof PropertyExpression) { // e.g. obj.a 1, 2
+                methodCallExpression =
+                        configureAST(
+                                this.createMethodCallExpression(
+                                        (PropertyExpression) baseExpr, 
arguments),
+                                arguments);
+
+            } else if (baseExpr instanceof MethodCallExpression && 
!isInsideParentheses(baseExpr)) { // e.g. m {} a, b  OR  m(...) a, b
+                if (asBoolean(arguments)) {
+                    // The error should never be thrown.
+                    throw new GroovyBugError("When baseExpr is a instance of 
MethodCallExpression, which should follow NO argumentList");
+                }
+
+                methodCallExpression = (MethodCallExpression) baseExpr;
+            } else if (
+                    !isInsideParentheses(baseExpr)
+                            && (baseExpr instanceof VariableExpression /* e.g. 
m 1, 2 */
+                            || baseExpr instanceof GStringExpression /* e.g. 
"$m" 1, 2 */
+                            || (baseExpr instanceof ConstantExpression && 
isTrue(baseExpr, IS_STRING)) /* e.g. "m" 1, 2 */)
+            ) {
+                methodCallExpression =
+                        configureAST(
+                                this.createMethodCallExpression(baseExpr, 
arguments),
+                                arguments);
+            } else { // e.g. a[x] b, new A() b, etc.
+                methodCallExpression = 
configureAST(this.createCallMethodCallExpression(baseExpr, arguments), 
arguments);
             }
 
-            methodCallExpression = (MethodCallExpression) baseExpr;
-        } else if (
-                !isInsideParentheses(baseExpr)
-                        && (baseExpr instanceof VariableExpression /* e.g. m 
1, 2 */
-                        || baseExpr instanceof GStringExpression /* e.g. "$m" 
1, 2 */
-                        || (baseExpr instanceof ConstantExpression && 
isTrue(baseExpr, IS_STRING)) /* e.g. "m" 1, 2 */)
-                ) {
-            methodCallExpression =
-                    configureAST(
-                            this.createMethodCallExpression(baseExpr, 
arguments),
-                            arguments);
-        } else { // e.g. a[x] b, new A() b, etc.
-            methodCallExpression = 
configureAST(this.createCallMethodCallExpression(baseExpr, arguments), 
arguments);
-        }
+            methodCallExpression.putNodeMetaData(IS_COMMAND_EXPRESSION, true);
 
-        methodCallExpression.putNodeMetaData(IS_COMMAND_EXPRESSION, true);
+            if (!hasCommandArgument) {
+                return configureAST(methodCallExpression, ctx);
+            }
+        }
 
-        if (!asBoolean(ctx.commandArgument())) {
-            return configureAST(methodCallExpression, ctx);
+        if (hasCommandArgument) {
+            baseExpr.putNodeMetaData(IS_COMMAND_EXPRESSION, true);
         }
 
         return configureAST(
                 (Expression) ctx.commandArgument().stream()
                         .map(e -> (Object) e)
-                        .reduce(methodCallExpression,
+                        .reduce(null == methodCallExpression ? baseExpr : 
methodCallExpression,
                                 (r, e) -> {
                                     CommandArgumentContext 
commandArgumentContext = (CommandArgumentContext) e;
                                     
commandArgumentContext.putNodeMetaData(CMD_EXPRESSION_BASE_EXPR, r);

http://git-wip-us.apache.org/repos/asf/groovy/blob/b9b69444/subprojects/parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/SemanticPredicates.java
----------------------------------------------------------------------
diff --git 
a/subprojects/parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/SemanticPredicates.java
 
b/subprojects/parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/SemanticPredicates.java
index ca6fb44..3efb00c 100644
--- 
a/subprojects/parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/SemanticPredicates.java
+++ 
b/subprojects/parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/SemanticPredicates.java
@@ -21,12 +21,25 @@ package org.apache.groovy.parser.antlr4;
 import org.antlr.v4.runtime.CharStream;
 import org.antlr.v4.runtime.Token;
 import org.antlr.v4.runtime.TokenStream;
+import org.antlr.v4.runtime.tree.ParseTree;
 
 import java.util.Collections;
+import java.util.List;
 import java.util.Set;
 
-import static org.apache.groovy.parser.antlr4.GroovyParser.*;
-
+import static org.apache.groovy.parser.antlr4.GroovyParser.ASSIGN;
+import static 
org.apache.groovy.parser.antlr4.GroovyParser.BuiltInPrimitiveType;
+import static 
org.apache.groovy.parser.antlr4.GroovyParser.CapitalizedIdentifier;
+import static org.apache.groovy.parser.antlr4.GroovyParser.DOT;
+import static org.apache.groovy.parser.antlr4.GroovyParser.ExpressionContext;
+import static org.apache.groovy.parser.antlr4.GroovyParser.Identifier;
+import static org.apache.groovy.parser.antlr4.GroovyParser.LBRACK;
+import static org.apache.groovy.parser.antlr4.GroovyParser.LPAREN;
+import static org.apache.groovy.parser.antlr4.GroovyParser.LT;
+import static 
org.apache.groovy.parser.antlr4.GroovyParser.PathExpressionContext;
+import static 
org.apache.groovy.parser.antlr4.GroovyParser.PostfixExprAltContext;
+import static 
org.apache.groovy.parser.antlr4.GroovyParser.PostfixExpressionContext;
+import static org.apache.groovy.parser.antlr4.GroovyParser.StringLiteral;
 /**
  * Some semantic predicates for altering the behaviour of the lexer and parser
  *
@@ -91,10 +104,34 @@ public class SemanticPredicates {
      * Check whether following a method name of command expression.
      * Method name should not end with "2: arguments" and "3: closure"
      *
-     * @param t the type of pathExpression
+     * @param context the preceding expression
+     * @return
      */
-    public static boolean isFollowingMethodName(int t) {
-        return !(2 == t || 3 == t);
+    public static boolean isFollowingArgumentsOrClosure(ExpressionContext 
context) {
+        if (context instanceof PostfixExprAltContext) {
+            List<ParseTree> peacChildren = ((PostfixExprAltContext) 
context).children;
+
+            if (1 == peacChildren.size()) {
+                ParseTree peacChild = peacChildren.get(0);
+
+                if (peacChild instanceof PostfixExpressionContext) {
+                    List<ParseTree>  pecChildren = ((PostfixExpressionContext) 
peacChild).children;
+
+                    if (1 == pecChildren.size()) {
+                        ParseTree pecChild = pecChildren.get(0);
+
+                        if (pecChild instanceof PathExpressionContext) {
+                            PathExpressionContext pec = 
(PathExpressionContext) pecChild;
+                            int t = pec.t;
+
+                            return (2 == t || 3 == t);
+                        }
+                    }
+                }
+            }
+        }
+
+        return false;
     }
 
     /**

Reply via email to