[ruby-core:105522] [Ruby master Feature#18231] `RubyVM.keep_script_lines`

["Eregon (Benoit Daloze)" <noreply@ruby-lang.org>]

Issue #18231 has been updated by Eregon (Benoit Daloze).


I am strongly against this proposal as it currently stands.

This is yet another hack added under RubyVM.
I think adding new APIs under RubyVM is careless and lazy, because:
* It hurts the portability of the Ruby language, this code won't work on other Ruby implementations
* Because it's under RubyVM CRuby devs feel they can add anything because it's a "special module where things are experimental", even if the API is hacky and unreliable
* Such API will typically be used by gems at some points(e.g. because no other proper public API provides the functionality), but that's bad because it's typically hacky and unreliable, i.e., it's likely to break the usages whenever it evolves (e.g., RubyVM exposing the internal order or AST children which obviously will evolve when the syntax evolves)

I think every time someone thinks to add a new API under RubyVM they should instead think what a proper API would look like, and propose that instead.
Take the time to design a new API, don't rush a hack under RubyVM.

So, for our concrete use cases here:

For `error_highlight` I already mentioned from the start it was a bad idea to read code from disk and re-parse. And now it's clear this also doesn't work for eval'd code.
What we should do is design a proper API.
What does error_highlight need? Is it just the code location (i.e. col+line+length info) of the whole call and of the receiver? Maybe we can add such info in Exception#backtrace_locations and as NameError#receiver_code_location then.
Is it more complicated than that and it needs more info from the AST? How about preserving the needed metadata when parsing (so as some bytecode metadata for CRuby), and then expose it cleanly like NameError#highlight_code_location.
Since `error_highlight` needs to show the code, we also need a way to access the code for a loaded source. Those `code_location` could be objects and have that functionality (e.g., via to_s or some other method).
It's not hard, but both of these designs are much more reliable than the current `error_highlight`, they are portable and not hacky.

For the debugger/debug gem, could you describe what is needed?

`RubyVM::keep_script_lines = true` feels hacky.
Either CRuby is OK to preserve the source for every source loaded (until the whole file/source is GC'd), or it's not.
Also returning a String of the code location's code seems better than already splitting in lines for `script_lines`.
I would think having `code_location` on exceptions and methods is also enough for the debugger.

----------------------------------------
Feature #18231: `RubyVM.keep_script_lines`
https://bugs.ruby-lang.org/issues/18231#change-93973

* Author: ko1 (Koichi Sasada)
* Status: Open
* Priority: Normal
* Assignee: ko1 (Koichi Sasada)
* Target version: 3.1
----------------------------------------
This ticket proposes the method `RubyVM.keep_script_lines` to manage the flag to keep the source code.

Now, `SCRIPT_LINES__ = {}` constant is defined, the hash will stores the compiled script with a path name (`{path => src}`). However, `eval` source code doesn't stores in it (maybe because it can be bigger and bigger).

## Proposal

This proposal to add script lines to the ISeq and AST.

```ruby
RubyVM::keep_script_lines = true

eval("def foo = nil\ndef bar = nil")
pp RubyVM::InstructionSequence.of(method(:foo)).script_lines
#=> ["def foo = nil\n", "def bar = nil"]
```

In this case, methods `foo` and `bar` are defined by `eval()` and ISeq of `foo` can returns script lines (all script for `eval`).
`ISeq#script_lines` returns compiled script lines.

When `ISeq` is GCed, then the source code will be also free'ed.

## Discussion

* This feature will be used by debugger (to show the source code) or REPL support (irb and so on) with `error_highlight`.
* Of course memory usage will be increased.
* We can introduce new status `only_eval` in future which will help REPL systems.
* We can implement `ISeq#source` method like `AST#source` method (similar to `toSource` in JavaScript), but this ticket doesn't contain it.

## Implementation

https://github.com/ruby/ruby/pull/4913

For the Ractor support script lines object should be immutable (not implemented yet).




-- 
https://bugs.ruby-lang.org/