Define statements are parsed as let statements

[sifraser]

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]

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".

[nickbattle]

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]

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]

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' :)

[nickbattle]

Any update on this?