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

RubyLanguage.findLocalScopes(node, null) provides all local variables in the scope ignoring node's position #1335

Open
numberpi opened this issue Apr 20, 2018 · 8 comments

Comments

@numberpi
Copy link

def halloHallo(x)
    a = 1
    b = 2
end

findLocalScopes(node, null) returns always [x, a, b] for a node inside halloHallo(x). I think this is the default implementation returning all frame slots, if frame parameter in findLocalScopes(node, frame) is null.

Is it possible to implement it for RubyLanguage (or Truffle in general?) the way SL is doing it (see SLLanguage.findLocalScopes() how it should be done by collecting only the local variables already defined).

I would appreciate this to do language agnostic static code analysis for Truffle languages.

@numberpi
Copy link
Author

see com.oracle.truffle.api.vm.DefaultScope.getVariables(root, frame)

@eregon
Copy link
Member

eregon commented Apr 20, 2018

Hello, thank you for the report.

I am not sure what you mean.
What do you expect for findLocalScopes(node, null) if called between a = 1 and b = 2?
[x, a]?

In Ruby, all local variables are defined from the start of the function, so for instance:

def halloHallo(x)
  a = 1
  p binding.eval("b") # reads local var b
  b = 2
end

halloHallo(3)

prints nil, the value of local variables until assigned.
So I think it's natural to return [x, a, b] everywhere in the method.
nil is also a valid Ruby value, and cannot easily be differentiated from the "not yet assigned variable" value.

Note that in SL, if the implementation of findLocalScopes(node, null) uses rootNode.getFrameDescriptor().getSlots(), the slots will only appear added one by one on the first execution.
For subsequent executions, I would expect all slots to already be defined.
In Truffle, the FrameDescriptior and its slots are expected to stabilize and contain all variables for all executions so far.

We should probably override findLocalScopes() to also add captured variables in blocks though, but that's a different issue.

@numberpi
Copy link
Author

numberpi commented Apr 20, 2018

In Ruby, all local variables are defined from the start of the function, so for instance:

Oh, I was not aware of this.

Yes I was expecting [x, a], but if this is not true for Ruby, everything is fine.

Note that in SL, if the implementation of findLocalScopes(node, null) uses rootNode.getFrameDescriptor().getSlots(), the slots will only appear added one by one on the first execution.

In SL, they traverse the AST until they reach the node given as argument to findLocalScopes(node, null) and collect only the local variables (via SLWriteLocalVariableNode instances) which are assigned with a value.

function main() {
  a = 1;
  b = 2;
  c = 3;
}

So in SL you get [a] when calling findLocalScopes(node, null) with node b = 2

@eregon
Copy link
Member

eregon commented Apr 20, 2018

In SL, they traverse the AST until they reach the node given as argument to findLocalScopes(node, null) and collect only the local variables (via SLWriteLocalVariableNode instances) which are assigned with a value.

Could you point to the code doing that? I didn't see it from a quick look at SL's findLocalScopes().
Sounds quite fancy, but it might make sense to do it for Ruby too for the findLocalScopes() API.
In the debugger, I think it would make sense to only show variables which have been assigned for instance.

@eregon
Copy link
Member

eregon commented Apr 20, 2018

I found it, the logic is in https://github.com/oracle/graal/blob/9fa5e821d944ee9e0c8f9422e713903b66e4d15d/truffle/src/com.oracle.truffle.sl/src/com/oracle/truffle/sl/nodes/local/SLLexicalScope.java#L253.

@chrisseaton @chumer Should we implement a similar logic in TruffleRuby to only return variables which have been assigned for TruffleLanguage#findLocalScopes()?

@chrisseaton
Copy link
Collaborator

chrisseaton commented Apr 21, 2018

We have an issue to implement findLocalScopes yes.

I've got another opinion on when local variable are 'defined' logically though:

def halloHallo(x)
  a = 1
  p defined?(a) # "local-variable"
  p defined?(b) # nil
  b = 2
end

halloHallo(3)

So I think maybe we should only return them when Ruby considers them to be defined?.

@chrisseaton
Copy link
Collaborator

Being tracked internally as GR-4055.

@eregon
Copy link
Member

eregon commented Aug 20, 2020

findLocalScopes() is implemented now, and also tracks returns variables from surrounding scopes if it's a block.
The only missing piece here is to only return the variables that have been set at that position of the method.

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