lrama icon indicating copy to clipboard operation
lrama copied to clipboard

Add lexer.rbs

Open akouryy opened this issue 2 years ago • 1 comments

Added lexer.rbs.

Discussions:

  • Type and Token have type parameters named SValue. I expect them to work well in parser.rb.
    • They are invariant. I could not find a good way to make them covariant while keeping create_token(Token::Number, str, ...) untypable.
  • I added patch.rbs, which patches the types of StringScanner#[] and StringScanner#getch.
    • Another approach would be to define the non-nil versions of the methods, e.g., StringScanner#fetch and StringScanner#getch!, as soutaro remarked.
    • Yet another approach would be to replace every ss[n] with (ss[n] || raise) and so on, but that would be hard to read.
  • On the other hand, I avoided patching Array#[] and Array#first because Array is used widely, and such patches may affect other files. Instead, I replaced array[0...l] with array.take(l) and array.first with array.fetch(0).

akouryy avatar May 27 '23 03:05 akouryy

  • type variance: I'm not sure the point. Can I ask you to share the code which does not work well?
  • patch.rbs: I agree to the current approach. There are 3 options for it as you write. However rewriting the method call like (ss[n] || raise) make codes un-readable. I don't have strong preference between defining StringScanner#fetch and patching rbs definitions. But patching rbs is bit better as a first step because StringScanner is not used widely in this project.
  • Array: Both replacement seems reasonable for me.

yui-knk avatar May 28 '23 08:05 yui-knk

Thanks for working on this. The following commit adds a type definition to the Lexer class, so this PR is closed.

  • https://github.com/ruby/lrama/commit/e44b6d380b08c064befe642ba27fc76cfd223c65

ydah avatar May 12 '25 18:05 ydah