__dir__ won't work with embed paths such as uri:classloader: (#4611) #4658
Conversation
| @@ -1781,7 +1781,8 @@ public static IRubyObject __method__(ThreadContext context, IRubyObject recv) { | |||
|
|
|||
| @JRubyMethod(name = "__dir__", module = true, visibility = PRIVATE, reads = FILENAME) | |||
| public static IRubyObject __dir__(ThreadContext context, IRubyObject recv) { | |||
| String dir = RubyFile.dirname(context, new File(context.gatherCallerBacktrace()[1].getFileName()).getAbsolutePath()); | |||
| final String __FILE__ = context.getFile(); | |||
| String dir = RubyFile.dirname(context, __FILE__); | |||
There was a problem hiding this comment.
Caller logic is needed here for methods that have jitted (which won't update context.getFile()). The rest is ok but the filename needs to be acquired using caller logic.
|
The caller logic needs to be there because jitted methods don't set context.getFile to anything. This is obviously unfortunate since it requires generating a stack trace and then mining the filename out of it. A future improvement would be to mark "file" as a frame field needed for code that calls anything like "dir". It would force a frame and force setting file so that downstream calls could pick it up. Another would be to implement dir as a pseudo-keyword, not making a call if we know it's the builtin one, so it can access the filename of the current method directly. dir would essentially compile down to PR looks good to me! |
|
Oops, I spoke too soon...I didn't realize you were removing the caller logic. So caller logic doesn't work because our compiler doesn't embed the full uri:classloader stuff as the name, or something? |
| @@ -1781,7 +1781,8 @@ public static IRubyObject __method__(ThreadContext context, IRubyObject recv) { | |||
|
|
|||
| @JRubyMethod(name = "__dir__", module = true, visibility = PRIVATE, reads = FILENAME) | |||
| public static IRubyObject __dir__(ThreadContext context, IRubyObject recv) { | |||
| String dir = RubyFile.dirname(context, new File(context.gatherCallerBacktrace()[1].getFileName()).getAbsolutePath()); | |||
| final String __FILE__ = context.getFile(); | |||
| String dir = RubyFile.dirname(context, __FILE__); | |||
There was a problem hiding this comment.
Caller logic is needed here for methods that have jitted (which won't update context.getFile()). The rest is ok but the filename needs to be acquired using caller logic.
|
ah, I see ... that's unfortunate. I'll put the |
|
|
... if
File#getAbsolutePath()is used - which seems like its not necessaryand we can pretty much just do what MRI does
File.dirname(__FILE__)wonder if caller is actually necessary in
__dir__// cc @headius @eneboas adviced in #4611 (comment)
... this is relying on the existing uri:classloader: handling logic in
RubyFile.dirnameverified this works fine for liquid gem's case presented in #4611
still need to figure out a where to put a test-case to test this properly