Issue #19452 has been updated by mame (Yusuke Endoh). @ioquatix Please, please write a use case in every proposal. First of all, I think the POC itself is very naive for daily use. Consider the following method call. For Thread::Bactrace::Location of the call to foo, `#first_column` will point before the receiver, not before the method name. And `#last_column` will point after all arguments. ``` very_very_long_receiver.foo(many_many_arguments) ^ ^ | | +--- #first_column #last_columns ---+ ``` Without a use case, it would be impossible to discuss whether and how it is useful. This is a heavier feature than you might think to introduce for the sake of "better than nothing". --- `ErrorHighlight.spot` is a bit more intelligent: it cuts out the method name part. ``` very_very_long_receiver.foo(many_many_arguments) ^^^^ | +--- ErrorHighlight.spot ``` To achieve this, ErrorHighlight uses `RubyVM::AST.of`: it reparses the source code, identify the node in the method call, and draw an underline after its receiver and before its arguments. Because of using `RubyVM::AST.of`, ErrorHighlight works only with CRuby. That is unfortunate. Currently @kddeisz is working on a new Ruby parser project called [yarp](https://github.com/Shopify/yarp). As far as I know, its goal is to be a common parser for all major Ruby implementations including CRuby and TruffleRuby. If the goal is accomplished, I will use yarp for ErrorHighlight, which will (hopefully) allow ErrorHighlight to work on Ruby interpreters rather than CRuby. Even in that case, it is still necessary to map `Thread::Backtrace::Location` to the AST node. We may use `Thread::Backtrace::Location#first_lineno`, etc. to identify the AST node corresponding to the `Thread::Backtrace::Location`. But this should be considered after yarp actually becomes a CRuby parser. (Implementation note: currently, `Thread::Backtrace::Location` has the ID of the AST node, which is a number assigned internally to the nodes in the AST in (approximate) pre-order. `RubyVM::AST.of` re-parses the source code to recover the full AST, and then identifies the AST node by using the node ID. This is the method suggested by @ko1. The four values, first_lineno / first_column / last_lineno / last_column, could be another way to identify nodes. But this is slightly less accurate: if there is another node in the exact same code range, it is not uniquely identifiable. I don't think this is a problem for ErrorHighlight, though.) @eregon @kddeisz Any opinions? ---------------------------------------- Feature #19452: `Thread::Backtrace::Location` should have column information if possible. https://bugs.ruby-lang.org/issues/19452#change-102173 * Author: ioquatix (Samuel Williams) * Status: Open * Priority: Normal ---------------------------------------- I discussed this with @mame and it would be pretty useful if we could also get the column information from exception backtrace location, even if it was slow. A POC: ```ruby class Thread::Backtrace::Location if defined?(RubyVM::AbstractSyntaxTree) def first_column RubyVM::AbstractSyntaxTree.of(self, keep_script_lines: true).first_column end else def first_column raise NotImplementedError end end end ``` It would be good to have a standard interface, so we follow the same interface as https://bugs.ruby-lang.org/issues/19451 and vice versa where it makes sense. I'll investigate it. -- https://bugs.ruby-lang.org/

Issue #19452 has been updated by mame (Yusuke Endoh). @eregon Thank you for your comment. Eregon (Benoit Daloze) wrote in #note-5: I think there is a confusion of what the word "reparse" means here. 1. When CRuby compiles a AST to a bytecode, it copies just node_id and lineno to each instruction but discards the column information. (@ko1 is not willing to copy column information due to memory consumption concerns.) So `RubyVM::AST.of` needs to "reparse" the whole source code to recover the whole AST. 2. CRuby's `RubyVM::AST` does not keep the column information of the method name because it converts the name to a symbol. It needs to "reparse" (or "re-tokenize"?) the code to identify where the method name is. Here I am talking about 1, which will not change even if YARP is introduced. If YARP's AST keeps the method name column information, 2 will be unnecessary, which is good. This is somewhat a matter of taste, but since this is a user interface, I believe it is important to provide pinpoint information at a glance. So I think it is worthwhile to elaborate it, even if it is somewhat tedious. ---------------------------------------- Feature #19452: `Thread::Backtrace::Location` should have column information if possible. https://bugs.ruby-lang.org/issues/19452#change-102175 * Author: ioquatix (Samuel Williams) * Status: Open * Priority: Normal ---------------------------------------- I discussed this with @mame and it would be pretty useful if we could also get the column information from exception backtrace location, even if it was slow. A POC: ```ruby class Thread::Backtrace::Location if defined?(RubyVM::AbstractSyntaxTree) def first_column RubyVM::AbstractSyntaxTree.of(self, keep_script_lines: true).first_column end else def first_column raise NotImplementedError end end end ``` It would be good to have a standard interface, so we follow the same interface as https://bugs.ruby-lang.org/issues/19451 and vice versa where it makes sense. I'll investigate it. -- https://bugs.ruby-lang.org/

