Skip to content

Move variable definitions to <local-def>#78

Merged
kostis merged 2 commits intokostis:masterfrom
JohnnyPol:fix/var_def
Jul 30, 2025
Merged

Move variable definitions to <local-def>#78
kostis merged 2 commits intokostis:masterfrom
JohnnyPol:fix/var_def

Conversation

@JohnnyPol
Copy link
Contributor

Refactors the placement of var definitions in function bodies so they appear in the <local-def> section at the top of the function, rather than inside the <block> of statements.

@kostis
Copy link
Owner

kostis commented Jul 29, 2025

Refactors the placement of var definitions in function bodies so they appear in the section at the top of the function, rather than inside the of statements.

And the reason to do this refactoring is what exactly? Is it because the programs without these refactoring(s) are erroneous (i.e. not allowed by the Dana language specification) or is just because "they look/feel better" ?

If it is the former, then I think you should amend the PR with some justification why they are erroneous.

If it is the latter, then I think you are missing the point for having these programs in the list of valid programs of the Dana test suite: namely to also have "somewhat weird albeit correct programs" that a correct (i.e. specification-conforming) compiler for the Dana language should better handle.

@JohnnyPol
Copy link
Contributor Author

JohnnyPol commented Jul 29, 2025

It's the former, because in Dana specs in the grammar ⟨var-def⟩ are allowed only in the following case:
⟨func-def⟩ ::= “def” ⟨header⟩ (⟨local-def⟩)∗ ⟨block⟩
⟨local-def⟩ ::= ⟨func-def⟩ | ⟨func-decl⟩ | ⟨var-def⟩
⟨var-def⟩ ::= “var” (⟨id⟩)+ “is” ⟨type⟩

not inside the block of the function. Our parser throws an error when the var definitions are inside the block, and that's why I created this and the previous Pull Request (named "Update test file").

@kostis
Copy link
Owner

kostis commented Jul 30, 2025

You are right, of course. Thanks for noticing this and for your contribution. I will merge.

@kostis kostis merged commit b8b25f4 into kostis:master Jul 30, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants