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

Remove unsafe code from expansion #7

Open
50U10FCA7 opened this issue Jan 17, 2025 · 4 comments
Open

Remove unsafe code from expansion #7

50U10FCA7 opened this issue Jan 17, 2025 · 4 comments
Assignees
Labels
enhancement Improvement of existing features or bugfix k::refactor Refactoring, technical debt elimination and other improvements of existing code base

Comments

@50U10FCA7
Copy link
Contributor

Background

Removing unsafe code from macro expansion.

Problem to solve

Usage of unsafe transmute to convert from type T into Wrapper<T>.

Possible solutions

Replace transmute with:

  • Into::into - for methods with self receiver;
  • RefCast::ref_cast - for methods with &self receiver;
  • RefCast::ref_cast_mut - for methods with &mut self receiver.
@50U10FCA7 50U10FCA7 added enhancement Improvement of existing features or bugfix k::refactor Refactoring, technical debt elimination and other improvements of existing code base labels Jan 17, 2025
@50U10FCA7 50U10FCA7 self-assigned this Jan 17, 2025
@50U10FCA7
Copy link
Contributor Author

50U10FCA7 commented Jan 17, 2025

@tyranron For now we can use the RefCast solution only for private::Wrapper, but not for the wrappers that creates on macro expansion (in case of foreign/external types/traits).

When foreign type is delegated we expands the following wrapper:

#[automatically_derived]
#[derive(Clone, Copy, Debug, ::delegation::private::ref_cast::RefCast)]
#[doc(hidden)]
#[repr(transparent)]
pub struct __delegate_AsRefDef__Wrapper<T>(T)
where
    T: ?::core::marker::Sized;

impl<T> From<T> for __delegate_AsRefDef__Wrapper<T> {
    fn from(inner: T) -> Self {
        Self(inner)
    }
}

but #[derive(::delegation::private::ref_cast::RefCast)] cannot be expanded because ::ref_cast doesn't exists in dependencies of the crate that uses our #[delegate].

Solution 1 (not working)

Also, we can't just use ref_cast_impl crate directly and expand it inside #[delegate] because #[proc_macro_derive] doesn't expose actual method defining the macro expansion, so

// `codegen` crate.
let definition = quote! {
pub struct #wrapper_ty<T>(T)
where
    T: ?::core::marker::Sized;
};

let definition = ref_cast_impl::derive_ref_cast(definition.to_token_stream()); // ERROR: `ref_cast_impl::derive_ref_cast` not found.

will not work.

Solution 2 (probably not working)

Another idea we can check is to use macro_rules! to manipulate order of the macro expansions and inject some changes into #[derive(ref_cast::RefCast)] result:

// `codegen` crate
let definition = quote! {
macro_rules! injection {
    (...) => { ::delegate::ref_cast_injection(...) } // Replace all the `::ref_cast` paths with `::delegate::private::ref_cast`.
}

injection! {  // This should expand next.
#[derive(::delegation::private::ref_cast::RefCast)] // This should expand first.
pub struct #wrapper_ty<T>(T)
where
    T: ?::core::marker::Sized;
}
};

This way we need to check, because it may not work as expected.

upd. Will not work

upd2.

Will not work too:

#[derive(::delegation::private::ref_cast::RefCast)]
#[::delegation::private::inject_ref_cast] // will not capture `#[derive()]` output
pub struct #wrapper_ty<T>(T)
where
    T: ?::core::marker::Sized;

@50U10FCA7
Copy link
Contributor Author

50U10FCA7 commented Jan 17, 2025

And we also can't rely on RefCast to customize the crate it imports machinery from. See dtolnay/ref-cast#23

@50U10FCA7
Copy link
Contributor Author

We can wait for TokenStream::expand_expr stabilization to get this work:

  • Place #[inject] before the #[derive(RefCast)];
  • Use TokenStream::expand_expr on the #[derive(RefCast)] captured by the #[inject].

@tyranron
Copy link
Member

@50U10FCA7 I think we will wait for years until that will work. It's implemented only for literals for now:

Currently only expressions expanding to literals will succeed, although this may be relaxed in the future.

And generalization for any AST doesn't seem to happen in near future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement of existing features or bugfix k::refactor Refactoring, technical debt elimination and other improvements of existing code base
Projects
None yet
Development

No branches or pull requests

2 participants