Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Parser] Issues with BoolOp node generation in new parser #777

Closed
akshanshbhatt opened this issue Jul 18, 2022 · 3 comments
Closed

[Parser] Issues with BoolOp node generation in new parser #777

akshanshbhatt opened this issue Jul 18, 2022 · 3 comments
Labels
Parser Issues or improvements related to parser

Comments

@akshanshbhatt
Copy link
Collaborator

cat examples/expr2.py
a or b or c or dlpy --show-ast examples/expr2.py --tree  # Existing Parser-Module
  |-body=|-Expr
  |-value=BoolOp
  |     |-boolopType=Or
  |-values=|       |-Name
  |       | |-id=a
  |       |-expr_contextType=Load
  |       |-Name
  |       | |-id=b
  |       |-expr_contextType=Load
  |       |-Name
  |       | |-id=c
  |       |-expr_contextType=Load
  |-Name
  |         |-id=d
  |-expr_contextType=Load-type_ignores=↧

❯ lpy --show-ast --new-parser examples/expr2.py --tree  # New Parser-Module
  |-body=|-Expr
  |-value=BoolOp
  |     |-boolopType=Or
  |-values=|       |-BoolOp
  |       | |-boolopType=Or
  |       |-values=|       |   |-BoolOp
  |       |   | |-boolopType=Or
  |       |   |-values=|       |   |   |-Name
  |       |   |   | |-id=a
  |       |   |   |-expr_contextType=Load
  |       |   |-Name
  |       |   |     |-id=b
  |       |   |-expr_contextType=Load
  |       |-Name
  |       |     |-id=c
  |       |-expr_contextType=Load
  |-Name
  |         |-id=d
  |-expr_contextType=Load-type_ignores=

The new parser currently only supports BoolOp node generation with two operands (like BinOp). If an expression contains multiple and or or, the parser keeps creating new BoolOp nodes instead of retaining them in a single node with multiple expr values.

@akshanshbhatt
Copy link
Collaborator Author

Some other examples that should work -

a or (b or c) or d
a or ((b or c) or d) or e
a and b and c or d and e and f or g and h and i

We can add them as tests.

@Thirumalai-Shaktivel Thirumalai-Shaktivel added the Parser Issues or improvements related to parser label Jul 18, 2022
@Thirumalai-Shaktivel
Copy link
Collaborator

Some issues are fixed in #773. The only left out part is:

(a or b) or c

The problem here is that, as parenthesis information is not used or stored by the macros, it is difficult to handle the above example:
OLD Parser AST:

$ lpython --show-ast examples/expr2.py --tree
└-Module
  |-body=↧
  | └-Expr
  |   └-value=BoolOp
  |     |-boolopType=Or
  |     └-values=↧
  |       |-BoolOp
  |       | |-boolopType=Or
  |       | └-values=↧
  |       |   |-Name
  |       |   | |-id=a
  |       |   | └-expr_contextType=Load
  |       |   └-Name
  |       |     |-id=b
  |       |     └-expr_contextType=Load
  |       └-Name
  |         |-id=c
  |         └-expr_contextType=Load
  └-type_ignores=↧

NEW Parser AST (this PR):

$ lpython --show-ast examples/expr2.py --tree --new-parser
└-Module
  |-body=↧
  | └-Expr
  |   └-value=BoolOp
  |     |-boolopType=Or
  |     └-values=↧
  |       |-Name
  |       | |-id=a
  |       | └-expr_contextType=Load
  |       |-Name
  |       | |-id=b
  |       | └-expr_contextType=Load
  |       └-Name
  |         |-id=c
  |         └-expr_contextType=Load
  └-type_ignores=↧

@Thirumalai-Shaktivel
Copy link
Collaborator

Closing in favour of #848.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Parser Issues or improvements related to parser
Projects
None yet
Development

No branches or pull requests

2 participants