RBS additions #1

larskanis · 2024-01-02T21:53:32Z

Happy new year as well @ParadoxV5 !

I did some minor adjustments to the CI to make it green. Then I checked the RBS files against the specs and was surprised about the many failures. So I went further to fix them in the subsequent commits.

I used the following command line:

RBS_TEST_TARGET='FFI::*' RUBYOPT='-rrbs/test/setup'  bundle exec rake spec

I was able to fix most RBS issues, but a few specs fail with mismatch in Struct#layout . I wasn't able to fix it properly. And Function#attach fails with endless recursion. Probably an incompatibility with super call and the prepend method

Could you please check my additions?

rb_class_new_instance() automatically passes the block forward to the initialize method. That way the block of a callback is forwarded to the Struct initializer, like here in the specs: s = LibTest::S8F32S32.new ret = LibTest.testCallbackVrT { s } Struct#initialize ignores the block, so that the code works, but it doesn't make sense. This stood up when enabling rbs type checks, which complained about an unspecified block to Struct#initialize.

They stood up when running with rbs type checks enabled.

ParadoxV5

By Struct#layout you meant Struct.layout (AKA Struct::layout).

Test failures show that a [Symbol, Integer] 2-tuple is also valid to come after a member name. This wasn’t documented.

layout :a, :int, :c, :test_enum,
  :d, [ TestEnum, TestEnum.symbols.length ]

layout( :a, [:char, 10],
        :i, :int,
        :f, :float,
        :d, :double,
        :s, :short,
        :l, :long,
        :j, :long_long,
        :c, :char )

It’d be more consistent (to type check with, as well) if the array form has each entry be a 2–3 tuple in the form [Symbol name, layout_type type, ?Integer offset].

[Update]
I thought the integer in the 2-tuple is the offset, but the tuple actually stands for an array, with the first element as the field type and the interger the array size.
This is documented in “Char arrays” in the wiki, though the array can be of any type.

sig/ffi/enum.rbs

ParadoxV5 · 2024-01-03T04:13:24Z

sig/ffi/enum.rbs

    def []: (Symbol query) -> Integer
-          | (Integer query) -> Symbol
+          | (Integer query) -> Symbol?


If so, may as well?

Suggested change

def []: (Symbol query) -> Integer

| (Integer query) -> Symbol

| (Integer query) -> Symbol?

def []: (Symbol query) -> Integer?

| (Integer query) -> Symbol?

RBS for Ruby Core classes like ::Array does it like this:

Suggested change

def []: (Symbol query) -> Integer

| (Integer query) -> Symbol

| (Integer query) -> Symbol?

def []: %a{implicitly-returns-nil} (Symbol query) -> Integer

| %a{implicitly-returns-nil} (Integer query) -> Symbol

Can you please decide on your own? I only read a crash course about RBS and static typing...

%a{implicitly-returns-nil} doesn’t work with RBS Tests.

ParadoxV5 · 2024-01-03T04:19:51Z

sig/ffi/function.rbs

      ) -> self

+#        ?blocking: boolish?, ?convention: Library::convention?, ?enums: Enums?


See ruby/rbs#1639

For now, it’s more intuitive if there isn’t a blank line gapping #initialize and this comment.

ParadoxV5 · 2024-01-03T04:28:51Z

sig/ffi/pointer.rbs

@@ -28,7 +27,6 @@ module FFI
    def read_string_length: (Integer len) -> String
    def read_string_to_null: () -> String
    def slice: (Integer offset, Integer length) -> Pointer
-    def to_ptr: () -> self


This is to indicate that Pointer#to_ptr returns self (which is a Pointer).

sig/ffi/pointer.rbs

sig/ffi/struct_by_reference.rbs

ParadoxV5 · 2024-01-03T04:51:02Z

sig/ffi/abstract_memory.rbs

@@ -159,6 +159,6 @@ module FFI
    def write_array_of_long_long: (Array[Integer] ary) -> self
    def write_array_of_float: (Array[Numeric] ary) -> self
    def write_array_of_double: (Array[Numeric] ary) -> self
-    def write_array_of_pointer: (Array[Pointer::_ToPtr] ary) -> self
+    def write_array_of_pointer: (Array[Pointer::_ToPtr?] ary) -> self


Given that nil is also accepted whereëver _ToPtr is, would type pointer = _ToPtr? in Pointer RBS be a valuable addition?

RBS builtins has things like type int = Integer | _ToInt, albeit they’re redundant.

Can you please decide on your own?

It’s implicit, but #(put|write)_array_of_pointer also accepts Integer elements (but not #to_ints since they share the conversion implementation with #(put|write)_pointer.

While FFI currently only mention #to_ptr in AbstractMemory, the _ToPtr?|Integer pattern is useful in user libraries’ (i.e., bindings based on FFI) RBS as well, so I’ll surface the type alias to the FFI namespace.

sig/ffi/abstract_memory.rbs

ParadoxV5 · 2024-01-03T05:10:05Z

sig/ffi/function.rbs

+    # TODO: leads to a endless recursion when used with -rrbs/test/setup
+    # def attach: (Module mod, String name) -> self


Perhaps porting Function#attach to Ruby (similar to its cousin, VariadicInvoker#attach) and flatten the RegisterAttach private module away would solve/workaround this problem.

Did you know that even CRuby is migrating some implementations from C to Ruby?
With all the JIT improvements, Ruby code’s performance is rivaling that of C itself!

In fact, it makes sense to me if Function and VariadicInvoker share some implementations with a common ancestor module, or even (if past design decisions aren’t in the way) have VariadicInvoker inherit from Function.

