Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Method Overloads #218

Merged
merged 1 commit into from
Nov 13, 2024
Merged

Support Method Overloads #218

merged 1 commit into from
Nov 13, 2024

Conversation

cfis
Copy link
Collaborator

@cfis cfis commented Nov 11, 2024

When playing with OpenCV it became clear to me that Rice really needs to support construtor and method overloading. This PR is quite large, but it can be broken down into four key changes:

  • First, update the NativeRegistry to be keyed on Ruby class and method id and to store a vector of Native instances (NativeFunction, NativeIterator, NativeAttributeGet, NativeAttributeSet). If a method is overridden, there will be more than one native stored.

  • Second, create a base class Native for NativeFunction, NativeIterator, NativeAttributeGet and NativeAttributeSet. This allows the NativeRegistry to store unique_ptrs instead of std::any, which is hopefully a bit clearer and more importantly allows memory to be cleaned up via the use of std::unique_ptr (Natives are allocated on the heap but previously were never freed). It also reduces the amount of code by sharing it via the base Native class. Also NativeAttribute was split into NativeAttributeGet and NativeAttributeSet so they could share more code with Native.

  • Third, update From_Ruby#is_convertible's return type from bool to a new enum called Convertible which has three values - Exact, TypeCast or None. Exact means the Ruby type and C++ type are the same (example float), TypeCast means the Ruby type can be casted to the C++ type (example integer to float) and None means the Ruby and C++ types are not compatible. This is a backwards incompatible change for anyone who implemented their own version of From_Ruby#is_convertible but that seems fairly unlikely because it was previously used only for updating std::variants.

  • Fourth, update Natives to resolve overloaded methods at runtime by comparing the types of Ruby VALUEs to the C++ types using From_Ruby#is_convertible as described above. Exact matches take precedence over TypeCast matches. If no matches are found an exception is thrown.

@cfis cfis force-pushed the overloads branch 5 times, most recently from 41f7f26 to 8659e93 Compare November 11, 2024 08:03
… to support construtor and method overloading. This PR is quite large, but it can be broken down into four key changes:

* First, update the NativeRegistry to be keyed on Ruby class and method id and to store a vector of Native instances (NativeFunction, NativeIterator, NativeAttributeGet, NativeAttributeSet). If a method is overridden, there will be more than one native stored.

* Second, create a base class Native for NativeFunction, NativeIterator, NativeAttributeGet and NativeAttributeSet. This allows the NativeRegistry to store unique_ptrs instead of std::any, which is hopefully a bit clearer and more importantly allows memory to be cleaned up via the use of std::unique_ptr (Natives are allocated on the heap but previously were never freed). It also reduces the amount of code by sharing it via the base Native class. Also NativeAttribute was split into NativeAttributeGet and NativeAttributeSet so they could share more code with Native. This should have probably been a different commit but I realized its benefits after doing all the other changes.

* Third, update From_Ruby#is_convertible's return type from bool to a new enum called Convertible which has three values - Exact, TypeCast or None. Exact means the Ruby type and C++ type are the same (example float), TypeCast means the Ruby type can be casted to the C++ type (example integer to float) and None means the Ruby and C++ types are not compatible. This is a backwards incompatible change for anyone who implemented their own version of From_Ruby#is_convertible but that seems fairly unlikely because it was previously used only for updating std::variants.

* Fourth, update Natives to resolve overloaded methods at runtime by comparing the types of Ruby VALUEs to the C++ types using From_Ruby#is_convertible as described above. Exact matches take precedence over TypeCast matches. If no matches are found an exception is thrown.
Copy link
Collaborator

@jasonroelofs jasonroelofs left a comment

Choose a reason for hiding this comment

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

Looks good to me, can't say I have any specific questions. Feel free to merge it when you're ready.

@cfis
Copy link
Collaborator Author

cfis commented Nov 12, 2024

Cool - I'll update the docs and then merge the changes.

@cfis cfis merged commit c23986c into master Nov 13, 2024
13 checks passed
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.

2 participants