Re: [2/2] groovy git commit: Refine the grammar: 1)`def` is still required for performance when declaring tuple; 2)Stop supporting command expression in parentheses

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: [2/2] groovy git commit: Refine the grammar: 1)`def` is still required for performance when declaring tuple; 2)Stop supporting command expression in parentheses

paulk_asert
I think this is worthy of further discussion on the mailing list. I know it's a parrot only feature so far but supporting
command expressions in parentheses seems like a worthy feature for DSLs.

Cheers, Paul.

On Sat, Sep 16, 2017 at 2:02 AM, <[hidden email]> wrote:
Refine the grammar: 1)`def` is still required for performance when declaring tuple; 2)Stop supporting command expression in parentheses

(cherry picked from commit 7c77392)


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

Branch: refs/heads/GROOVY_2_6_X
Commit: 21bae556d29c2f879f92d6b00ea771deda0882ee
Parents: b3f386a
Author: sunlan <[hidden email]>
Authored: Fri Sep 15 23:53:49 2017 +0800
Committer: sunlan <[hidden email]>
Committed: Sat Sep 16 00:02:42 2017 +0800

----------------------------------------------------------------------
 src/antlr/GroovyParser.g4                         |  7 ++++++-
 .../apache/groovy/parser/antlr4/AstBuilder.java   | 18 +++++++++++++++++-
 .../groovy/parser/antlr4/GroovyParserTest.groovy  |  4 +++-
 .../core/LocalVariableDeclaration_02x.groovy      |  3 +++
 4 files changed, 29 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/21bae556/src/antlr/GroovyParser.g4
----------------------------------------------------------------------
diff --git a/src/antlr/GroovyParser.g4 b/src/antlr/GroovyParser.g4
index a03da00..3ae8366 100644
--- a/src/antlr/GroovyParser.g4
+++ b/src/antlr/GroovyParser.g4
@@ -812,7 +812,7 @@ parExpression
     ;

 expressionInPar
-    :   LPAREN enhancedStatementExpression rparen
+    :   LPAREN enhancedExpression rparen
     ;

 expressionList[boolean canSpread]
@@ -948,6 +948,11 @@ expression
                      enhancedStatementExpression                                            #assignmentExprAlt
     ;

+enhancedExpression
+    :   expression
+    |   standardLambdaExpression
+    ;
+
 commandExpression
     :   pathExpression
         (

http://git-wip-us.apache.org/repos/asf/groovy/blob/21bae556/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 a9a29fc..4239300 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
@@ -1806,6 +1806,22 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> implements Groov
     }

     @Override
+    public Expression visitEnhancedExpression(EnhancedExpressionContext ctx) {
+        Expression expression;
+
+        if (asBoolean(ctx.expression())) {
+            expression = (Expression) this.visit(ctx.expression());
+        } else if (asBoolean(ctx.standardLambdaExpression())) {
+            expression = this.visitStandardLambdaExpression(ctx.standardLambdaExpression());
+        } else {
+            throw createParsingFailedException("Unsupported enhanced expression: " + ctx.getText(), ctx);
+        }
+
+        return configureAST(expression, ctx);
+    }
+
+
+    @Override
     public ExpressionStatement visitCommandExprAlt(CommandExprAltContext ctx) {
         return configureAST(new ExpressionStatement(this.visitCommandExpression(ctx.commandExpression())), ctx);
     }
@@ -1929,7 +1945,7 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> implements Groov

     @Override
     public Expression visitExpressionInPar(ExpressionInParContext ctx) {
-        return this.visitEnhancedStatementExpression(ctx.enhancedStatementExpression());
+        return this.visitEnhancedExpression(ctx.enhancedExpression());
     }

     @Override