Issue #19452 has been updated by kddeisz (Kevin Newton). YARP keeps column information around for this. In the call node there's column information at each of the points below: ``` ruby very_very_long_receiver.foo(many_many_arguments) ^ ^^ ^^ ^ ``` This may be too much at the end of the day, but for now it's working out nicely. In terms of memory consumption, I would think it would be 1-to-1 if we dropped node_id and replaced it with column ---------------------------------------- Feature #19452: `Thread::Backtrace::Location` should have column information if possible. https://bugs.ruby-lang.org/issues/19452#change-102295 * Author: ioquatix (Samuel Williams) * Status: Open * Priority: Normal ---------------------------------------- I discussed this with @mame and it would be pretty useful if we could also get the column information from exception backtrace location, even if it was slow. A POC: ```ruby class Thread::Backtrace::Location if defined?(RubyVM::AbstractSyntaxTree) def first_column RubyVM::AbstractSyntaxTree.of(self, keep_script_lines: true).first_column end else def first_column raise NotImplementedError end end end ``` It would be good to have a standard interface, so we follow the same interface as https://bugs.ruby-lang.org/issues/19451 and vice versa where it makes sense. I'll investigate it. -- https://bugs.ruby-lang.org/

Issue #19452 has been updated by Eregon (Benoit Daloze). I'm OK with that, I don't need these methods urgently. It will be nice to have public APIs for this kind of functionality vs experimental RubyVM:: stuff. ---------------------------------------- Feature #19452: `Thread::Backtrace::Location` should have column information if possible. https://bugs.ruby-lang.org/issues/19452#change-102308 * Author: ioquatix (Samuel Williams) * Status: Open * Priority: Normal ---------------------------------------- I discussed this with @mame and it would be pretty useful if we could also get the column information from exception backtrace location, even if it was slow. A POC: ```ruby class Thread::Backtrace::Location if defined?(RubyVM::AbstractSyntaxTree) def first_column RubyVM::AbstractSyntaxTree.of(self, keep_script_lines: true).first_column end else def first_column raise NotImplementedError end end end ``` It would be good to have a standard interface, so we follow the same interface as https://bugs.ruby-lang.org/issues/19451 and vice versa where it makes sense. I'll investigate it. -- https://bugs.ruby-lang.org/

Issue #19452 has been updated by mame (Yusuke Endoh). Thank you all! kddeisz (Kevin Newton) wrote in #note-8: What concerns me slightly is that it makes it impossible to uniquely identify nodes that have the same column information, such as NODE_SCOPE or its child node. Perhaps it is not a very big deal for ErrorHighlight, though. Eregon (Benoit Daloze) wrote in #note-9: When adding node_id, I evaluated the memory usage with `rails s`. (See #17930.) It increased about 3 % (97 MB -> 100 MB). Not very small to ignore. ufuk (Ufuk Kayserilioglu) wrote in #note-11: That makes sense. Although some efforts have already been made in compression, I guess that there would be room for improvement. ---------------------------------------- Feature #19452: `Thread::Backtrace::Location` should have column information if possible. https://bugs.ruby-lang.org/issues/19452#change-102326 * Author: ioquatix (Samuel Williams) * Status: Open * Priority: Normal ---------------------------------------- I discussed this with @mame and it would be pretty useful if we could also get the column information from exception backtrace location, even if it was slow. A POC: ```ruby class Thread::Backtrace::Location if defined?(RubyVM::AbstractSyntaxTree) def first_column RubyVM::AbstractSyntaxTree.of(self, keep_script_lines: true).first_column end else def first_column raise NotImplementedError end end end ``` It would be good to have a standard interface, so we follow the same interface as https://bugs.ruby-lang.org/issues/19451 and vice versa where it makes sense. I'll investigate it. -- https://bugs.ruby-lang.org/

Issue #19452 has been updated by janosch-x (Janosch Müller). `TracePoint` also lacks column information AFAICT so it might be worthwhile to consider it in search of a consistent solution. ---------------------------------------- Feature #19452: `Thread::Backtrace::Location` should have column information if possible. https://bugs.ruby-lang.org/issues/19452#change-103245 * Author: ioquatix (Samuel Williams) * Status: Open * Priority: Normal ---------------------------------------- I discussed this with @mame and it would be pretty useful if we could also get the column information from exception backtrace location, even if it was slow. A POC: ```ruby class Thread::Backtrace::Location if defined?(RubyVM::AbstractSyntaxTree) def first_column RubyVM::AbstractSyntaxTree.of(self, keep_script_lines: true).first_column end else def first_column raise NotImplementedError end end end ``` It would be good to have a standard interface, so we follow the same interface as https://bugs.ruby-lang.org/issues/19451 and vice versa where it makes sense. I'll investigate it. -- https://bugs.ruby-lang.org/