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 a ScopeAnalyzer #1200

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Add a ScopeAnalyzer #1200

wants to merge 3 commits into from

Conversation

dcodeIO
Copy link
Member

@dcodeIO dcodeIO commented Mar 29, 2020

This adds a little ScopeAnalyzer doing a pre-pass on functions, analyzing variable scopes within. Will be helpful to tackle #1190 in a more robust way as well as to obtain the information necessary to compile closures. So far all it does is generate diagnostics when something is redeclared that shouldn't be redeclared, and hook into the parser tests to make it testable.

One important implementation criteria is that this works with programmatically generated ASTs, so can't be part of the parser. Not quite happy with the _scope properties on the AST nodes yet, since it would be much nicer if we could serialize the information.

@MaxGraey
Copy link
Member

I wonder is it open possibilities for doing some ARC early optimizations as well? Or this require Escape Analysis?

@dcodeIO
Copy link
Member Author

dcodeIO commented Mar 30, 2020

Not sure, since it neither knows the types nor what exactly a name is referencing after serializing into the AST. The real deal would be a proper checker that does all of this, hmm.

@dcodeIO
Copy link
Member Author

dcodeIO commented May 6, 2020

Since this PR has been mentioned a couple times recently, perhaps a few notes on why I think it isn't feasible yet. The information it provides currently is "vars bound to this scope", but not "vars not bound to this scope" or "captured by this function". The latter would allow us to prepare captured locals early, in the outer function, while the former triggers too late, that is when a captured variable is first accessed in the inner function. As such this isn't easily forward-compatible to mutable captures.

@dcodeIO
Copy link
Member Author

dcodeIO commented May 16, 2020

Keeping this PR open as part of 2020 vacuum. Even though it will most likely not make it in its current form, it might still be a base for a better implementation.

@dcodeIO dcodeIO marked this pull request as draft May 16, 2020 19:09
@dcodeIO dcodeIO removed the vacuumed label May 28, 2020
@DuncanUszkay1
Copy link
Contributor

Since this PR has been mentioned a couple times recently, perhaps a few notes on why I think it isn't feasible yet.

Is it not feasible or just not yet done? Could we alter the behaviour of the ScopeAnalyzer to capture when a variable refers to a name from an outer block scope? If not, do you have in mind what would be required to do so?

I'd like to move this forward as the first step towards Closure implementation but I'm not sure if I should use this as a base or if there's something fundamental about it's design that prevents it from detecting enclosed variables that you're referring to in this comment

@dcodeIO
Copy link
Member Author

dcodeIO commented Sep 7, 2020

The issue with this is that the evaluation happens too early, when not all the information closures depend on is available yet (all we know is abstract scopes). The worst case scenario where this fails is a compile time check within a function, where some branches become eliminated and a local may or may not be closed over in the final function, or there's even an inner closure in a closure within such a branch. As such I guess that it'll be necessary to figure out closure-ness during compilation of a function, potentially recompiling if a made assumption did not hold, while propagating that an env is necessary to the enclosing function, which needs to share the environment.

@DuncanUszkay1
Copy link
Contributor

The worst case scenario where this fails is a compile time check within a function, where some branches become eliminated and a local may or may not be closed over in the final function, or there's even an inner closure in a closure within such a branch.

Is it possible to run a pass which performs that elimination before the scope analysis and real compilation? Or are these compile time checks not independently resolvable?

I'm not against a recompilation strategy but it seems like eliminating that problem would create easier to follow code 🤔

@dcodeIO
Copy link
Member Author

dcodeIO commented Sep 7, 2020

We don't know the values used in static checks without "compiling" the code, i.e. we can precompute partial expressions. Compiling in this sense means generating Binaryen IR, not necessarily Wasm, which isn't as expensive as it sounds.

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.

None yet

3 participants