http://git-wip-us.apache.org/repos/asf/groovy/blob/21bae556/subprojects/parser-antlr4/src/test/groovy/org/apache/groovy/parser/antlr4/GroovyParserTest.groovy
----------------------------------------------------------------------
diff --git a/subprojects/parser-antlr4/src/test/groovy/org/apache/groovy/parser/antlr4/GroovyParserTest.groovy b/subprojects/parser-antlr4/src/test/groovy/org/apache/groovy/parser/antlr4/GroovyParserTest.groovy
index 4ff6ad3..5b3fb7b 100644
--- a/subprojects/parser-antlr4/src/test/groovy/org/apache/groovy/parser/antlr4/GroovyParserTest.groovy
+++ b/subprojects/parser-antlr4/src/test/groovy/org/apache/groovy/parser/antlr4/GroovyParserTest.groovy
@@ -290,6 +290,8 @@ class GroovyParserTest extends GroovyTestCase {

     void "test groovy core - LocalVariableDeclaration"() {
         doTest('core/LocalVariableDeclaration_01.groovy', [Token]); // [class org.codehaus.groovy.syntax.Token][startLine]:: 9 != 8
+        doRunAndTest('core/LocalVariableDeclaration_02x.groovy')
+
     }

     void "test groovy core - MethodDeclaration"() {
@@ -339,7 +341,7 @@ class GroovyParserTest extends GroovyTestCase {
         doTest('core/Command_03.groovy', [ExpressionStatement, Parameter]);
         doTest('core/Command_04.groovy', [ExpressionStatement]);
         doTest('core/Command_05.groovy');
-        doRunAndTest('core/Command_06x.groovy')
+//        doRunAndTest('core/Command_06x.groovy')
     }

     /*

http://git-wip-us.apache.org/repos/asf/groovy/blob/21bae556/subprojects/parser-antlr4/src/test/resources/core/LocalVariableDeclaration_02x.groovy
----------------------------------------------------------------------
diff --git a/subprojects/parser-antlr4/src/test/resources/core/LocalVariableDeclaration_02x.groovy b/subprojects/parser-antlr4/src/test/resources/core/LocalVariableDeclaration_02x.groovy
new file mode 100644
index 0000000..1e3a2ba
--- /dev/null
+++ b/subprojects/parser-antlr4/src/test/resources/core/LocalVariableDeclaration_02x.groovy
@@ -0,0 +1,3 @@
+def (int x, int y) = [1, 2]
+assert 1 == x
+assert 2 == y


Reply | Threaded
Open this post in threaded view
|

Re: [2/2] groovy git commit: Refine the grammar: 1)`def` is still required for performance when declaring tuple; 2)Stop supporting command expression in parentheses

Guillaume Laforge
Administrator
DSL is a key use case, so let's not reduce groovy's capabilities in this area, indeed.

Le 16 sept. 2017 04:23, "Paul King" <[hidden email]> a écrit :
I think this is worthy of further discussion on the mailing list. I know it's a parrot only feature so far but supporting
command expressions in parentheses seems like a worthy feature for DSLs.

Cheers, Paul.

On Sat, Sep 16, 2017 at 2:02 AM, <[hidden email]> wrote:
Refine the grammar: 1)`def` is still required for performance when declaring tuple; 2)Stop supporting command expression in parentheses

(cherry picked from commit 7c77392)


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

Branch: refs/heads/GROOVY_2_6_X
Commit: 21bae556d29c2f879f92d6b00ea771deda0882ee
Parents: b3f386a
Author: sunlan <[hidden email]>
Authored: Fri Sep 15 23:53:49 2017 +0800
Committer: sunlan <[hidden email]>
Committed: Sat Sep 16 00:02:42 2017 +0800

----------------------------------------------------------------------
 src/antlr/GroovyParser.g4                         |  7 ++++++-
 .../apache/groovy/parser/antlr4/AstBuilder.java   | 18 +++++++++++++++++-
 .../groovy/parser/antlr4/GroovyParserTest.groovy  |  4 +++-
 .../core/LocalVariableDeclaration_02x.groovy      |  3 +++
 4 files changed, 29 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/21bae556/src/antlr/GroovyParser.g4
----------------------------------------------------------------------
diff --git a/src/antlr/GroovyParser.g4 b/src/antlr/GroovyParser.g4
index a03da00..3ae8366 100644
--- a/src/antlr/GroovyParser.g4
+++ b/src/antlr/GroovyParser.g4
@@ -812,7 +812,7 @@ parExpression
     ;

 expressionInPar
-    :   LPAREN enhancedStatementExpression rparen
+    :   LPAREN enhancedExpression rparen
     ;

 expressionList[boolean canSpread]
@@ -948,6 +948,11 @@ expression
                      enhancedStatementExpression                                            #assignmentExprAlt
     ;

+enhancedExpression
+    :   expression
+    |   standardLambdaExpression
+    ;
+
 commandExpression
     :   pathExpression
         (

http://git-wip-us.apache.org/repos/asf/groovy/blob/21bae556/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 a9a29fc..4239300 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
@@ -1806,6 +1806,22 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> implements Groov
     }

     @Override
+    public Expression visitEnhancedExpression(EnhancedExpressionContext ctx) {
+        Expression expression;
+
+        if (asBoolean(ctx.expression())) {
+            expression = (Expression) this.visit(ctx.expression());
+        } else if (asBoolean(ctx.standardLambdaExpression())) {
+            expression = this.visitStandardLambdaExpression(ctx.standardLambdaExpression());
+        } else {
+            throw createParsingFailedException("Unsupported enhanced expression: " + ctx.getText(), ctx);
+        }
+
+        return configureAST(expression, ctx);
+    }
+
+
+    @Override
     public ExpressionStatement visitCommandExprAlt(CommandExprAltContext ctx) {
         return configureAST(new ExpressionStatement(this.visitCommandExpression(ctx.commandExpression())), ctx);
     }
@@ -1929,7 +1945,7 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> implements Groov

     @Override
     public Expression visitExpressionInPar(ExpressionInParContext ctx) {
-        return this.visitEnhancedStatementExpression(ctx.enhancedStatementExpression());
+        return this.visitEnhancedExpression(ctx.enhancedExpression());
     }

     @Override

http://git-wip-us.apache.org/repos/asf/groovy/blob/21bae556/subprojects/parser-antlr4/src/test/groovy/org/apache/groovy/parser/antlr4/GroovyParserTest.groovy
----------------------------------------------------------------------
diff --git a/subprojects/parser-antlr4/src/test/groovy/org/apache/groovy/parser/antlr4/GroovyParserTest.groovy b/subprojects/parser-antlr4/src/test/groovy/org/apache/groovy/parser/antlr4/GroovyParserTest.groovy
index 4ff6ad3..5b3fb7b 100644
--- a/subprojects/parser-antlr4/src/test/groovy/org/apache/groovy/parser/antlr4/GroovyParserTest.groovy
+++ b/subprojects/parser-antlr4/src/test/groovy/org/apache/groovy/parser/antlr4/GroovyParserTest.groovy
@@ -290,6 +290,8 @@ class GroovyParserTest extends GroovyTestCase {

     void "test groovy core - LocalVariableDeclaration"() {
         doTest('core/LocalVariableDeclaration_01.groovy', [Token]); // [class org.codehaus.groovy.syntax.Token][startLine]:: 9 != 8
+        doRunAndTest('core/LocalVariableDeclaration_02x.groovy')
+
     }

     void "test groovy core - MethodDeclaration"() {
@@ -339,7 +341,7 @@ class GroovyParserTest extends GroovyTestCase {
         doTest('core/Command_03.groovy', [ExpressionStatement, Parameter]);
         doTest('core/Command_04.groovy', [ExpressionStatement]);
         doTest('core/Command_05.groovy');
-        doRunAndTest('core/Command_06x.groovy')
+//        doRunAndTest('core/Command_06x.groovy')
     }

     /*

http://git-wip-us.apache.org/repos/asf/groovy/blob/21bae556/subprojects/parser-antlr4/src/test/resources/core/LocalVariableDeclaration_02x.groovy
----------------------------------------------------------------------
diff --git a/subprojects/parser-antlr4/src/test/resources/core/LocalVariableDeclaration_02x.groovy b/subprojects/parser-antlr4/src/test/resources/core/LocalVariableDeclaration_02x.groovy
new file mode 100644
index 0000000..1e3a2ba
--- /dev/null
+++ b/subprojects/parser-antlr4/src/test/resources/core/LocalVariableDeclaration_02x.groovy
@@ -0,0 +1,3 @@
+def (int x, int y) = [1, 2]
+assert 1 == x
+assert 2 == y


Reply | Threaded
Open this post in threaded view
|

Re: [2/2] groovy git commit: Refine the grammar: 1)`def` is still required for performance when declaring tuple; 2)Stop supporting command expression in parentheses

Daniel Sun
It is an enhanced feature introduced by Parrot, but I found it results in
some issues, e.g.
1) `foo(String boo)`
2) multi-assigin issue, e.g. `(X x, Y y)` is parsed as command expression in
parentheses.

Parrot introduces many new features, which impacts performance a lot too...


Cheers,
Daniel.Sun



--
Sent from: http://groovy.329449.n5.nabble.com/Groovy-Dev-f372993.html
Reply | Threaded
Open this post in threaded view
|

Re: [2/2] groovy git commit: Refine the grammar: 1)`def` is still required for performance when declaring tuple; 2)Stop supporting command expression in parentheses

Guillaume Laforge
Administrator
So the command expression aspects that you removed were only stuff that was new in Parrot?
I mistakenly thought it was stuff that was there before parrot.

On Sat, Sep 16, 2017 at 1:56 PM, Daniel Sun <[hidden email]> wrote:
It is an enhanced feature introduced by Parrot, but I found it results in
some issues, e.g.
1) `foo(String boo)`
2) multi-assigin issue, e.g. `(X x, Y y)` is parsed as command expression in
parentheses.

Parrot introduces many new features, which impacts performance a lot too...


Cheers,
Daniel.Sun



--
Sent from: http://groovy.329449.n5.nabble.com/Groovy-Dev-f372993.html



--
Guillaume Laforge
Apache Groovy committer & PMC Vice-President
Developer Advocate @ Google Cloud Platform

Reply | Threaded
Open this post in threaded view
|

Re: [2/2] groovy git commit: Refine the grammar: 1)`def` is still required for performance when declaring tuple; 2)Stop supporting command expression in parentheses

Daniel.Sun
Yeah. It's a new feature introduced by Parrot.
We can discuss command expression further later.

Cheers,
Daniel.Sun



--
Sent from: http://groovy.329449.n5.nabble.com/Groovy-Dev-f372993.html