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

Trait for Entry handler function types? #438

Closed
brandonchinn178 opened this issue Sep 23, 2023 · 2 comments · Fixed by #439
Closed

Trait for Entry handler function types? #438

brandonchinn178 opened this issue Sep 23, 2023 · 2 comments · Fixed by #439

Comments

@brandonchinn178
Copy link
Contributor

I wanted to write a helper that generalized the function to set for an entry, like:

fn init_entry<F>(entry: Entry<F>, handle_fn: F) {
    ...
}

but Entry.set_handler_fn is explicitly implemented for each HandlerFunc type separately, so I don't think I can provide an adequate type bound on F.

Instead of implementing Entry.set_handler_fn explicitly for each HandlerFunc type, why not provide a trait for all HandlerFunc types? e.g.

trait HandlerFuncType {
    fn to_handler_fn_addr(&self) -> VirtAddr;
}

macro_rules! impl_handler_func_type {
    ($f:ty) => {
        impl HandlerFuncType for $f {
            #[inline]
            fn to_handler_fn_addr(&self) -> x86_64::VirtAddr {
                VirtAddr::new(*self as u64)
            }
        }
    };
}

impl_handler_func_type!(HandlerFunc);
// all other HandlerFuncs

And then do

impl Entry {
    #[inline]
    pub fn set_handler_fn<F: HandlerFuncType>(&mut self, handler: &F) -> &mut EntryOptions {
        unsafe { self.set_handler_addr(handler.to_handler_fn_addr()) }
    }
}

Would this work? I think this is backwards-compatible, so I don't see any downsides to making this change. I'm fairly new to Rust, so let me know if I'm missing anything.

@Freax13
Copy link
Contributor

Freax13 commented Sep 23, 2023

I wanted to write a helper that generalized the function to set for an entry, like:

fn init_entry<F>(entry: Entry<F>, handle_fn: F) {
    ...
}

but Entry.set_handler_fn is explicitly implemented for each HandlerFunc type separately, so I don't think I can provide an adequate type bound on F.

As a workaround you could also implement your own trait responsible for setting the value:

trait HandlerFuncType: Sized {
    fn set(self, entry: &mut Entry<Self>) -> &mut EntryOptions;
}

Instead of implementing Entry.set_handler_fn explicitly for each HandlerFunc type, why not provide a trait for all HandlerFunc types? e.g.

trait HandlerFuncType {
    fn to_handler_fn_addr(&self) -> VirtAddr;
}

macro_rules! impl_handler_func_type {
    ($f:ty) => {
        impl HandlerFuncType for $f {
            #[inline]
            fn to_handler_fn_addr(&self) -> x86_64::VirtAddr {
                VirtAddr::new(*self as u64)
            }
        }
    };
}

impl_handler_func_type!(HandlerFunc);
// all other HandlerFuncs

And then do

impl Entry {
    #[inline]
    pub fn set_handler_fn<F: HandlerFuncType>(&mut self, handler: &F) -> &mut EntryOptions {
        unsafe { self.set_handler_addr(handler.to_handler_fn_addr()) }
    }
}

Would this work?

The F generic would have to be on Entry and not just set_handler_fn, but other than that I think that should work.

I think this is backwards-compatible, so I don't see any downsides to making this change.

My gut tells me that this will be backwards-compatible, but we'll have to see what the semver-checks CI job says.

I agree that this would be an improvement. Feel free to open a pr.

@brandonchinn178 brandonchinn178 changed the title [Question] Trait for Entry handler function types? Trait for Entry handler function types? Sep 23, 2023
@brandonchinn178
Copy link
Contributor Author

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 a pull request may close this issue.

2 participants