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

Implement bind operators #273

Closed
wants to merge 3 commits into from
Closed

Conversation

XeCycle
Copy link
Contributor

@XeCycle XeCycle commented Oct 13, 2015

Bind operators as specified by https://github.com/zenparsing/es-function-bind.

This turns off E4X accessors separately because there is a conflict; simple XML literals as used by JSX is preserved.

The option is off by default because:

1. E4X is deprecated;
2. JSX, while using inline XML literals, does not use accessors;
3. it conflicts with bind operators.
@XeCycle
Copy link
Contributor Author

XeCycle commented Oct 14, 2015

Wait some time, some changes are needed:

  • AST for a::b() is like (a)::(b()), but it should be like ((a)::(b))()
  • for the above reason, a statement like a::b() is considered to have no side effects

`js2-parse-member-expr` used to handle all 3 productions `NewExpression`,
`MemberExpression`, and `CallExpression`, the last being toggled by
`&optional allow-call-syntax`.  This commit splits up `CallExpression` from
it and rename it to `js2-parse-member-or-new-expr`, so that future changes
in the standard to these productions are easier to work with; two new
functions `js2-parse-lhs-expr` and `js2-parse-lhs-expr-tail` are added as
an API similar to the old, and the renamed `js2-parse-member-or-new-expr`
no longer parses a `CallExpression`.
`CallExpression' with optional `Arguments' at end. Unary bind is
handled specially because it is the only one not beginning with a
`NewExpression'."
(let ((pn (if (= (js2-current-token-type) js2-COLONCOLON)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this works, I wonder why it is js2-current-token-type not js2-peek-token. What is the difference?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the latter function looks at the next token?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see; so this function, similar to the previous js2-parse-member-expr, expects current token to be the start of its expression. It is now called only from js2-parse-unary-expr which does that. Seems different parse functions expect different preconditions, e.g. js2-parse-assign-expr begins with (let ((tt (js2-get-token)) ....

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct. Not sure if it's easy or beneficial to make this aspect more consistent.

@dgutov
Copy link
Collaborator

dgutov commented Oct 15, 2015

What's the status of this proposal? Can we be confident that the syntax won't change?

@XeCycle
Copy link
Contributor Author

XeCycle commented Oct 15, 2015

It is stage 0, and I am not that confident. But this is one of the 3 implemented stage 0 proposals in babel https://babeljs.io/docs/usage/experimental/ (class properties entered stage 1), and there is already a library supporting its usage https://github.com/jussi-kalliokoski/trine. I feel positive on this, but yes, further discussion needed.

@dgutov
Copy link
Collaborator

dgutov commented Oct 15, 2015

Conflict with E4X is not that big a problem (I'm not aware of anyone using it either, except for JSX).

But since the proposal is not going ahead, this is a no-go.

@dgutov dgutov closed this Oct 15, 2015
@XeCycle XeCycle deleted the bind-op-master branch October 15, 2015 14:53
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