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

yield in nested function should be parsed as identifier #552

Closed
RReverser opened this issue May 20, 2017 · 5 comments
Closed

yield in nested function should be parsed as identifier #552

RReverser opened this issue May 20, 2017 · 5 comments

Comments

@RReverser
Copy link
Member

RReverser commented May 20, 2017

function *f1() {
  function g() {
    return yield / 1;
  }
}

function *f2() {
  () => yield / 1
}

function *f3() {
  ({
    g() {
      return yield / 1;
    }
  })
}

and other variations currently cause "Unterminated regular expression".

This is due to us (#525 cc @marijnh) trying to extend Sweet.js algorithm to recognise yield as operator vs yield as an identifier at tokenizer level, but, after playing around, I'm not sure it's possible to do that correctly for all variations of nested functions.

@marijnh
Copy link
Member

marijnh commented May 20, 2017

It should be possible to further refine the token context tracking to accurately track what kind of function is currently is in, and to have the loop in inGeneratorContext stop at the first function context it finds.

@RReverser
Copy link
Member Author

RReverser commented May 20, 2017

For the normal functions, sure, but how would we track arrow functions, for example? Even in full-featured parser it requires quite a bit of hacks to detect them, and here

() => a + b - yield / 1

we would somehow need to figure out if we are still in the body of arrow function expression or not.

I'm somewhat afraid of tokenizer getting bloated with things that were earlier needed only for parser, but if you see a clean way to do that, I'd be happy to take a look.

@marijnh
Copy link
Member

marijnh commented May 20, 2017

Oh, right, it'd also have to work for brace-less arrow bodies. Yeah, that might be too difficult to get right. That's a shame, though.

(I noticed Esprima has a half-hearted, largely broken implementation of this algorithm, but only uses it when tokenizing without parsing. That seems a bit dodgy. Sweet.js itself has been upgraded to ES6, but fails on more corner cases than Acorn, so there's not much to be learned from that either.)

I'm not sure which would be worse, not supporting pure tokenization at all, or supporting it but having it break on corner cases like this.

@RReverser
Copy link
Member Author

Agree... guess let's keep this issue open for now and try "best effort" approach to support at least contexts which we can easily recognise.

@marijnh
Copy link
Member

marijnh commented Sep 11, 2018

That solution was backed out again due to being problematic, so I'm reopening this.

@marijnh marijnh reopened this Sep 11, 2018
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

No branches or pull requests

2 participants