Skip to content

Fix __dir__ for eval filenames#9500

Open
samuel-williams-shopify wants to merge 1 commit into
jruby:masterfrom
samuel-williams-shopify:fix-eval-dir-relative-filename
Open

Fix __dir__ for eval filenames#9500
samuel-williams-shopify wants to merge 1 commit into
jruby:masterfrom
samuel-williams-shopify:fix-eval-dir-relative-filename

Conversation

@samuel-williams-shopify

@samuel-williams-shopify samuel-williams-shopify commented Jun 20, 2026 •
edited
Loading

Copy link
Copy Markdown

Fix Kernel#__dir__ for code evaluated with explicit filenames so relative eval filenames stay relative, matching CRuby.

Context

JRuby currently expands __dir__ for eval filenames through the normal file path handling. This differs from CRuby for code like:

instance_eval(File.read("print.rb"), "foo/bar/print.rb")

CRuby reports __FILE__ as "foo/bar/print.rb" and __dir__ as "foo/bar". JRuby already reports __FILE__ correctly, but __dir__ was expanded to an absolute path. For evals without an explicit filename, CRuby returns nil for __dir__.

Changes

  • Track whether an eval filename was synthesized by JRuby when eval is invoked without an explicit filename.
  • Carry that metadata through Binding and IREvalScript, including methods defined inside eval.
  • Update Kernel#__dir__ to use eval source metadata:
    • explicit eval filename: return File.dirname(filename) without expansion
    • synthetic eval filename: return nil
    • normal loaded files: keep the existing expansion behavior
  • Remove the now-passing __dir__ spec tags.
  • Add coverage for instance_eval with an explicit filename and a method defined inside eval.

Tophatting

Ran locally with JDK 25:

JAVA_HOME=/opt/homebrew/opt/java PATH=/opt/homebrew/opt/java/bin:$PATH ./mvnw -Dcore
JAVA_HOME=/opt/homebrew/opt/java PATH=/opt/homebrew/opt/java/bin:$PATH ./bin/jruby spec/mspec/bin/mspec ci spec/ruby/core/kernel/__dir___spec.rb
JAVA_HOME=/opt/homebrew/opt/java PATH=/opt/homebrew/opt/java/bin:$PATH ./bin/jruby -e 'p Object.new.instance_eval("[__FILE__, __dir__]", "foo/bar/print.rb")'
JAVA_HOME=/opt/homebrew/opt/java PATH=/opt/homebrew/opt/java/bin:$PATH ./bin/jruby -e 'p eval("[__FILE__, __dir__]", binding); p eval("[__FILE__, __dir__]", nil, "(eval at explicit/foo.rb)")'

@samuel-williams-shopify
Fix __dir__ for eval filenames
c2280f4 
JRuby expanded all Kernel#__dir__ source locations through File.expand_path, including filenames supplied to eval, instance_eval, and module_eval. CRuby treats explicit eval filenames differently: __FILE__ reports the filename as supplied and __dir__ returns File.dirname(filename) without expanding a relative path. For evals without an explicit filename, __dir__ returns nil.\n\nTrack whether an eval filename was synthesized by JRuby when the eval is created, carry that bit through Binding and IREvalScript, and have Kernel#__dir__ use the eval source metadata to choose the CRuby-compatible behavior. Normal loaded files keep the existing expansion path.\n\nThis also removes the old failing tags and adds coverage for instance_eval and methods defined inside eval, so the behavior survives beyond the immediate dynamic eval frame.
@samuel-williams-shopify

samuel-williams-shopify commented Jun 20, 2026

Copy link
Copy Markdown
Author

A note on the shape of this fix, since CRuby has a useful distinction here.

CRuby stores source location as both path and realpath on the iseq. Kernel#__dir__ asks for the current real filepath; for normal loaded files it gets the canonical realpath, while for eval-created code the realpath is absent so it falls back to the raw eval path. If that eval path is an implicit/synthetic location like "(eval at ...)", CRuby returns nil for __dir__; otherwise it returns File.dirname(path) without expanding it.

This JRuby patch is intentionally narrower than copying CRuby's full path/realpath model. JRuby does not currently have the same iseq pathobj structure in this path, so the patch preserves existing behavior for non-eval files and only adds the missing eval distinction:

  • non-eval code keeps the existing load-service/path expansion behavior;
  • explicit eval filenames are treated as raw source paths and __dir__ returns dirname(filename) without expansion;
  • implicit eval filenames are marked as synthetic and __dir__ returns nil.

The fileNameSynthetic boolean is therefore not meant to encode "path vs realpath" generally. It only records whether JRuby invented the eval filename because the caller did not provide one. Being in an IREvalScript with a non-synthetic filename is what corresponds to CRuby's "eval path with no realpath" case.

I think that keeps this as a targeted compatibility bug fix rather than a broader source-location refactor, while still matching CRuby's observable semantics for __dir__ in these cases.

@headius

headius commented Jul 10, 2026

Copy link
Copy Markdown
Member

Ignoring for the moment that it didn't pass specs, I'll respond to the changes...

JRuby does not currently have the same iseq pathobj structure in this path, so the patch preserves existing behavior for non-eval files and only adds the missing eval distinction:

This is perhaps a good short-term fix but I actually want to get full and relative paths both propagated through the AST into the JRuby IR and JIT output. There's a number of reasons for this, but probably the biggest would be to make it easier for JVM debuggers to find loaded files based on the compiled code; currently we embed a full path, which most debuggers will ignore (they can't can the entire system for sources).

The other compatiblity issues (and __dir__ in eval is just one of them.

I'll review the changes separately.

headius
headius reviewed Jul 10, 2026
View reviewed changes

@headius headius left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is fine as a workaround for the lack of relative+full path information in the AST and IR, but I would want to push toward adding that information and eliminating this logic in the future.

The failing spec appears to be one added specifically for this PR, so it appears there's a bug there in the spec or the code.

(The stdlib failures are known-to-be-flaky tests)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@headius headius headius left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@samuel-williams-shopify @headius