Feature #22212: Add Thread::Backtrace::Location#source_range

• Status: Closed
• Assignee: Eregon (Benoit Daloze)
• Target version: 4.1

[Eregon (Benoit Daloze)]

Motivation

The main motivation is to be able to implement Prism.find(Thread::Backtrace::Location),
and similar use cases which need to locate or extract the source code associated with a Thread::Backtrace::Location,
precisely and cleanly on any Ruby implementation, in a way which does not depend on implementation details like node_id.
For that we need the start/end line/column and the absolute_path, which is exactly what Ruby::SourceRange provides.

In #21998 we added source_range for {Method,UnboundMethod,Proc}.

However we also need source_range for Thread::Backtrace::Location as this is used in error_highlight, power_assert, etc.
Those tools currently use RubyVM::AbstractSyntaxTree.node_id_for_backtrace_location(Thread::Backtrace::Location), however:

• that's a CRuby-only experimental API
• it exposes CRuby internals (node_id)
• it adds a lot of complexity for all usages because they need to handle both Prism::Node and RubyVM::AbstractSyntaxTree::Node.

We solve all of this by adding a new portable API which exposes universal concepts like line and column, which are stable to find AST nodes.

I also know this API can be implemented on other Ruby implementations, notably on TruffleRuby without needing node_id, which illustrates its generality and portability.

Implementation

How can we get the start/end line/column when we only keep the start line in CRuby?
By re-parsing, and specifically by re-parsing with exactly the same parser that the interpreter used to compile to bytecode.
That way there is no issue of using different Prism versions, it's the Prism C library built in CRuby, or parse.y in --parser=parse.y mode.
Either way, we reparse to extract the start/end line/column.
We make the returned start/end line/column match for both Prism and parse.y, in practice the only difference is for calls with blocks, where we adapt the location from parse.y to match Prism.
(the fact the lines & columns match for both parsers for all cases except blocks also illustrates the stability of those locations)

