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

Subtype polymorphism #1392

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jasper310899
Copy link

As part of my bachelorthesis I worked on exending Autocxx's support for polymorphism, specifically subtype polymorphism.

This allows non-virtual methods to be called on child class objects as well as using child class references as baseclass refernce parameters values.

Currently only 75% of the tests are passing so I wanted to get an image of if you think this is worth fixing up.

The way I implemented it was as post processing after the last step before codegen:
Methods get implemented on a extension trait which is implemented for all childclasses.
Furthermore functions with reference parameters get wrapped in a function that takes a impl-type parameter.

This process is currently disabled for virtual functions with reference parameters.

For examples of what this allows please check out: https://git.yasupa.de/jasper3108/BA-2/src/branch/master/Abgabe/ch7/src/main.rs and https://git.yasupa.de/jasper3108/BA-2/src/branch/master/Abgabe/ch7/src/header.hpp for the C++ code.

My CLA is already applied but im still waiting for it.

Jasper W.

@jasper310899 jasper310899 changed the title Autoxcxx polymorphism Subtype polymorphism Sep 16, 2024
@jasper310899
Copy link
Author

jasper310899 commented Sep 16, 2024

I should also add that the traitbound used for polymorphis automatically detects anything that can be derefenced into a childcass.

This allows doing things like bindings::call_inherit_me(&bindings::B::new().within_box()); or moveit! { let mut obj = bindings::B::new() }; assert_eq!(bindings::call_mut(&mut obj).0, 42); while not breaking backwards compability.

@adetaylor
Copy link
Collaborator

Thanks for raising this!

I'm definitely interested in getting this to a place where it can be merged.

The main things missing from this PR are:

  • integration tests
  • docs (in book/ and the APIs)
  • probably an example too

Once it's a bit more complete (and tests all pass) I'll go through and do a review.

@jasper310899
Copy link
Author

Still haven't found the time to finish this up. Since this is a basic feature I want to offer this for everybody to finish if their interested. It should work mostly, there must be some edge cases that make the tests fail. Or would it make sense to make this an optional experiemental feature for now?

@adetaylor
Copy link
Collaborator

Thanks, but we won't be able to seriously consider merging this until the CI is green at least.

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