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

Added syntactic sugar for variable introduction #339

Merged
merged 1 commit into from
Mar 7, 2016
Merged

Added syntactic sugar for variable introduction #339

merged 1 commit into from
Mar 7, 2016

Conversation

EliasC
Copy link
Contributor

@EliasC EliasC commented Mar 1, 2016

This commit adds the following rule to the desugaring stage:

{
  let x = e1;
  e2;
  ...;
  en;
}

=>

{
  let x = e1 in {e2; ...; en}
}

This lets you add variables in a scope (seemingly) without introducing a
nested scope. The rule works left to right, so if there are more lets
in e2; ...; en, these will also be unfolded. An example of what you
can write now is in the test miniLet.enc.

This commit adds the following rule to the desugaring stage:

```
{
  let x = e1;
  e2;
  ...;
  en;
}

=>

{
  let x = e1 in {e2; ...; en}
}
```

This lets you add variables in a scope (seemingly) without introducing a
nested scope. The rule works left to right, so if there are more `let`s
in `e2; ...; en`, these will also be unfolded. An example of what you
can write now is in the test `miniLet.enc`.
@EliasC
Copy link
Contributor Author

EliasC commented Mar 1, 2016

I developed this when working on a completely different thing, but I thought others might be interested in using it as well. See it as a workaround before the long-awaited syntax revamp 😉

@kaeluka
Copy link
Contributor

kaeluka commented Mar 1, 2016

The PR is documented, tested. The code is focused.

My vote: merge.

@albertnetymk
Copy link
Contributor

Generally, dropping the braces for single expr would be a no-op, but doing so for the following program fails for me. It's not clear from the doc if this is the intended behavior:

class Main
  def main() : void {
    let x = 0
  }                                               

@kaeluka
Copy link
Contributor

kaeluka commented Mar 1, 2016

This should fail, as the expression you're desugaring to is not well formed. It lacks a body.

Even if you don't know about the desugaring: it's ok that this fails, as you're not using the variable.
An improvement would be to infer ; () in your case at the end, or to make it clear in the docs.

@EliasC
Copy link
Contributor Author

EliasC commented Mar 1, 2016

@albertnetymk: I think this is reasonable behaviour as it is unclear what the program

class Main
  def main() : void
    let x = 0 

would mean. The documentation mentions that this is a special case. Also, the error message

unexpected end of input
expecting identifier or "in"

is pretty informative to how one could get the program working.

On a more pragmatic note, this is the version I threw together for my other project, and it keeps the interference with the rest of the parsing code small.

@albertnetymk
Copy link
Contributor

@kaeluka I get the feeling that you misunderstood my comment. Actually, the program I posted doesn't fail. It fails only after I drop the braces.

I don't think mentioning it as a special case is convincing enough to expose this behavior. The thing concerns me the most is that it breaks the consistency that braces are optional for single expr, but, apparently, whether this is an issue or not is quite subjective as well.

@EliasC
Copy link
Contributor Author

EliasC commented Mar 1, 2016

@albertnetymk: Would it be enough to add something like "this notation is only useable in a sequence (i.e. between curly braces)"?

@EliasC
Copy link
Contributor Author

EliasC commented Mar 6, 2016

Ping @albertnetymk.

@albertnetymk
Copy link
Contributor

@EliasC Sorry. I was mostly focusing on the migration.

With the merging of this PR, the aforementioned inconsistency might bite some poor soul writing Encore code, but considering the syntax of Encore is evolving rapidly, introducing this inconsistency may not be so catastrophic.

Therefore, based on pure pragmatical reason, I have no objections on merging this PR.

@albertnetymk
Copy link
Contributor

I would merge this after lunch, if no other opinions from others.

@kaeluka
Copy link
Contributor

kaeluka commented Mar 7, 2016

👍

albertnetymk added a commit that referenced this pull request Mar 7, 2016
Added syntactic sugar for variable introduction
@albertnetymk albertnetymk merged commit a1e7109 into parapluu:development Mar 7, 2016
@EliasC EliasC deleted the features/minilet branch December 21, 2016 10:45
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.

4 participants