Fix scope-related regressions and await vs. class property initializers issue

[adams85]

This PR makes an attempt at solving #1338, including the regressions introduced by 9e365f7

The main idea is to set inClassFieldInit in both this and var scope when parsing the initializer part of a class property.

This way it seems to work - or at least I haven't been able to construct code that breaks it yet. Regardless, the whole approach to tracking class property initializer context doesn't feel too clean...

What do you think? Do you see an issue with this approach?

[marijnh]

I was exploring a patch that adds a scope level with a special flag around class field initializers, which feels less kludgy than the temporarily mutated inClassField property. I'm not sure I'll be able to make all the queries work in a reasonable way though. I'll fall back to your patch if my approach doesn't turn out to be viable.

[marijnh]

Patch 3221fa5 makes all of test262 pass again. Do you see any remaining issues with it?

[adams85]

AFAICS, patch 3221fa5 tackles the issue at hand but there's one thing that looks problematic:

The patch changes the semantics of currentVarScope and currentThisScope, which might cause subtle, difficult-to-see issues. For example, inFunction continues to use currentVarScope, thus it won't return the same scope in an initializer as before. But inFunction is used at numerous places in the parser, where the original behavior is still expected.

The change to the semantics of currentVarScope and currentThisScope doesn't feel good from a theoretical point of view either. Class property initializers have nothing to do with function scopes, so this change may lead to complications in the future.

It looks to me that mine is the lesser evil of the two solutions.

[marijnh]

For example, inFunction continues to use currentVarScope, thus it won't return the same scope in an initializer as before. But inFunction is used at numerous places in the parser, where the original behavior is still expected.

inFunction is used in precisely one place (parseReturnStatement) which syntactically cannot be reached in a class field initializer.

Several places that used curentVarScope/currentThisScope has to explicitly add checks for inClassFieldInit because the returned scope made no sense there. Those fall away with my patch.

Do you see any actual issue my approach causes?

[adams85]

inFunction is used in precisely one place (parseReturnStatement) which syntactically cannot be reached in a class field initializer.

You're right. I'd done a quick search for "inFunction" and got multiple matches but didn't look deeper. Apart from the place you mentioned, the rest are not relevant indeed.

So, the logic with your patch looks OK - at least, at the moment. However, I'm afraid that it makes easier to accidentally introduce bugs in the future because of the new, non-obvious behavior of curentVarScope/currentThisScope. So adding a comment describing this quirk may be a good idea.

Anyway, I'm closing this PR in favor of your patch.