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 back custom operators #517

Closed
disnet opened this issue Mar 28, 2016 · 14 comments · Fixed by #665
Closed

Add back custom operators #517

disnet opened this issue Mar 28, 2016 · 14 comments · Fixed by #665

Comments

@disnet
Copy link
Member

disnet commented Mar 28, 2016

Pre 1.0 had the ability to define custom operators with precedence and associativity. It would be nice to have that back. Some syntax ideas:

operator ** 14 right = function (left, right) {
  return #`Math.pow(${left}, ${right})`;
}

Most of the plumbing for this features is already done. Just need to decide on the right syntax.

@vendethiel
Copy link

How to define unary operators with this syntax?

@gabejohnson
Copy link
Member

@disnet are you thinking of hard-coding this into the enforester/macro-context or implementing it as a macro? It seems like operators and infix macros require some of the same features (access to a prefix of tokens).

@GuillaumeLeclerc
Copy link

This is a very important feature in my opinion. It would be good to have it

@dmitriz
Copy link

dmitriz commented Dec 27, 2016

@disnet That sounds like very useful feature. Are there any reference to the old version supporting it or other projects?

@disnet
Copy link
Member Author

disnet commented Dec 27, 2016

@dmitriz Sure, the old documentation is available at http://sweetjs.org/doc/main/sweet.html

@gabejohnson
Copy link
Member

@disnet I just got another question about this on Gitter. Do you want to have a design discussion about it?

@disnet
Copy link
Member Author

disnet commented Jan 21, 2017

@gabejohnson absolutely, do you have some design ideas kicking around? I haven't had the chance to think about it deeply in a while.

@mooreniemi
Copy link

Looks like this has been hanging out for about a year now. Anything I can do to help it land?

@disnet
Copy link
Member Author

disnet commented Feb 23, 2017

@mooreniemi if you'd like to dive in that would be awesome! Nothing has been blocking it, just not on top of my stack.

There should be two pieces to getting this landed. One is the syntax design and the other is wiring it up.

For wiring it up you need to:

  • make the enforester recognize the new special form (like how it currently does with syntax)
  • add the operator definition to the compile time environment (similar to here)
  • look in the environment when enforesting operators for any custom definitions (should be happening here
  • and then using the custom definition, essentially do a macro expansion instead of combine

For the syntax design it sounded like @gabejohnson had some ideas?

@gabejohnson
Copy link
Member

gabejohnson commented Feb 23, 2017

@mooreniemi Any help would be welcome!

As far as design goes, I want to make sure we don't introduce new primitive forms unnecessarily. And I can't shake the feeling that this has to tie in to infix operators.

I know operators are some of the more complex forms to parse, but if we exposed Enforester#opCtx in MacroContext. I think we could do this by making an operator macro.

operator foo (left, right, associativity='left', precedence=13) {
  // left is already enforested at this point as it will be with infix macros
  return #`${op}(${left}, ${right.expand('Expression')})`;
}

The operator context stack juggling could be done in the definition of the operator macro. associativity and precedence in the above don't even have to be used in the defined transform function. They can just be plucked out by the macro definition.

There would be a lot of duplicate code b/w the macro and the enforester unless we were able to leverage the enforester or have all operators dispatch to macros. See the bottom of this #516 (comment). Though that might slow things down too much.

As to defining something like:

operator |> (...) {
  ...
}

It would be my preference starting out just defining the new punctuator w/ readtables. The only other reasonable way to do it, to my mind, would be to have the whole pipeline be lazy and extend the readtable on the fly.

Whatever happens @disnet, I think getting infix macros done first makes the most sense as I think they're a superset of all other syntactic extensions.

@disnet
Copy link
Member Author

disnet commented Feb 26, 2017

@gabejohnson so I started writing a long post saying why I thought that wouldn't work but then realized it actually kinda would and that it's super easy to add infix macros. So #643.

I'm still not entirely sure I'm comfortable with exposing opCtx to macros. I much prefer keeping internals internal so as to maintain invariants. Also speed, as you noted making every operator a macro invocation is probably not great. My guess is that having a custom operator special form is the best approach.

But we can totally experiment.

@gabejohnson
Copy link
Member

gabejohnson commented Feb 27, 2017

@disnet I don't want to beat a dead horse here but I'd still want like to avoid creating a single purpose special form (if possible).

Consider this:

syntax m: { precedence: 13, associativity: 'left' } = ctx => {
  ...
}

This syntax would put additional information on the BindingIdentifier that will get passed to the CompiletimeTransform instance. This information would be used during enforestation to determine whether we're entering into or remaining in the operator parsing machinery.

I realized that in my example above...

operator foo (left, right, associativity='left', precedence=13) {
  // left is already enforested at this point as it will be with infix macros
  return #`${op}(${left}, ${right.expand('Expression')})`;
}

...that conflating local, runtime binding syntax w/ what were essentially compile-time bindings was misguided. But there is already precedent in JS for compile-time bindings using this syntax (i.e. Flow, TypeScript).

A side benefit is that this syntax could be adapted in the future for type hints if we so desire (in this case a parameterized one, Operator(13, left)). To make that useful for the user, however, we would need to attach this information to BindingIdentifiers whenever it is encountered. Then we strip it out during term reduction.

It might make sense to do this work at the same time that we implement your proposal in #620 (if we go forward with that).

@disnet
Copy link
Member Author

disnet commented Feb 28, 2017

I'll have to stew on it for a while but I think I like your proposed syntax.

We could also throw in a # somewhere to signal "syntax things are happening":

syntax m: #{ precedence: 13, associativity: 'left' } = ctx => {
  ...
}

@gabejohnson
Copy link
Member

gabejohnson commented Feb 28, 2017

We could also throw in a # somewhere to signal "syntax things are happening"

I don't think it adds any information that isn't already provided by syntax. Unless you mean to signal the enforester which I think we do by just providing the data on the BindingIdentifier. In fact, I don't think it has to be primitive. We could implement this as a macro if we can extend existing terms w/ additional data.

disnet added a commit that referenced this issue Apr 3, 2017
gabejohnson pushed a commit that referenced this issue Apr 5, 2017
* Implement custom operators; fixes #517

* Add support for postfix custom operators
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants