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

• Status: Open
• Assignee: -
• Target version: -

[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)