One can then easily use Prism to get a node matching that Ruby::SourceRange and use the resulting Prism::Node as they wish.
This also means all versions of Prism can be used, and for example if some dependency requires some specific version of Prism it works fine and is not a problem.
As a result, the user of this API can choose the Prism version they want and always get a Prism::Node (and not having to handle RubyVM::AbstractSyntaxTree::Node too, or a duplicate Ruby::Node API as mentioned in #21795).
Other users of this API can also directly use the Ruby::SourceRange start/end line/column to e.g. display/highlight/link-to that source code range, without creating a Prism::Node AST.

PR: https://github.com/ruby/ruby/pull/18043

I also made a PR to update ErrorHighlight to use this new API and show how well it works: https://github.com/eregon/ruby/pull/2
This in turn could be a major step towards getting ErrorHighlight to work on other implementations than CRuby.

[Eregon (Benoit Daloze)]

• Related to Feature #21795: Methods for retrieving ASTs added

[Eregon (Benoit Daloze)]

• Related to Feature #21998: Add {Method,UnboundMethod,Proc}#source_range added

[Eregon (Benoit Daloze)]

• Description updated (diff)

[Eregon (Benoit Daloze)]

• Description updated (diff)

[Eregon (Benoit Daloze)]

• Description updated (diff)

[Eregon (Benoit Daloze)]

• Description updated (diff)

[mame (Yusuke Endoh)]

Why not just use Backtrace::Location#syntax_tree?

As the maintainer of error_highlight, I have no plan to use this method. I will use Backtrace::Location#syntax_tree. Therefore, the use cases for this feature should be explained separately.

[Eregon (Benoit Daloze)]

I understand that, as the maintainer of error_highlight, you do not plan to use Thread::Backtrace::Location#source_range.
However, I do not think #syntax_tree can currently be considered a reliable alternative to this proposal.

The proposed #syntax_tree takes a node_id produced by the parser embedded in CRuby and looks up that ID in a newly parsed syntax tree.
When Prism is used, that syntax tree may be produced by a different version of the Prism gem.

node_id is not stable across Prism versions.
It reflects Prism's internal node allocation order, including temporary nodes which never appear in the resulting tree.
Normal parser refactoring and optimizations can therefore change subsequent node IDs without changing the source or even the public AST shape.
This is explained with concrete examples in this analysis and summarized in this comment.

The same cross-version node_id lookup has already caused a silent incorrect result.
The reproduction in this comment asks for the node corresponding to method(:b), but receives the DefNode for a.
And when asking the node corresponding to method(:a), it returns an IfNode.
A source hash cannot detect this because the source bytes are unchanged.
A warning also does not prevent the method from returning the wrong node.

This is precisely the correctness issue Matz identified: the parser that interpreted the program and the parser returning the syntax tree must be the same.
He explicitly opposed adding the syntax-tree methods until that identity is guaranteed.
The current #syntax_tree prototype does not guarantee it and can therefore return exactly the kind of incorrect node he was concerned about.
Emitting a warning does not satisfy that requirement because the method still returns an incorrect value.

Independently, #syntax_tree has no consistent return type.
It returns a Prism::Node under --parser=prism, but a RubyVM::AbstractSyntaxTree::Node under --parser=parse.y, as discussed in this comment.
These are substantially different APIs with no common node interface.
Every caller would need two implementations depending on a CRuby parser option.
That largely defeats the purpose of exposing this as a core abstraction.

#source_range deliberately provides a smaller, parser-independent contract.
CRuby uses the same parser which compiled the ISeq to translate its internal node_id into source coordinates.
A different Prism version never interprets that internal ID.
The result is always a Ruby::SourceRange (or nil for native code), works with both CRuby parsers, and can be implemented consistently by other Ruby implementations.

It also does not claim to return a syntax tree corresponding exactly to the bytecode.
It returns source coordinates.
A consumer such as Prism.find can then use those coordinates with its chosen Prism version without trying to interpret an internal identifier (node_id) produced by a different parser version.

Therefore I think the use cases for #source_range should be evaluated independently of whether error_highlight adopts it.
error_highlight demonstrates one practical use, but it is not the only one.
Independently of error_highlight, this is a public and portable replacement for the CRuby-specific RubyVM::AbstractSyntaxTree.node_id_for_backtrace_location.
It allows Prism.find and other source-based tools to identify the source expression associated with a backtrace location without exposing or interpreting a parser-specific node_id.

For these reasons, the current #syntax_tree prototype does not eliminate the need for the smaller and more portable #source_range API.

---

I also want to clarify that ideally I would like an API such as #syntax_tree.
However, the discussion in #21795 showed that the current design cannot provide it reliably without first solving the parser-identity and inconsistent-return-type problems.
Hence this proposal, which provides a smaller, well-defined contract and avoids those problems.

[Eregon (Benoit Daloze)]

Because that reply is long I'll also make a concise reply regarding use cases:

This API is useful to any tool or gem that needs to locate or extract the source code associated with an exception's backtrace_locations (or caller_locations).

It does so without requiring the Prism gem or constructing Prism::Node objects, thereby separating the concern of locating code in the source from parsing that code into an AST.

[Eregon (Benoit Daloze)]

• Description updated (diff)

[Eregon (Benoit Daloze)]

• Description updated (diff)

[Eregon (Benoit Daloze)]

• Description updated (diff)

[Eregon (Benoit Daloze)]

• Description updated (diff)

[ko1 (Koichi Sasada)]

Just to confirm: the PR's implementation of Thread::Backtrace::Location#source_range seems fragile because it reparses the script from the file system.
Is this behavior intentional?
I understand the implementation and its current limitations, but I wonder whether users will be able to understand or anticipate this behavior from this method name.

[Eregon (Benoit Daloze)]

ko1 (Koichi Sasada) wrote in #note-14:

Just to confirm: the PR's implementation of Thread::Backtrace::Location#source_range seems fragile because it reparses the script from the file system.
Is this behavior intentional?

Yes, this is intentional to avoid memory overhead.
To obtain an AST node, one needs to reparse anyway since CRuby discards the AST after compiling to bytecode.
For getting the source range, one must either reparse or retain additional information.

To avoid reparsing for #source_range, we would need to retain ranges for every relevant bytecode location, for example 8 bytes for a uint32_t offset and a uint32_t length, plus line offsets per file; or 16 bytes for the four line/column values.
I chose on-demand reparsing instead.
This makes Thread::Backtrace::Location#source_range slower, but confines that cost to callers requesting a source range and avoids increasing memory usage for all executed Ruby code.

RubyVM.keep_script_lines = true, when enabled before compilation, avoids dependence on the file system by retaining the original source.
This still requires reparsing, but works even if the file is later changed or removed.

I have added source-hash validation from @mame (Yusuke Endoh) 's work. This seems a great safety net against file modifications.
A changed source file is now rejected instead of potentially returning a range for different source code.

I understand the implementation and its current limitations, but I wonder whether users will be able to understand or anticipate this behavior from this method name.

I have updated the documentation to state explicitly that CRuby re-reads and reparses the source file and raises if the source is unavailable or has changed.
Other Ruby implementations may obtain the range differently (e.g. if it always keeps sources in memory) but regardless of that the user should be aware of the potential exceptions.

[kddnewton (Kevin Newton)]

If #syntax_tree gets merged, I am no longer certain of the use case here, because to my knowledge that entirely encapsulates this work. I can't imagine needing the exact source range (more than you already have) but not needing the syntax tree.

[Eregon (Benoit Daloze)]

Yes, if #syntax_tree reliably returns the correct node, the returned node can provide the start/end line/column.
The main gain of #source_range is providing those stable coordinates without relying on the version-specific node_id identity that remains unresolved in #21795.
The current #syntax_tree proposal also has other limitations, such as not working in --parser=parse.y mode.

Some use cases need the source range but not an AST. For example, displaying the exact source-code regions corresponding to an exception's backtrace_locations only requires source ranges.
Using #syntax_tree for that would allocate the entire Prism AST as Ruby objects merely to obtain those ranges.

#source_range provides a smaller, parser-independent abstraction and I think it is a natural extension now that Ruby::SourceRange is returned by {Proc,Method,UnboundMethod}#source_range.

[matz (Yukihiro Matsumoto)]

Accepted. Thread::Backtrace::Location#source_range is a natural extension of #21998, the failure behavior is fine as proposed, and source_range is the right name.

Regarding #note-7 and #note-16: I also intend to accept #syntax_tree. The two features will overlap, and I accept that. They offer different trade-offs, and callers can compare the conditions and choose. I will comment on #syntax_tree itself in #21795.

Matz.

[Eregon (Benoit Daloze)]

• Status changed from Open to Closed

Applied in changeset git|84a9e6bed71d009fa9ba272cfad96820127e097d.

---

[Feature #22212] Add Thread::Backtrace::Location#source_range

[Eregon (Benoit Daloze)]

• Target version set to 4.1

[Eregon (Benoit Daloze)]

• Assignee set to Eregon (Benoit Daloze)

[Eregon (Benoit Daloze)]

• Blocks Feature #21826: Deprecating RubyVM::AbstractSyntaxTree added