opencypher / openCypher

Specification of the Cypher property graph query language
http://www.opencypher.org
Apache License 2.0
855 stars 150 forks source link

Why is pipeline used to define Expression rule? #380

Open hagen666 opened 5 years ago

hagen666 commented 5 years ago

Hi, I read the Cypher.g4 file and have many questions. One is about the Expression.

It seems that the oC_Expression rule use a pipeline:

oC_Expression : oC_OrExpression ; oC_OrExpression : oC_XorExpression ( SP OR SP oC_XorExpression ) ; oC_XorExpression : oC_AndExpression ( SP XOR SP oC_AndExpression ) ; oC_AndExpression : oC_NotExpression ( SP AND SP oC_NotExpression ) ; oC_NotExpression : ( NOT SP? ) oC_ComparisonExpression ; oC_ComparisonExpression : oC_AddOrSubtractExpression ( SP? oC_PartialComparisonExpression ) ; oC_AddOrSubtractExpression : oC_MultiplyDivideModuloExpression ( ( SP? '+' SP? oC_MultiplyDivideModuloExpression ) | ( SP? '-' SP? oC_MultiplyDivideModuloExpression ) ) ; oC_MultiplyDivideModuloExpression : oC_PowerOfExpression ( ( SP? '' SP? oC_PowerOfExpression ) | ( SP? '/' SP? oC_PowerOfExpression ) | ( SP? '%' SP? oC_PowerOfExpression ) ) ; oC_PowerOfExpression : oC_UnaryAddOrSubtractExpression ( SP? '^' SP? oC_UnaryAddOrSubtractExpression ) ; oC_UnaryAddOrSubtractExpression : ( ( '+' | '-' ) SP? ) oC_StringListNullOperatorExpression ;

What's the advantage of the pipeline rules? Why not use the rules like this? For example, BooleanExpression:

oC_BoolExpression: oC_BooleanLiteral | oC_StringListNullOperatorExpression | oC_ComparisonExpression | (ops=NOT SP?) oC_BoolExpression | '(' SP? oC_BoolExpression SP? ')' | oC_BoolExpression SP ops=AND SP oC_BoolExpression | oC_BoolExpression SP ops=OR SP oC_BoolExpression | oC_BoolExpression SP ops=XOR SP oC_BoolExpression ; I think the AST generated by this way may be more graceful. Looking forward to your comments or opinion.

Mats-SX commented 5 years ago

@hagen666 I can not directly see a reason where either of these is more expressive, which indicates that it may only be a matter of style. You could try to rewrite the source definitions and issue a PR, to see if the tests still pass. The file you'd need to edit is this one: https://github.com/opencypher/openCypher/blob/master/grammar/basic-grammar.xml#L188-L314

hagen666 commented 5 years ago

OK. I'll have a try!