I opened an issue regarding the stack level too deep.

sig/ffi/library.rbs

For better readability. And add some comments.

Similar to commit 5b4b406 rb_class_new_instance() automatically passes the block forward to the initialize method. That way the block of an array function is forwarded to the Struct initializer. Struct#initialize ignores the block, so that the code works, but it doesn't make sense. This stood up when enabling rbs type checks, which complained about an unspecified block to Struct#initialize.

11) MemoryPointer allows writing as a sized int Failure/Error: put(type, 0, value) RBS::Test::Tester::TypeError: TypeError: [FFI::AbstractMemory#put] ArgumentTypeError: expected `bot` (value) but given `1`

11) MemoryPointer allows writing as a sized int Failure/Error: m.write :uint32, 1 RBS::Test::Tester::TypeError: TypeError: [FFI::Pointer#write] ArgumentTypeError: expected `bot` (value) but given `1` # /home/lars/.rvm/gems/ruby-3.3.0/gems/rbs-3.4.1/lib/rbs/test/tester.rb:158:in `call' # /home/lars/.rvm/gems/ruby-3.3.0/gems/rbs-3.4.1/lib/rbs/test/observer.rb:10:in `notify' # /home/lars/.rvm/gems/ruby-3.3.0/gems/rbs-3.4.1/lib/rbs/test/hook.rb:178:in `ensure in write__with__RBS_TEST_eb5149_4888829b' # /home/lars/.rvm/gems/ruby-3.3.0/gems/rbs-3.4.1/lib/rbs/test/hook.rb:178:in `write__with__RBS_TEST_eb5149_4888829b' # ./spec/ffi/rbx/memory_pointer_spec.rb:65:in `block (2 levels) in <top (required)>' # ------------------ # --- Caused by: --- # RBS::Test::Tester::TypeError: # TypeError: [FFI::AbstractMemory#put] ArgumentTypeError: expected `bot` (value) but given `1` # /home/lars/.rvm/gems/ruby-3.3.0/gems/rbs-3.4.1/lib/rbs/test/tester.rb:158:in `call'

…mbol

By convention as discussed in ParadoxV5#1 (comment)

https://github.com/ParadoxV5/ruby-ffi/pull/1/files#r1440105066

larskanis added 6 commits January 2, 2024 22:41

Exclude rbs.gem install from rubies that are not supported

1bb1f0c

CI: Print mkmf.log on failure on Appveyor

45c283a

Fix documentation about Pointer

907f236

Various adjustments to rbs definitions

5220fb5

They stood up when running with rbs type checks enabled.

Remove double definition of Pointer#to_ptr

fb26374

ParadoxV5 self-assigned this Jan 2, 2024

ParadoxV5 reviewed Jan 3, 2024

View reviewed changes

Fix Struct::layout

4868bfa

ParadoxV5 reviewed Jan 3, 2024

View reviewed changes

sig/ffi/library.rbs Outdated Show resolved Hide resolved

larskanis added 10 commits January 4, 2024 17:27

Move typedef, type_find and related methods around

d9af6dc

For better readability. And add some comments.

RBS: Type maps can also have DataConverters as key

30ddc6a

RBS: custom_typemap is private

7c7178e

RBS: Fix errors in puts kind of:

947755e

11) MemoryPointer allows writing as a sized int Failure/Error: put(type, 0, value) RBS::Test::Tester::TypeError: TypeError: [FFI::AbstractMemory#put] ArgumentTypeError: expected `bot` (value) but given `1`

RBS: Integer values are converted by to_int

a014273

RBS: Struct#order(Symbol) returns a Struct whereas order() returns Sy…

a44eba5

…mbol

RBS: declare initialize methods as returning void

eebe26a

By convention as discussed in ParadoxV5#1 (comment)

RBS: remove parantheses as suggested

ff9d8e9

https://github.com/ParadoxV5/ruby-ffi/pull/1/files#r1440105066

ParadoxV5 mentioned this pull request Jan 5, 2024

Re: RBS additions larskanis/ffi#2

Closed

ParadoxV5 merged commit ff9d8e9 into ParadoxV5:rbs-init Jan 8, 2024

larskanis deleted the ParadoxV5-rbs-init branch March 29, 2024 17:11

larskanis mentioned this pull request Mar 29, 2024

RBS for key modules and classes (mainly user-facing ones) ffi/ffi#1042

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RBS additions #1

RBS additions #1

larskanis commented Jan 2, 2024

ParadoxV5 left a comment •

edited

Loading

ParadoxV5 Jan 3, 2024

larskanis Jan 4, 2024

ParadoxV5 Jan 5, 2024

ParadoxV5 Jan 3, 2024

ParadoxV5 Jan 3, 2024

ParadoxV5 Jan 3, 2024

larskanis Jan 4, 2024

ParadoxV5 Jan 4, 2024 •

edited

Loading

ParadoxV5 Jan 3, 2024 •

edited

Loading

larskanis Jan 5, 2024

		) -> self

		# ?blocking: boolish?, ?convention: Library::convention?, ?enums: Enums?

		# TODO: leads to a endless recursion when used with -rrbs/test/setup
		# def attach: (Module mod, String name) -> self

RBS additions #1

RBS additions #1

Conversation

larskanis commented Jan 2, 2024

ParadoxV5 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ParadoxV5 Jan 4, 2024 • edited Loading

Choose a reason for hiding this comment

ParadoxV5 Jan 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ParadoxV5 left a comment •

edited

Loading

ParadoxV5 Jan 4, 2024 •

edited

Loading

ParadoxV5 Jan 3, 2024 •

edited

Loading