Skip to content

Comments

Fix function array argument calling convention#10

Open
davispuh wants to merge 1 commit intoMVV90:mainfrom
davispuh:args
Open

Fix function array argument calling convention#10
davispuh wants to merge 1 commit intoMVV90:mainfrom
davispuh:args

Conversation

@davispuh
Copy link

@davispuh davispuh commented Feb 2, 2026

C language allows function definitions to be like:

GLAPI void GLAPIENTRY glLoadTransposeMatrixd( const GLdouble m[16] )

But currently such definitions create invalid FFI type:

TypeError: unable to resolve type '[:double, 16]' (TypeError)

        raise TypeError, "unable to resolve type '#{name}'"
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This PR fixes this by correctly defining it as pointer type.
So now with this PR and with #9 rake test completes successfully.

Copilot AI review requested due to automatic review settings February 2, 2026 17:16
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adjusts generated Ruby FFI signatures so C array parameters (e.g., const double m[16]) are emitted as pointer types in function/callback signatures, avoiding invalid FFI array type resolution errors.

Changes:

  • Add an optional usecase parameter to ruby_ffi_type across type classes to allow context-sensitive type emission.
  • Emit :pointer for ArrayType when used as a function argument, while retaining array literals for struct/union layouts.
  • Update function/callback signature generation to call ruby_ffi_type(:function_argument) for parameters.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
lib/ffi_generator/generators/from_c_generator/unknown_type.rb Updates ruby_ffi_type signature to accept a context parameter.
lib/ffi_generator/generators/from_c_generator/struct_or_union.rb Updates ruby_ffi_type signature to accept a context parameter.
lib/ffi_generator/generators/from_c_generator/string_type.rb Updates ruby_ffi_type signature to accept a context parameter.
lib/ffi_generator/generators/from_c_generator/primitive_type.rb Updates ruby_ffi_type signature to accept a context parameter.
lib/ffi_generator/generators/from_c_generator/pointer_type.rb Updates ruby_ffi_type signature to accept a context parameter.
lib/ffi_generator/generators/from_c_generator/function_or_callback.rb Passes :function_argument context when generating parameter FFI signatures.
lib/ffi_generator/generators/from_c_generator/enum.rb Updates ruby_ffi_type signature to accept a context parameter.
lib/ffi_generator/generators/from_c_generator/by_value_type.rb Updates ruby_ffi_type signature to accept a context parameter.
lib/ffi_generator/generators/from_c_generator/array_type.rb Emits :pointer for function-argument arrays; keeps array literals elsewhere.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


def ruby_ffi_type
def ruby_ffi_type(usecase = nil)
"#{@inner_type.ruby_ffi_type}.by_value"
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ByValueType accepts the new usecase argument but doesn’t forward it to @inner_type.ruby_ffi_type. To keep behavior consistent (and avoid future context-specific type mismatches), pass usecase through when calling the inner type.

Suggested change
"#{@inner_type.ruby_ffi_type}.by_value"
"#{@inner_type.ruby_ffi_type(usecase)}.by_value"

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant