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

Add type mechanism for single value list expressions #70

Closed
rdvdijk opened this issue Jul 18, 2016 · 4 comments
Closed

Add type mechanism for single value list expressions #70

rdvdijk opened this issue Jul 18, 2016 · 4 comments
Assignees
Milestone

Comments

@rdvdijk
Copy link
Contributor

rdvdijk commented Jul 18, 2016

Examples: In Def and Nod the size expression needs to evaluate to a list with a single value. Parsing will fail otherwise. RepN needs a list with a single value for its n argument.

Is there a way to enforce this using the type system?

jvdb added a commit that referenced this issue Jan 26, 2019
…on that returns a single (optional) Value. Applied it to a small set of cases.
@jvdb jvdb self-assigned this Jan 26, 2019
@jvdb
Copy link
Contributor

jvdb commented Jan 27, 2019

In 94a8d2a I have added a SingleValueExpression interface that does not return a list but a single Optional<Value>. I have created it as a subclass of the ValueExpression interface, so that it can be used wherever a ValueExpression is required. A default method is present to convert to a list whenever needed.

Some things to consider:

  1. It works quite well: almost all modifications to the test code are in order to be more precise when specifying a ValueExpression with a single value, which means this change actually makes the type system help us when creating a Token.
  2. Currently only Last and Const have been converted as well as the size argument to Def. There are many more candidates, but in order to consider this change, a minimal set seems useful.
  3. Other candidates for conversion if we want to make this permanent are, among the ValueExpressions, Count, CurrentIteration, CurrentOffset, First, Self, as well as all the Fold implementations. The RepN Token's n argument as well as some of the ValueExpression arguments of the While Token can be considered for conversion as well.
  4. Error-prone complains about the @FunctionalInterface annotation since it may give surprising behaviour when casting lambda expressions. I've suppressed it for now since I believe our default method's behaviour is in line with their best practice, but I'm not entirely sure.
  5. One fundamental change is that when using a SingleValueExpression where a ValueExpression is required, there is no distinction between an empty list and a list containing a single empty value. In fact, using the default method to handle ValueExpression behaviour, an empty list is never returned. If this is a big problem the SingleValueExpression can override the eval() method, but that may be cumbersome. Right now I'm not even entirely sure if it's a problem if we lose this distinction.

@rdvdijk
Copy link
Contributor Author

rdvdijk commented Jan 27, 2019

Looks pretty good!

One thing, code like this is becoming a bit weird to look at (pasted from a test) :

def("d", last(len(last(ref("c")))))

We might want to introduce Shorthands for these def cases, where in the current code we would not need a last around a len (for example).

@ccreeten
Copy link
Collaborator

ccreeten commented Jan 28, 2019

But len would also become a single value expression, right? But for other cases, another option would be to introduce shorthand overloads for expressions instead, e.g.:

ValueExpression offset(ValueExpression)
SingleValueExpression offset(SingleValueExpression)

For full support of compiler.

In any case, this is looking good 😄

@jvdb jvdb pinned this issue Feb 3, 2019
jvdb added a commit that referenced this issue Feb 11, 2019
…f Def to SingleValueExpression, along with all dependencies.
jvdb added a commit that referenced this issue Feb 11, 2019
jvdb added a commit that referenced this issue Feb 11, 2019
jvdb added a commit that referenced this issue Feb 11, 2019
jvdb added a commit that referenced this issue Feb 12, 2019
jvdb added a commit that referenced this issue Feb 17, 2019
jvdb added a commit that referenced this issue Feb 17, 2019
…tional-list

Change return value of ValueExpression.eval(...) to ImmutableList<Value> (#279), extract Value interface and create dedicated NotAValue class and instance (#280), implement SingleValueExpression to let the compiler help more when writing incorrect Metal tokens/expressions (#70).
@jvdb
Copy link
Contributor

jvdb commented Feb 17, 2019

#283 is merged, so issue closed (no autoclose it seems because it was merged through another branch/PR first...)

@jvdb jvdb closed this as completed Feb 17, 2019
@jvdb jvdb unpinned this issue Feb 17, 2019
@jvdb jvdb added this to the 8.0.0 milestone Feb 17, 2019
jvdb added a commit that referenced this issue Feb 7, 2021
…lueExpression. Discovered when preparing for #285.
jvdb added a commit that referenced this issue Feb 7, 2021
jvdb added a commit that referenced this issue Feb 10, 2021
…lueExpression. Discovered when preparing for #285.
jvdb added a commit that referenced this issue Feb 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants