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

Original-only bounds #6

Open
bonsairobo opened this issue Aug 3, 2023 · 5 comments
Open

Original-only bounds #6

bonsairobo opened this issue Aug 3, 2023 · 5 comments

Comments

@bonsairobo
Copy link
Owner

I haven't thought of a convincing use case for omitting bounds from the original impl or fn items, but I suspect such a use case exists. Implementing this feature would not be very challenging, so this could be a good first issue for new contributors. I will leave open until I encounter the need for this.

@bonsairobo bonsairobo added the good first issue Good for newcomers label Aug 3, 2023
@bonsairobo
Copy link
Owner Author

Actually, I already started work on this here: https://github.com/bonsairobo/rkyv_impl/tree/omit_bounds

There are no tests yet, but the implementation seemed straightforward enough. The only interesting detail was that instead of parsing type identifiers from the argument list, we parse Type ASTs, to support bounds on types like &T or [T; 8].

@bonsairobo
Copy link
Owner Author

OK I actually encountered something that hints at a use case.

So let's say you have a type Foo<T> and you want to write:

#[archive_impl]
impl<T: Clone> Foo<T> {
    fn do_stuff(&self) {
        self.specialized_method();
    }
}

Where specialized_method is explicitly defined differently for Foo and ArchivedFoo. For example, this method could either deserialize a field of ArchivedFoo or just clone it in the case of Foo (hence the T: Clone) bound.

In this case, the T: Clone bound is only necessary for the impl Foo, not for impl ArchivedFoo. So it might be desirable to omit that bound on the archived impl.

It might be worth considering whether this bound should be overridden by omit_bounds(T) or provided as a separate nonarchive_bounds(T: Clone) option. The latter would be a bit more specific (being opt-in), and would prevent cases where you don't want to omit all bounds on T.

@bonsairobo
Copy link
Owner Author

bonsairobo commented Aug 6, 2023

I actually prefer the nonarchive_bounds idea (not sure if the name is the best). But in principal, you get to define a "template" of the impl block that is inherited by both the nonarchive and archived type. This treats both Foo and ArchivedFoo more equally w.r.t. the macro rather than having asymmetry and giving Foo a privileged position.

@bonsairobo
Copy link
Owner Author

bonsairobo commented Aug 6, 2023

A little more prototyping:

#[archive_impl]
#[original(add_bounds(T: Clone))]
#[archived(add_bounds(T::Archived: Deserialize<T, Infallible>))]
impl<T> Foo<T> {
    fn do_stuff(&self) {
        self.specialized_method();
    }
}

where original only applies to the impl Foo and archived only applies to impl ArchivedFoo.

@bonsairobo
Copy link
Owner Author

Not sure if #[archived] is needed when we could just use archive_impl and archive_method for the same thing.

@bonsairobo bonsairobo changed the title omit_bounds meta Original-only bounds Aug 6, 2023
@bonsairobo bonsairobo removed the good first issue Good for newcomers label Aug 6, 2023
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

No branches or pull requests

1 participant