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

Alternative implementation of assert_type_eq_all? #56

Open
ypoluektovich opened this issue Apr 7, 2023 · 0 comments
Open

Alternative implementation of assert_type_eq_all? #56

ypoluektovich opened this issue Apr 7, 2023 · 0 comments

Comments

@ypoluektovich
Copy link

I'm trying to check that a bunch of types (actually types of a function's parameters) are all the same struct with some generic bound. E. g., function(a: ByRef<String>, b: ByRef<i32>) is ok, but function(x: usize) is not.

Looking at the implementation of assert_type_eq_all!, I saw that it compares the first supplied type to all other types. So, I hoped that I could gather a list of all parameter types and check them all with one invocation.

assert_type_eq_all!(ByRef<_>, #(#arg_types),*);

(Don't mind the weird # syntax, it's all happening within quote!{} in a proc macro. I'm also omitting most of the reexporting boilerplate.)

However, when given a mismatching type, this is the error I get:

error[E0271]: type mismatch resolving `<ByRef<_> as TypeEq>::This == Handle<AtomicI32>`
  --> tests/single-fn.rs:12:1
   |
12 | #[sarasa]
   | ^^^^^^^^^ expected `Handle<AtomicI32>`, found `ByRef<_>`
   |
   = note: expected struct `Handle<AtomicI32>`
              found struct `ByRef<_>`
note: required by a bound in `_::{closure#0}::assert_type_eq_all`
  --> tests/single-fn.rs:12:1
   |
12 | #[sarasa]
   | ^^^^^^^^^
   | |
   | required by a bound in this function
   | required by this bound in `assert_type_eq_all`
   = note: this error originates in the attribute macro `sarasa` (in Nightly builds, run with -Z macro-backtrace for more info)

This output is problematic in two ways: first, I'd like the error to point to the type in the function declaration, and second, it says "expected Handle<AtomicI32>, found ByRef<_>" when actually it's the other way around — I am expecting ByRef and got Handle.

So, apparently, assert_type_eq_all! asserts that the first type is equal to all others, not that all other types are equal to the first one. The difference sure is subtle :)

Armed with this insight, I reversed the order of arguments to the macro: let the first one be the checked type and the second one the expected one:

assert_type_eq_all!(#hty, ByRef<_>);     // #hty is the reference to the type of the first parameter of the function

And, lo and behold, both problems magically disappear:

error[E0271]: type mismatch resolving `<Handle<AtomicI32> as TypeEq>::This == ByRef<_>`
  --> tests/single-fn.rs:13:22
   |
13 | async fn function(h: Handle<AtomicI32>, s: ByRef<S>) -> Result<ByRef<S>, Infallible> {
   |                      ^^^^^^^^^^^^^^^^^ type mismatch resolving `<Handle<AtomicI32> as TypeEq>::This == ByRef<_>`
   |
note: expected this to be `ByRef<_>`
  --> tests/single-fn.rs:12:1
   |
12 | #[sarasa]
   | ^^^^^^^^^
   = note: expected struct `ByRef<_>`
              found struct `Handle<AtomicI32>`
note: required by a bound in `_::{closure#0}::assert_type_eq_all`
  --> tests/single-fn.rs:12:1
   |
12 | #[sarasa]
   | ^^^^^^^^^
   | |
   | required by a bound in this function
   | required by this bound in `assert_type_eq_all`
   = note: this error originates in the macro `::sarasa::__reexports::assert_type_eq_all` which comes from the expansion of the attribute macro `sarasa` (in Nightly builds, run with -Z macro-backtrace for more info)

If I were to use assert_type_eq_all!, I'd have to generate several invocations of the macro, one for each parameter to be checked.

And thus we come to the alternative implementation that I plan to embed in my crate:

// assert_types_eq.rs

pub trait TypeEq {
    type This: ?Sized;
}

impl<T: ?Sized> TypeEq for T {
    type This = Self;
}

pub fn assert_types_eq<T, U>()
where
    T: ?Sized,
    U: ?Sized + TypeEq<This = T>,
{
}

#[macro_export]
macro_rules! assert_types_eq {
    ($x:ty, $($xs:ty),+ $(,)*) => {
        const _: fn() = || {
            $( $crate::assert_types_eq::assert_types_eq::<$x, $xs>(); )+
        };
    };
}

Which with the input of assert_types_eq!(ByRef<_>, #(#arg_types),*); gives the following:

error[E0271]: type mismatch resolving `<Handle<AtomicI32> as TypeEq>::This == ByRef<_>`
  --> tests/single-fn.rs:13:22
   |
13 | async fn function(h: Handle<AtomicI32>, s: ByRef<S>) -> Result<ByRef<S>, Infallible> {
   |                      ^^^^^^^^^^^^^^^^^ expected `ByRef<_>`, found `Handle<AtomicI32>`
   |
   = note: expected struct `ByRef<_>`
              found struct `Handle<AtomicI32>`
note: required by a bound in `assert_types_eq`
  --> /mnt/vista/work/sarasa/src/assert_types_eq.rs:12:24
   |
12 |     U: ?Sized + TypeEq<This = T>,
   |                        ^^^^^^^^ required by this bound in `assert_types_eq`

There are two changes in my implementation:

  • a different set of type bounds on fn assert_types_eq, achieving the correct error message;
  • the trait and the function are moved out of the macro and instead exported from my crate.
  1. I suspect that the first change may negatively impact compilation times — it now generates impl TypeEq for all types except the first instead of only for the first. (I don't expect this to be significant, however.) As for the second — I have no idea why you didn't do it in the first place. Could you please explain the benefit of regenerating the trait and fn for each comparison?

  2. Even if there aren't significant disadvantages to my approach, I don't expect it to replace the current implementation of assert_type_eq_all, as it will break all existing usages that may rely on the argument order just like I do. However, if we manage to bikeshed a good name for it, could it be included in this crate as a new assertion?

(All of the above outputs were obtained using rustc 1.70.0-nightly (0599b6b93 2023-04-01) and static_assertions 1.1.0.)

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