-
-
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
Add an assert_not_impl macro #17
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is exciting!
I needed to modify the design slightly, as the method call was possibly not completely unambiguous. Now the ambiguity has moved to the type parameter of a single locally defined trait which allows us to restrict the set of possibly applicable methods to that trait alone using turbo-fish syntax. By leaving out the trait's type parameter it can only be inferred when one impl is applicable. This also makes it somewhat simpler as only one type and trait is involved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much for this!! I was previously convinced that this wasn't possible. I love cursed hacks like these. ❤
Regarding style, please remove whitespace between empty braces.
I'll be more than happy to merge this PR once you've made my requested changes. Also, the changes in this PR should be detailed in CHANGELOG.md
under "Unreleased" in a new "Added" section.
src/assert_not_impl.rs
Outdated
/// [`Sync`]: https://doc.rust-lang.org/std/marker/trait.Sync.html | ||
/// [blanket]: https://doc.rust-lang.org/book/second-edition/ch10-02-traits.html#using-trait-bounds-to-conditionally-implement-methods | ||
#[macro_export(local_inner_macros)] | ||
macro_rules! assert_not_impl { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rename this to assert_not_impl_all!
and add a assert_not_impl_any!
counterpart. For assert_not_impl_any!
, see my suggestion below.
src/assert_not_impl.rs
Outdated
trait AmbiguousIfImpl<A> { fn some_item() { } } | ||
|
||
impl<T: ?Sized> AmbiguousIfImpl<()> for Check<T> { } | ||
impl<T: ?Sized $(+ $t)*> AmbiguousIfImpl<u8> for Check<T> { } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than use u8
here, add struct Invalid;
under Check
(or a better name if you think of one). Since it never gets constructed, make sure to add #[allow(dead_code)]
to the declaration.
For assert_not_impl_any!
, you should implement this bit like the following. Notice that each Invalid
type is unique because of the scoping.
$({
#[allow(dead_code)]
struct Invalid;
impl<A: ?Sized + $t> AmbiguousIfImpl<Invalid> for Check<A> {}
})+
Also, you should be matching multiple $t
via +
and not *
.
src/assert_not_impl.rs
Outdated
impl<T: ?Sized> AmbiguousIfImpl<()> for Check<T> { } | ||
impl<T: ?Sized $(+ $t)*> AmbiguousIfImpl<u8> for Check<T> { } | ||
|
||
<Check::<$x> as AmbiguousIfImpl<_>>::some_item() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do let _ = <Check::<$x> as AmbiguousIfImpl<_>>::some_item;
.
src/assert_not_impl.rs
Outdated
/// ``` | ||
/// | ||
/// Note that all traits given in a single invocation need to not be implemented. If you want to | ||
/// check that none of multiple trait are implemented you must invoke the macro several times. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change this to refer to assert_not_impl_any!
instead with a working local docs link. Also move this comment to right before "Examples".
src/assert_not_impl.rs
Outdated
/// # #[macro_use] extern crate static_assertions; | ||
/// # use static_assertions::_core::cell::Cell; | ||
/// # fn main() {} | ||
/// // Cell<u32> is not both Sync and Send |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make this normal docs text.
src/assert_not_impl.rs
Outdated
/// # #[macro_use] extern crate static_assertions; | ||
/// # use static_assertions::_core::cell::Cell; | ||
/// # fn main() {} | ||
/// // But it is Send, this fails to compile |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make this normal docs text. For visual purposes, I don't like having code blocks separated without anything in between to pad them.
Out of curiosity, why do To check that a type trait ShouldNotImplT {}
impl ShouldNotImplT for U where U: T {}
impl ShouldNotImplT for S {} |
@nox No particular reason, it turns out, only that a previous version of the implementation had a |
This is the polar opposite of the assert_impl macro and abuses name resolution with trait. A trait with type parameter `A` provides a simple method. The trait is implemented on the checked type `T` potentially twice: with type parameter unit `A=()` for all types `T`, and with another parameter type -abitrarily a new `Invalid`- when the `T` implements the trait bounds. We then try to select `T` as an instance of the trait, using turbo-fish to try and access one of its items, without providing the trait type parameter. When only the first instance is implemented the type checker can infer `A=()` but when the second is implemented as well it can not. No other ambiguity can occur, only the trait's type parameter is ambiguous. Thus, type inference and compilation succeeds if the trait bound is not satisified by the input type.
Makes use of the same trick as assert_not_impl_all but adding multiple possible implementations of the trait instead of at most two.
Awesome, thank you very much! |
Coming back to this a few years later, the async fn some_lib_feature() {
// Do some async work. To future proof this, and not cause unintentional semver breaks,
// we might want to assert this is !Send
}
// We can use a very similar trait to the one in the PR
trait AmbiguousIfImpl<A> { fn some_item(&self) {} }
impl<T: ?Sized> AmbiguousIfImpl<()> for T {}
struct Invalid;
impl<T: ?Sized + Send> AmbiguousIfImpl<Invalid> for T {}
// And use it to test the future is not Send
let fut = some_lib_feature(); // We can construct a value of the type, but can't name it
let () = <_ as AmbiguousIfImpl<_>>::some_item(&fut);
// ^ type is deduced to the type of the future, although we can not name it. * * This comment might become a bit outdated with the stabilization of |
This is the polar opposite of the assert_impl macro and abuses name
resolution with trait.
A trait with type parameter
A
provides a simple method. The trait isimplemented on the checked type
T
potentially twice: with typeparameter unit
A=()
for all typesT
, and with another parameter type-abitrarily a new
Invalid
- when theT
implements the trait bounds.We then try to select
T
as an instance of the trait, using turbo-fishto try and access one of its items, without providing the trait type
parameter. When only the first instance is implemented the type checker
can infer
A=()
but when the second is implemented as well it can not.No other ambiguity can occur, only the trait's type parameter is
ambiguous. Thus, type inference and compilation succeeds if the trait
bound is not satisified by the input type.