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

Dynamic dispatch with supertraits #3124

Merged
merged 1 commit into from
Sep 10, 2024
Merged

Conversation

liamnaddell
Copy link
Contributor

@liamnaddell liamnaddell commented Aug 10, 2024

There was only one function that needed modification, the rest is testing for this new feature :)

    gcc/rust/ChangeLog:

            * backend/rust-compile.cc:
            Modify compute_address_for_trait_item to support supertraits
            * typecheck/rust-tyty.cc:
            Remove auto

    gcc/testsuite/ChangeLog:

            * rust/compile/trait13.rs:
            Add test for supertraits of supertraits
            * rust/compile/trait14.rs:
            Diamond problem with supertraits test
            * rust/execute/torture/trait14.rs:
            Add test for dynamic dispatch with supertraits
            * rust/execute/torture/trait15.rs:
            Add test for dynamic dispatch with generics
            * rust/execute/torture/trait16.rs:
            Add test for dynamic dispatch with lifetime params 1
            * rust/execute/torture/trait17.rs:
            Add test for dynamic dispatch with lifetime params 2
            * rust/execute/torture/trait18.rs:
            Add test for default implementations with dynamic dispatch and
            supertraits

Fixes #914


Modifies compute_address_for_trait_item to support supertraits. The algorithm works by looping over receiver_bounds for all relevant traits, and attempting to find one that matches the type, and contains the correct function implementation.

@liamnaddell liamnaddell marked this pull request as draft August 10, 2024 19:21
@liamnaddell liamnaddell force-pushed the compile2 branch 3 times, most recently from f95ecd6 to 7e33f40 Compare August 12, 2024 01:37
@liamnaddell liamnaddell marked this pull request as ready for review August 12, 2024 01:37
@liamnaddell liamnaddell changed the title [#914] Dynamic dispatch with supertraits Dynamic dispatch with supertraits Aug 12, 2024
@liamnaddell
Copy link
Contributor Author

liamnaddell commented Aug 14, 2024

NOTE: I found a bug with the following code sample:

extern "C" {
    fn printf(s: *const i8, ...);
}

struct Foo {
    my_int: u32,
}

trait Parent {
    fn parent(&self) -> bool;
}

trait Child : Parent {
    fn child(&self);
}

impl Parent for Foo {
    fn parent(&self) -> bool {
        // Call supertrait method
        return true;
    }
}

impl Child for Foo {
    fn child(&self) {
        let _ = self;
    }
}

pub fn main() {
    let a = Foo{ my_int: 0xf00dfeed};
    let b: &dyn Child = &a;

    let c: &dyn Parent = b;

    c.parent();
}

I think this particular case might be best handled in a separate issue. Gating casts from dyn Child to dyn Parent seems like it will be quite involved. Also, out of scope of the original issue.

For context, the problem here is that you have to take Child's vtable, and prune it to match Parent's vtable. Rustc currently doesn't let you do this, however, they seem somewhat close-ish to stabilizing this conversion.

@liamnaddell liamnaddell force-pushed the compile2 branch 2 times, most recently from 6991386 to 313bc53 Compare August 15, 2024 22:18
@liamnaddell liamnaddell marked this pull request as draft August 17, 2024 15:30
@liamnaddell liamnaddell force-pushed the compile2 branch 2 times, most recently from 313bc53 to 1e437bc Compare August 20, 2024 22:45
@liamnaddell liamnaddell marked this pull request as ready for review August 20, 2024 22:45
@liamnaddell liamnaddell force-pushed the compile2 branch 2 times, most recently from 27b9be7 to 52e6831 Compare August 27, 2024 22:37
@P-E-P P-E-P self-requested a review August 28, 2024 09:26
gcc/rust/ChangeLog:

	* backend/rust-compile.cc:
	Modify compute_address_for_trait_item to support supertraits
	* typecheck/rust-tyty.cc:
	Remove auto

gcc/testsuite/ChangeLog:

	* rust/compile/trait13.rs:
	Add test for supertraits of supertraits
	* rust/compile/trait14.rs:
	Diamond problem with supertraits test
	* rust/execute/torture/trait14.rs:
	Add test for dynamic dispatch with supertraits
	* rust/execute/torture/trait15.rs:
	Add test for dynamic dispatch with generics
	* rust/execute/torture/trait16.rs:
	Add test for dynamic dispatch with lifetime params 1
	* rust/execute/torture/trait17.rs:
	Add test for dynamic dispatch with lifetime params 2
	* rust/execute/torture/trait18.rs:
	Add test for default implementations with dynamic dispatch and
	supertraits

Signed-off-by: Liam Naddell <liam.naddell@mail.utoronto.ca>
Copy link
Member

@P-E-P P-E-P 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, great work. I like all those new tests.

@P-E-P P-E-P added this pull request to the merge queue Sep 10, 2024
Merged via the queue into Rust-GCC:master with commit c5f9d6d Sep 10, 2024
11 checks passed
@liamnaddell liamnaddell deleted the compile2 branch September 10, 2024 15:19
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.

Dynamic objects support inheritance
2 participants