openCypher icon indicating copy to clipboard operation
openCypher copied to clipboard

Why is pipeline used to define Expression rule?

Open hagen666 opened this issue 5 years ago • 2 comments

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.

hagen666 avatar Aug 31 '19 03:08 hagen666

@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

Mats-SX avatar Sep 10 '19 06:09 Mats-SX

OK. I'll have a try!

hagen666 avatar Sep 10 '19 07:09 hagen666