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

Parser: How to know the block-local variables #4181

Closed
eregon opened this issue Sep 26, 2016 · 4 comments
Closed

Parser: How to know the block-local variables #4181

eregon opened this issue Sep 26, 2016 · 4 comments
Milestone

Comments

@eregon
Copy link
Member

eregon commented Sep 26, 2016

@enebo @headius
How does the AST carry the list of block-local variables?
Say the following code:

[1].each {|a,b,c; bl| bl = :in }

At some point @chrisseaton stored it e6da2d8 in the ArgsNode, but it seems @enebo's merge killed in 8d092c6 that code. Interestingly GitHub seems to hide part of the diff but a git log -m -p 8d092c6 and search for setBlockLocalVariables will show it.
Was it intentional to remove these calls?

What does JRuby use to get block-local variables?
The ArgsNode has no reference to it, the only way I see is through
the IterNode's StaticScope, by the mean of looking at variableNames and noticing that the 4th entry belongs to none of the Signature indicies. Surely, there must be a better way?

@headius
Copy link
Member

headius commented Sep 26, 2016

@enebo will have to answer with details, but I can say a few things.

This isn't really part of the arg list (or the signature of the block) at all. I believe it's handled in the parser by just forcibly declaring that variable name at the current level (depth 0), so it is from then on handled as local to the block. The fact that it was declared such is not directly reflected in any of the final parse artifacts. If it overlaps with an enclosing scope, it will just show up in the StaticScope hierarchy as an overlapping variable declaration. If it doesn't overlap, it may not be possible to tell a variable was declared this way.

@enebo
Copy link
Member

enebo commented Sep 26, 2016

@eregon I don't think it was intentionally removed but it maintains a a field and optionally a whole arraylist per argsnode. Memory is a huge problem for us so I guess if you really need to be able to determine this style of variable (we do not as we depend on StaticScope for this) we could maybe make a specialized ArgsNode so we only pay any extra memory cost when this feature is actually used.

@eregon
Copy link
Member Author

eregon commented Sep 26, 2016

@enebo I'm not quite following.
How can StaticScope be used properly to detect such variables?
Adding all variables from variableNames in the block scope seems to break tons of specs, and guessing the index only works in a debugger :p
Could we maybe add a method String[] getBlockLocalVariables() in StaticScope, is that possible?

One could add the variable when noticing a DAsgnNode with depth==0, but that seems a very indirect way too.

@eregon
Copy link
Member Author

eregon commented Sep 26, 2016

@enebo I ended up using the StaticScope, so feel free to remove blockLocalVariables-related things in ArgsNode, since it's never used anyway 😉

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

3 participants