Skip to content

Rework resolver infrastructure #473

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

Closed
dcodeIO opened this issue Feb 7, 2019 · 7 comments
Closed

Rework resolver infrastructure #473

dcodeIO opened this issue Feb 7, 2019 · 7 comments
Assignees

Comments

@dcodeIO
Copy link
Member

dcodeIO commented Feb 7, 2019

There are multiple issues outlining problems when trying to resolve members (of namespaces), both from within and from outside static scopes. This is caused by our lookup maps not being easily traversable respectively not including aliases for all possible scenarios.

So, this is a tracking meta issue for all those kinds of problems that should most likely be solved by reworking how our IR is linked with proper nested lookups, possibly also transitioning to symbol tables to avoid having to hash strings over and over again.

There are also multiple other issues that result from a different limitation of the resolver (can't resolve every kind of expression without first compiling it):

The latter are not directly tracked here but mentioned in case it turns out that there are ways to tackle them in one go.

@dcodeIO
Copy link
Member Author

dcodeIO commented Feb 20, 2019

Alright, the first batch of resolver rework is merged now. This fixes #423, #144, #127 and potentially #429. Symbol tables have been prepared, but not yet switched to symbols yet.

The other issues mentioned haven't been solved, yet, and remain to do for follow-up PRs.

@vgrichina
Copy link

@dcodeIO what's proper way to determine if DeclarationStatement is top-level export after these changes?

@dcodeIO
Copy link
Member Author

dcodeIO commented Feb 21, 2019

Maybe something like:

if (declaration.is(CommonFlags.EXPORT)) {
  let source = declaration.range.source;
  if (source.isEntry && source.statements.includes(declaration)) {
    ...
  }
}

Doesn't cover exports in exported namespaces though, that'd need traversal. Also doesn't catch variable declarations as these are wrapped in a top-level VariableStatement. And doesn't catch export { someDeclaration }. Actually doesn't cover quite a lot. Might make sense to provide a traverse(node).on(...) helper or something.

@vgrichina
Copy link

BTW, what is MODULE_EXPORT vs EXPORT difference?

@vgrichina
Copy link

Looks like #429 not fixed yet, I'll try to give repro later.

@dcodeIO
Copy link
Member Author

dcodeIO commented Feb 21, 2019

The EXPORT flag represents the presence of an export token on the declaration, while the MODULE_EXPORT flag marks everything that becomes an actual module export, i.e. to apply proper integer wrapping on exported WASM functions. But: The latter flag is applied on IR right after program initialization is complete (not AST) and not yet available in the parsing stage. An afterInitialize hook that is called once program IR is set up might help, that would have been my next suggestion, but except full information there's not much that can be modified at this stage in the compilation progress.

@dcodeIO dcodeIO mentioned this issue Feb 26, 2019
@dcodeIO dcodeIO added enhancement and removed bug labels Feb 28, 2019
@dcodeIO dcodeIO unpinned this issue Feb 28, 2019
@MaxGraey
Copy link
Member

MaxGraey commented Mar 9, 2020

@dcodeIO it seems all issues in top message already solved. But I understand that improvements on resolver still going on?

@dcodeIO dcodeIO closed this as completed Mar 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants