oracle / truffleruby

A high performance implementation of the Ruby programming language, built on GraalVM.
https://www.graalvm.org/ruby/
Other
3.03k stars 185 forks source link

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

Open numberpi opened 6 years ago

numberpi commented 6 years ago
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 commented 6 years ago

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

eregon commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

Being tracked internally as GR-4055.

eregon commented 4 years ago

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.