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

Hoisting variables #376

Closed
hello2dj opened this issue May 7, 2020 · 5 comments · Fixed by #392
Closed

Hoisting variables #376

hello2dj opened this issue May 7, 2020 · 5 comments · Fixed by #392
Assignees
Labels
bug Something isn't working
Milestone

Comments

@hello2dj
Copy link
Contributor

hello2dj commented May 7, 2020

now not founding Identifier will always be undefined, for function or var this is wrong.

moreinfo

example:

// v8
console.log(A); // [Function: A]
function A() {}
// boa
console.log(A); // undefined
function A() {}
@Razican Razican added the bug Something isn't working label May 7, 2020
@Razican
Copy link
Member

Razican commented May 7, 2020

This seems to be related to the fact that we execute nodes in order, instead of having a special category for this. These functions should be evaluated first, and then start evaluating the rest of statements.

This was referenced May 7, 2020
@Razican Razican self-assigned this May 14, 2020
@Razican Razican added this to the v0.9.0 milestone May 14, 2020
@Razican Razican linked a pull request May 14, 2020 that will close this issue
32 tasks
@jasonwilliams
Copy link
Member

I don't think v0.9.0 is realistic for this, for hoisting to work would require a big refactor of how execution works, including having some pre-parsing step.

We're no where near that point yet.

@Razican
Copy link
Member

Razican commented May 18, 2020

I don't think v0.9.0 is realistic for this, for hoisting to work would require a big refactor of how execution works, including having some pre-parsing step.

We're no where near that point yet.

Actually, this seems to be already working in #392. The Idea was to re-order function declarations to show before all the rest of the nodes in the node list. Not sure what else should be done.

@jasonwilliams
Copy link
Member

oh ok interesting.
Should we be doing something more closely related to https://v8.dev/blog/preparser#variable-allocation? is reordering sound?

@Razican
Copy link
Member

Razican commented May 18, 2020

Should we be doing something more closely related to https://v8.dev/blog/preparser#variable-allocation?

Definitelly, but I think it's a reasonable workaround in the meantime.

is reordering sound?

It should be, it's slower, as we will be doing a sort, but it's always maintaining ordering, so it's sound as far as I can tell.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants