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

Define statements are parsed as let statements #683

Open
sifraser opened this issue Jul 3, 2018 · 5 comments
Open

Define statements are parsed as let statements #683

sifraser opened this issue Jul 3, 2018 · 5 comments
Labels
bug Incorrect behaviour of the tool language Issues in parser, TC, interpreter, POG or CG
Milestone

Comments

@sifraser
Copy link
Contributor

sifraser commented Jul 3, 2018

Description

In the StatementReader define statements are parsed as let statements (see https://github.com/overturetool/overture/blob/development/core/parser/src/main/java/org/overture/parser/syntax/StatementReader.java#L946).

This means that after parsing it is impossible to disambiguate a let statement from a define statement. This means that, for example, it is not possible to faithfully pretty print from the AST.

Versions

2.6.2

Additional Information

I have a branch locally in which an ADefStm class is introduced, but which defers to ALetStm behaviour. This is a fairly naive implementation, but all tests pass and it enables me to disambiguate. I am not sure of the wider consequences of such a change, but will submit the PR should it prove useful.

@nickbattle
Copy link
Contributor

This is strange. VDMJ's StatementReader does create a ASTDefStatement, though it just extends ASTLetStatement. So it may be a copy/paste error when Overture was created/converted.

We should probably take this opportunity to clarify what was intended by "def", since even in VDMJ it seems identical to "let".

sifraser added a commit to anaplan-engineering/overture that referenced this issue Jul 5, 2018
A ADefStm class is introduced, which defers to ALetStm behaviour, but enables disambiguation between the two.
@nickbattle
Copy link
Contributor

I've had a quick look through commit 520df5b (your ADefStm additions). It is reasonable, but there's a lot of duplication because the two AST types are "the same, but different". It is possible to get the AST generator to create a common base class with let/def as subclasses (or perhaps def as a subclass of let), in which case the caseALetStm and caseADefStm methods could just call a common method that has a parameter of the base class. I'd need to check how to get it to do this! There are several cases where this is already done though.

@nickbattle
Copy link
Contributor

Look at the way that #SimpleBlock is defined as "block" or "nonDeterministic". That produces an abstract SSimpleBlockStmBase plus ABlockSimpleBlockStm and ANonDeterministicSimpleBlockStm subclasses. So you might be able to do the same with let/def and then implement a single method with the "S" base class that the two cases call.

@sifraser
Copy link
Contributor Author

Absolutely, that commit was just a 'path-of-least-resistance' approach for me to validate that the PP would work if we had a separate def statement (it is the root cause of a number of failures in my ++ pretty printing tests).

Will see if I can put together a proper 'solution' :)

@peterwvj peterwvj added the bug Incorrect behaviour of the tool label Sep 3, 2018
@peterwvj peterwvj added this to the v2.6.4 milestone Sep 3, 2018
@peterwvj peterwvj modified the milestones: v2.6.4, v2.6.6 Oct 18, 2018
@peterwvj peterwvj modified the milestones: v2.7.0, v2.7.2 Jun 3, 2019
@peterwvj peterwvj modified the milestones: v2.7.2, v2.7.4 Sep 30, 2019
@nickbattle
Copy link
Contributor

Any update on this?

@nickbattle nickbattle added the language Issues in parser, TC, interpreter, POG or CG label Dec 20, 2019
@idhugoid idhugoid modified the milestones: v2.7.4, v3.0.2 Mar 16, 2020
@idhugoid idhugoid modified the milestones: v3.0.0, v3.0.2 Aug 28, 2020
@idhugoid idhugoid modified the milestones: v3.0.2, v3.0.4 Nov 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Incorrect behaviour of the tool language Issues in parser, TC, interpreter, POG or CG
Projects
None yet
Development

No branches or pull requests

4 participants