-
-
Notifications
You must be signed in to change notification settings - Fork 23
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
Feature: assert_impl_any!
#19
Comments
I implemented an macro_rules! assert_impl_any {
($x:ty: $($t:path),+ $(,)?) => {
const _: fn() = || {
let previous = {
struct AssertImplAnyFallback;
AssertImplAnyFallback
};
// Ensures that blanket traits can't impersonate the method.
struct __ActualAssertImplAnyToken;
$(
let previous = {
struct __Wrapper<T, N>(core::marker::PhantomData<T>, N);
// If the `__static_assert__impl_any_trait` method for this wrapper can't be called then
// the compiler will insert a deref and try again. This forwards the compilers next
// attempt to the previous wrapper.
impl<T, N> core::ops::Deref for __Wrapper<T, N> {
type Target = N;
fn deref(&self) -> &Self::Target {
&self.1
}
}
// This impl has a trait bound on $x so the method can only be called if $x implements $t.
impl<T: $t, N> __Wrapper<T, N> {
#[allow(non_snake_case)]
fn __static_assert__impl_any_trait(&self) -> __ActualAssertImplAnyToken { __ActualAssertImplAnyToken }
}
__Wrapper::<$x, _>(core::marker::PhantomData, previous)
};
)+
{
trait AssertImplAnyToken: Sized {}
impl AssertImplAnyToken for __ActualAssertImplAnyToken {}
fn assert_impl_any_token<T: AssertImplAnyToken>(_token: T) {}
// Attempt to find a `__static_assert__impl_any_trait` method that can actually be called.
// The found method must return a type that implements the sealed `Token` trait, this ensures that blanket trait methods can't cause this macro to compile.
assert_impl_any_token(previous.__static_assert__impl_any_trait());
}
};
};
} Playground link (includes some tests) This code uses a trick that I also made a comment about in |
@Lej77 Thank you so much for this! I wouldn't even have thought of doing it this way. I implemented a simpler version of this based on your implementation (see 59ac54f). It didn't make sense to me as to why the |
The reason for the use playground ::assert_impl_any;
trait Interfere<T> {
#[allow(non_snake_case)]
fn _static_assertions_impl_any(&self) -> T { loop {} }
}
impl<T, Token> Interfere<Token> for T {}
// These should fail to compile but the above trait might interferes and causes them to actually compile anyway. Fortunately the function must return a special type to compile and that isn't possible.
assert_impl_any!((): From<u8>);
assert_impl_any!((): From<u8>, From<u16>); If you use the type directly then the users blanket trait can have a generic return type and compile correctly while if type inference only know that the return type must implement a trait then that won't work. It probably would be better to do something like this so that the token trait isn't in scope for the macro variables (notice that the trait is in a block with a new scope): {
trait AssertImplAnyTokenTrait {}
impl AssertImplAnyTokenTrait for AssertImplAnyToken {}
fn assert_impl_any_token<T: AssertImplAnyTokenTrait>(_token: T) {}
assert_impl_any_token(previous._static_assertions_impl_any());
} I updated my previous comment's suggested code to try and minimize possible name collisions with user provided types and identifiers in the macro. |
Ok I follow as to how it prevents that case from compiling. However, I feel as though this adds a bit too much complexity. I'm leaning towards it not being worth it just to stop people from bypassing this with In terms of how this could be used in a malicious way outside of the user's crate... I suppose one could add I'll add this protection for that reason, but I'm not happy with the added complexity. But, then again, this crate is filled with obscure and complex stuff anyway. 😅 |
Yeah, I would like it to be simpler and more elegant too but I thought that it was important that the assert couldn't compile when it shouldn't. The scenario I imagined was reading other peoples code and them having intentionally (maliciously?) interfered with an assert so that the reader would assume something untrue. |
I saw that you used this macro to implement an macro_rules! assert_impl_one {
($x:ty: $($t:path),+ $(,)?) => {
const _: fn() = || {
// Generic trait.
trait AmbiguousIfMoreThanOne<A> {
// Required for actually being able to reference the trait.
fn some_item() {}
}
// Creates multiple scoped `Token` types for each trait `$t`, over
// which a specialized `AmbiguousIfMoreThanOne<Token>` is implemented
// for every type that implements `$t`.
$({
#[allow(dead_code)]
struct Token;
impl<T: ?Sized + $t> AmbiguousIfMoreThanOne<Token> for T {}
})+
// If there is only one specialized trait impl, type inference with
// `_` can be resolved and this can compile. Fails to compile if
// `$x` implements more than one `AmbiguousIfMoreThanOne<Token>`.
let _ = <$x as AmbiguousIfMoreThanOne<_>>::some_item;
};
};
} |
Isn't that just shooting yourself in the foot as the crate author? I can't imagine a sane reason to trick the readers of your code that way. Seeing an unused Huh, so while that is more obscure than the current implementation of |
I imagine that the |
Uses a similar trick as `assert_not_impl_any!`. Although this is more obscure, it feels less complex than the previous version. It's not self-referential and this sort of thing already exists in the same file. This implementation was suggested by @Lej77 at: #19 (comment)
Yeah the security concerns are definitely valid, so I added back your protection in 421b6ae. |
Asserts a given type implements any of a given set of traits.
The text was updated successfully, but these errors were encountered: