rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
97.96k stars 12.69k forks source link

Tracking issue for RFC #2056: Allow trivial constraints to appear in where clauses #48214

Open aturon opened 6 years ago

aturon commented 6 years ago

This is a tracking issue for the RFC "Allow trivial constraints to appear in where clauses " (rust-lang/rfcs#2056).

Steps:

Unresolved questions:

matthewjasper commented 6 years ago

I'm working on this.

nikomatsakis commented 6 years ago

@matthewjasper ok. I've not thought hard about what it will take to support this. I had thought about doing it after making some more progress on general trait refactoring, but if it can be easily supported today seems fine.

nikomatsakis commented 6 years ago

Oh, I meant to add, please ping me on IRC/gitter with any questions! I can try to carve out some time to think about it as well.

matthewjasper commented 6 years ago

So with #51042 merged this feature is implemented on nightly, but the implementation isn't particularly clean and there are some edge cases that don't work as one would expect:

#![feature(trivial_bounds)]

struct A;
trait X<T> {
    fn test(&self, t: T) {}
}

impl X<i32> for A {}

fn foo(a: A) where A: X<i64> {
    a.test(1i64); // expected i32, found i64
}

Since Chalk might be able to fix some of these issues, I think that this feature is now waiting for Chalk to be integrated.

edit: As of 2018-12-28 -Zchalk does not currently solve this issue.

RalfJung commented 5 years ago

As another concrete example where trivially false constraints arise due to macros, see https://github.com/taiki-e/pin-project/issues/102#issuecomment-540472282. That crate now carries a weird hack to work around this limitation.

What is the current status of this feature?

matthewjasper commented 5 years ago

Not much has changed since my last comment.

Systemcluster commented 4 years ago

The current implementation generates a warning when using the pre-existing impl trait syntax, which, reading the RFC, I assume is a false-positive.

#![allow(dead_code, unused_variables,)]
#![feature(trait_alias, trivial_bounds)]

trait T = Fn(&i32, &i32) -> bool;
fn impl_trait_fn() -> impl T {
    |a: &i32, b: &i32| true
}

This code generates the warning warning: Trait bound impl T: T does not depend on any type or lifetime parameters. (playground link)

Luro02 commented 4 years ago

I think this change could cause problems with some proc-macros that rely on

struct _AssertCopy where String: ::core::marker::Copy;

to not compile, like @ExpHP wrote in https://github.com/rust-lang/rfcs/pull/2056

To play devil's advocate, I do have to wonder if some crates may have popped up since 1.7 that rely on these errors as a form of static assertion about what traits are implemented (as the author was likely unaware that trivially true bounds were intended to produce compiler errors). Out of concern for this, I might lean towards "warn by default", since macros can easily disable the warnings in their output.

but I can not comprehend why this should be warn by default as this would break existing proc-macros that rely on this behavior?

I can say for a fact that there are definitely crates that rely on this behavior, because I recently published my first proc-macro (shorthand), which does rely on this behavior and I saw those assertions (struct __AssertCopy where String: ::core::marker::Copy;) in some crates and adopted it into my own crate.

I think most of those assertions are only used to make better error messages.

For example my proc-macro emits

struct _AssertCopy where String: ::core::marker::Copy;

which is spanned around the field of a struct and this creates this error message:

error[E0277]: the trait bound `std::string::String: std::marker::Copy` is not satisfied
 --> $DIR/not_copy.rs:6:12
  |
6 |     value: String,
  |            ^^^^^^ the trait `std::marker::Copy` is not implemented for `std::string::String`
  |
  = help: see issue #48214
  = help: add `#![feature(trivial_bounds)]` to the crate attributes to enable

I would say this lint should be deny by default to keep backwards compatibility.

eddyb commented 4 years ago

@Luro02 What else do you generate? Does your proc macro use unsafe and would it do the wrong thing without that "doesn't implement Copy" error?

If so why not put the bound on some impl or fn that would misbehave?

That would actually protect it, unlike an unrelated and unused type definition.

(Alternatively, just use _AssertCopy somewhere in your unsafe code)

Luro02 commented 4 years ago

@eddyb Nope, in my case it is not using any unsafe code and it would not do the wrong thing, but who knows some macro might use this as a guard. For example one could have an attribute like this:

#[only(copy)]
struct Example {
    field: String, // error because not copy
}

this would silently be accepted.

Alternatively, just use _AssertCopy somewhere in your unsafe code

does this mean, that trivial bounds do not compile in unsafe code? 🤔

The unrelated/unused type definiton is there to get a specific error message (like String does not implement Copy instead of can not move String because it does not implement Copy)

If so why not put the bound on some impl or fn that would misbehave?

I think this would still compile/ignore the bound fn value(&self) where String: Copy and it is a bit more difficult, than simply emitting a line in the function body.

dtolnay commented 4 years ago

I think the suggestion to "just use the bound somewhere in code" means emitting something like:

struct _AssertCopy where String: ::core::marker::Copy;
let _: _AssertCopy;

There is no way this would compile with an unsatisfied bound.

eddyb commented 4 years ago

does this mean, that trivial bounds do not compile in unsafe code? thinking

No, @dtolnay explained what I meant. I was expecting you had unsafe code, but it sounds like you're just using it to get better diagnostics, which makes it less tricky of a situation.

I think this would still compile/ignore the bound

If it doesn't error when compiling the function, it will error when trying to use it. It's not going to let you call the function without erroring.

jjpe commented 4 years ago

Just ran into this issue while implementing a derive macro Delta that derives a trait DeltaOps that:

  1. has an associated type DeltaOps::Delta
  2. has nontrivial bounds on Self
  3. has nontrivial bounds on DeltaOps::Delta
  4. and is implemented for (among others) mutually recursive enums.

That caused an overflow in the compiler, with compile errors like this one:


error[E0275]: overflow evaluating the requirement `rule::Expr: struct_delta_trait::DeltaOps`
  --> src/parser.rs:26:10
   |
26 | #[derive(struct_delta_derive::Delta)]
   |          ^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: required because of the requirements on the impl of `struct_delta_trait::DeltaOps` for `std::vec::Vec<rule::Expr>`
   = note: required because of the requirements on the impl of `struct_delta_trait::DeltaOps` for `rule::Rule`
   = note: required because of the requirements on the impl of `struct_delta_trait::DeltaOps` for `std::borrow::Cow<'static, rule::Rule>`
   = note: required because of the requirements on the impl of `struct_delta_trait::DeltaOps` for `stack::StackFrame`
   = note: required because of the requirements on the impl of `struct_delta_trait::DeltaOps` for `std::vec::Vec<stack::StackFrame>`
   = note: required because of the requirements on the impl of `struct_delta_trait::DeltaOps` for `stack::Stack`
   = help: see issue #48214
   = help: add `#![feature(trivial_bounds)]` to the crate attributes to enable
   = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)

As you can see, the compiler gave me a suggestion to use #![feature(trivial_bounds)] in my crate. My question is, will that actually work i.e. will it solve the overflow problem with bounds checking on recursive types without breaking stable functionality?

Veetaha commented 4 years ago

There might be a potential drawback we haven't discussed yet. The proposed API guideline for asserting on trait impls may not be applied if this feature is landed, cc https://github.com/rust-lang/api-guidelines/issues/223

kupiakos commented 2 years ago

There's a particular pain point in the trait system that this feature assists with: impossible-to-satisfy bounds on impls of trait items.

It's standard practice to have some items on a trait "opt-out" of making the trait object-unsafe with a where Self: Sized bound. But what if you're implementing on a concrete DST?

trait Foo {
    fn foo() -> Self where Self: Sized;
    fn bar(&self);
}

impl Foo for str {
    // We can't compile without defining *something* for foo, even though it could never be called!
    fn bar(&self) {
        println!("bar");
    }
}

Theoretically, we shouldn't have to specify fn foo for our impl Foo for str, but the compiler doesn't (yet) remove trivially-impossible bounds from the list of items that must be implemented.

With #![feature(trivial_bounds)], this is the simplest way to be able to define impl Foo for str: by just copying the impossible bound:

    // We can never call `foo`, but we can `impl Foo for str`!
    fn foo() -> str where str: Sized {
        unreachable!()
    }

There is, however, a hacky workaround: you can specify trivial constraints on stable Rust with an unused HRTB:

    fn foo() -> str where for<'a> str: Sized {
        unreachable!()
    }

Given the existence of this workaround, I see no reason that this feature should be left unstable for some simple cases. Additionally, I believe if there is a lint, it should be a warning or allow by default, unless it has special detection of this scenario.

Note: this problem only applies to impls on concrete types, the following works:

impl<T> Foo for [T] {
    // does not require `#![feature(trivial_bounds)]`
    fn foo() -> Self where Self: Sized {
        unreachable!()
    }
    fn bar(&self) {
    }
}
CAD97 commented 2 years ago

The current stable status of this is that true trivial bounds are allowed with no warning, and false trivial bounds are rejected as an unfulfilled obligation.

For this reason, I think the trivial_bounds lint should be split into two. Specifically, I think the following code:

trait Trait {}

struct No;
struct Yes;
impl Trait for Yes {}

struct S1;
struct S2;

impl Trait for S1 where Yes: Trait {}
impl Trait for S2 where No: Trait {}

should have a result similar to the following:

warning: trait bound Yes: Trait does not depend on any type or lifetime parameters
  --> src/lib.rs:12:30
   |
12 | impl Trait for S1 where Yes: Trait {}
   |                              ^^^^^
   |
   = note: this bound always holds
   = note: `#[warn(trivially_true_bounds)]` on by default

error: trait bound No: Trait does not depend on any type or lifetime parameters
  --> src/lib.rs:13:29
   |
13 | impl Trait for S2 where No: Trait {}
   |                             ^^^^^
   |
   = note: this bound never holds
   = note: `#[error(trivially_false_bounds)]` on by default
Current result with feature(trivial_bounds) ``` warning: trait bound Yes: Trait does not depend on any type or lifetime parameters --> src/lib.rs:12:30 | 12 | impl Trait for S1 where Yes: Trait {} | ^^^^^ | = note: `#[warn(trivial_bounds)]` on by default warning: trait bound No: Trait does not depend on any type or lifetime parameters --> src/lib.rs:13:29 | 13 | impl Trait for S2 where No: Trait {} | ^^^^^ ```

This means that the default behavior of what code compiles stays the same, and still informs authors of ineffective bounds. The rationale for blocking compilation on a trivially false bound is that it is likely incorrect (thus the lint) and as this results in a missing trait impl, will likely cause other errors to block compilation. A trivially true bound, while still likely incorrect (thus the lint), is much less likely to cause a compilation failure (if the trait impl succeeds) and thus a deny would get in the way of the edit-compile-test[^2] cycle.

[^2]: Well, the cycle already contains more error message cycles than in dynamic languages, but the general arguments for most lints being warn by default with high signal-noise warning ratio still stand.

Authors of derives using where to register obligations can then use #[allow(unknown_lints)] #[allow(trivially_true_bounds)] #[deny(trivially_false_bounds)] to keep the current behavior where a failed derive errors, or switch to another method[^1] of registering obligations. Note that where based obligations are being used for deriving unsafe impl in the ecosystem. This isn't a soundness issue, though, since the failure case is just the impl not applying.

[^1]: My current preferred method is to define a generic fn (inside a const _ context to avoid namespace pollution) with the same signature as the type and then using an assert_impl_trait_val(&this.field) for each field. dtolnay's version works for true trivial bounds, but generic field type bounds still need to be handled by a derive, and can themselves potentially be trivial (trivial example: for<T> T:).

Jules-Bertholet commented 2 years ago

Theoretically, we shouldn't have to specify fn foo for our impl Foo for str, but the compiler doesn't (yet) remove trivially-impossible bounds from the list of items that must be implemented.

I would argue, to the contrary, that the compiler should not make your life easier in this instance, because Self bounds on trait methods without default impls are an anti-pattern and should be discouraged, even linted against IMO. The reason I think such code is questionable is that it means that if type A starts implementing trait T, that can suddenly invalidate its implementation of trait U because U had a where Self: T trait method with no default impl. Such code also makes it harder for traits to take advantage of extensions to dyn-safety without breaking implementers (as I brought up in an IRLO thread). The proper way to write this code is to either provide a default impl or have two separate traits.

Ekleog commented 1 year ago

FWIW, having a proc-macro that uses syn's DeriveInput.generics.split_for_impl() with a recursive struct generates code like this, which triggers this issue:

trait Trait {}

struct Foo {
    inner: Option<Box<Foo>>,
}

impl Trait for Foo
where Option<Box<Foo>>: Trait // the problematic line
{}

impl<T: Trait> Trait for Option<T> {}
impl<T: Trait> Trait for Box<T> {}

playground link

That said I'm not sure whether that's a syn bug or something actually coming from TokenStream itself. In both cases, changing the behavior would seem pretty hard, considering it'd be a breaking change.

To be quite honest, I'm not sure what the expected workaround would be, as this line does seem to be necessary in the general case. Does anyone know how proc macro authors are supposed to work around this? I can see serde does avoid setting this bound so there should be a way, but TBH I don't understand most of the source code there.

taiki-e commented 1 year ago

@Ekleog

As explained in the comment RalfJung mentioned, you can work around this by using wrapper type with lifetime or generics: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=f04192e77a9ae36bf3df07e61027d638 (To be exact, it delays the evaluation until actually used)

As for serde, they do not specify trait bounds in the where clause for non-generic types in the first place: https://github.com/serde-rs/serde/blob/6b948111ca56bb5cfd447f8516dea5fe1338e552/test_suite/tests/expand/de_enum.expanded.rs#L18-L20 (Since the trait method call will fail if the trait is not implemented, that is fine.)

Ekleog commented 1 year ago

@taiki-e I'm not sure I understand how the comment you link is applicable to a proc-macro author situation. It seems to me that the end-user would have to add a lifetime to their struct, which would make it much less ergonomic?

If the serde way is actually the only one for proc-macro authors, then I guess we're missing a crate to reimplement the serde way of sorting between the clauses that actually need to be passed through (because of actual generics) and the clauses that are trivial, so that not all proc-macro authors need to reimplement a visitor like serde :/

taiki-e commented 1 year ago

@Ekleog

the end-user would have to add a lifetime to their struct

No, I meant that the macro generates a where clause with trait bounds with a wrapper type.

If the serde way is actually the only one for proc-macro authors, then I guess we're missing a crate to reimplement the serde way of sorting between the clauses that actually need to be passed through (because of actual generics) and the clauses that are trivial, so that not all proc-macro authors need to reimplement a visitor like serde :/

AFAIK, what serde actually does is the same as what the derive macros in the standard library do: they refer to type parameters in generics rather than field types. Therefore, there is no need for a filter/visitor. (see https://github.com/rust-lang/rust/issues/26925 for more)

Ekleog commented 1 year ago

No, I meant that the macro generates a where clause with trait bounds with a wrapper type.

Oh ok I think I understand now!

AFAIK, what serde actually does is the same as what the derive macros in the standard library do: they refer to type parameters in generics rather than field types. Therefore, there is no need for a filter/visitor. (see https://github.com/rust-lang/rust/issues/26925 for more)

Hmm I guess the choice is then currently between having the issues listed in #26925 and being unable to derive on recursive structs, until someone designs a way to filter only on types that depend on actual generic arguments or this issue is fixed. Or to use the wrapper type trick to delay execution, but that probably can't be generalized to any crate.

I'm curious, do you know why the default with proc-macro + syn seems to be "unable to derive on recursive structs"? (I'm referring to the fact that this simple code, where generics comes straight from DeriveInput, seems to set default where bounds based on field types)

taiki-e commented 1 year ago

Hmm I guess the choice is then currently between having the issues listed in #26925 and being unable to derive on recursive structs, until someone designs a way to filter only on types that depend on actual generic arguments or this issue is fixed. Or to use the wrapper type trick to delay execution, but that probably can't be generalized to any crate.

Well, even if the filter were implemented, I don't think the approach of using field types at the bounds of the where clause would work in some cases. e.g., that approach still does not interact well with structs that have private types. (see https://github.com/rust-lang/rust/issues/48054 for more. there is a workaround for that problem as well, but IIRC that workaround only works with auto-traits)

EDIT: see also https://github.com/dtolnay/syn/issues/370

this simple code

Those lines are not causing the problem. The problem is that the way of changing the where clause in the generate_fields_type_gen does not work well with the current Rust's type system.

CAD97 commented 1 year ago

it'd be a breaking change

Just to clarify the space here a bit,

It becomes breaking because code generation could rely on code not compiling to enforce soundness constraints. However, this applies to literally any change which results in more code compiling; the code generation which is broken by such a change was only tenuously sound in the first place, as it was relying on a satisfiable bound actually being unsatisfiable (an error).

The former case is provably sound and does not break; the impl is bounded on the condition which makes the impl sound, and if that condition does not hold, the impl does not apply. The "minor breaking" change is that the unsatisfiable impl goes from an immediate error on definition to only an error when trying to use the impl; this delayed communication is why I've suggested that trivially false bounds could be a deny-by-default lint.

Ekleog commented 1 year ago

@taiki-e

Those lines are not causing the problem. The problem is that the way of changing the where clause in the generate_fields_type_gen does not work well with the current Rust's type system.

Oh… I'm stupid I had missed the fact that make_where_clause returns a mutable reference… Sorry for the noise, and thank you!

(And so, for @CAD97 this explains why I was saying it was a breaking change in my message: I was thinking this behavior was coming from either syn or rustc, which was a wrong assumption)

joshtriplett commented 1 year ago

I'm trying to figure out the current state of this feature. What's the current status of trivial_bounds? Are there still blocking issues that would prevent this from being stabilized? Or does this just need a stabilization report and FCP?

The last update I found about blocking concerns, across the several issues about this, seems to be from 2019 talking about limitations in chalk.

dhedey commented 1 year ago

Just to add, I approve of the attributes here: https://github.com/rust-lang/rust/issues/48214#issuecomment-1164805614

I want to conditionally implement a Trait in a derive macro on a "transparent" wrapper style new-type depending on if a given Trait is implemented on the inner type. Naively making use of a where <concrete_inner_type>: Trait would allow for this behaviour, but the warns/compile errors get in the way.

I'd love to be able to add the where clause and for the above to just work - and the two annotations would allow this to work as-expected:

 #[allow(trivially_false_bounds)]
 #[allow(trivially_true_bounds)]

This would allow an easy way for macro-writers to avoid the problem of handling generics and avoiding https://github.com/rust-lang/rust/issues/26925

As a work-around in my case, I'll try introducing some new trait Wrapped with associated type Inner = X and add a blanket impl of Trait for Wrapped<Inner: Trait> - but other cases don't have such clear work-arounds. EDIT: Doesn't really work because standard impls of Trait fail with:

conflicting implementations of trait `Trait` for type `T`
downstream crates may implement trait `Wrapped<_>` for type `T`

EDIT: Looks like this workaround should work: https://github.com/rust-lang/rust/issues/48214#issuecomment-1374378038

cybersoulK commented 1 year ago

ran into this issue when trying to serialize my type with:

#[derive(borsh::BorshSerialize, borsh::BorshDeserialize)]
struct MyType {
   range: RangeInclusive<u32>
   ...
}
joshlf commented 1 year ago

Chiming in to provide another use case: zerocopy also falls into the category of crates which rely on trivially-false bounds failing in order to guarantee that our derive-emitted code is sound. E.g., all of the derives in this file should fail, but with trivial_bounds enabled, only one of them fails. As long as there's some way for us to fail compilation when a certain type doesn't implement a certain trait, then we're fine. #[deny(trivially_false_bounds)] sounds like it'd work, but from our perspective it doesn't matter if that's the solution or if a different solution is chosen.

One potential concern is that we already have versions of our crate published which don't use #[deny(trivially_false_bounds)], so depending on how this feature is stabilized, it could result in those versions of our crate becoming unsound.

cramertj commented 1 year ago

@joshlf To clarify, with trivial_bounds enabled, those derives will not trigger a compilation failure, but the resulting unsound trait implementations also won't be present because their bound is false, correct?

RalfJung commented 1 year ago

@joshlf if what you really want is "check if type T implements trait Trait, then I'd express this like this:

fn test_trait(_x: impl Trait) {}
fn test_t_trait(x: T) { test_trait(x) }

It doesn't seem necessary to rely on trivial trait bounds for that?

joshlf commented 1 year ago

Apologies, it looks like I misunderstood what this feature did: I was incorrectly assuming that trivial bounds were simply removed. I probably should have realized that that behavior makes no sense lol.

In case it's useful, here's the error message that I saw that made me think "oh, if I enable trivial_bounds, this trait impl will start compiling." While that's technically true, I didn't realize it'd compile because the trait would be considered unimplemented. Maybe there's a way to make this more clear in the error message?

error[E0277]: the trait bound `HasPadding<AsBytes2, true>: ShouldBe<false>` is not satisfied
  --> tests/ui-nightly/struct.rs:23:10
   |
23 | #[derive(AsBytes)]
   |          ^^^^^^^ the trait `ShouldBe<false>` is not implemented for `HasPadding<AsBytes2, true>`
   |
   = help: the trait `ShouldBe<true>` is implemented for `HasPadding<AsBytes2, true>`
   = help: see issue #48214
   = help: add `#![feature(trivial_bounds)]` to the crate attributes to enable
   = note: this error originates in the derive macro `AsBytes` (in Nightly builds, run with -Z macro-backtrace for more info)
joshlf commented 1 year ago

I may have found a bug. I changed the PR linked above to use static_assertions::assert_impl_all! to confirm that even if the derive compiles successfully, it doesn't emit an unsound impl. See e.g. this line. However, the assert_impl_all! invocations seem to succeed (see the compiler error output). It seems that the traits are not actually implemented (see ), but I would expect the assert_impl_all! invocations to fail.

* There is one error in the error output; that's due to the fact that we derive FromBytes, which is a sub-trait of FromZeroes, which is not implemented.

RalfJung commented 1 year ago

How does assert_impl_all! work?

joshlf commented 1 year ago

How does assert_impl_all! work?

It calls assert_impl! once for each trait:

macro_rules! assert_impl {
    (for($($generic:tt)*) $ty:ty: $($rest:tt)*) => {
        const _: () = {
            fn assert_impl<$($generic)*>() {
                // Construct an expression using `True`/`False` and their
                // operators, that corresponds to the provided expression.
                let _: $crate::True = $crate::_does_impl!($ty: $($rest)*);
            }
        };
    };
    ($ty:ty: $($rest:tt)*) => {
        // Construct an expression using `True`/`False` and their operators,
        // that corresponds to the provided expression.
        const _: $crate::True = $crate::_does_impl!($ty: $($rest)*);
    };
}

does_impl! is defined here:

/// Returns `True` or `False` depending on whether the given type implements the
/// given trait boolean expression. Can be used in const contexts if it doesn't
/// depend on outer generic parameters.
///
/// This is the core of `assert_impl`.
#[doc(hidden)]
#[macro_export(local_inner_macros)]
macro_rules! _does_impl {
    ($ty:ty: $($rest:tt)*) => {{
        #[allow(unused_imports)]
        use $crate::{
            _bool::{True, False},
            _core::{marker::PhantomData, ops::Deref},
        };

        // Fallback trait that returns false if the type does not implement a
        // given trait.
        trait DoesntImpl {
            const DOES_IMPL: False = False;
        }
        impl<T: ?Sized> DoesntImpl for T {}

        // Construct an expression using `True`/`False` and their operators,
        // that corresponds to the provided expression.
        *_does_impl!(@boolexpr($ty,) $($rest)*)
    }};

    (@boolexpr($($args:tt)*) ($($expr:tt)*)) => {
        _does_impl!(@boolexpr($($args)*) $($expr)*)
    };
    (@boolexpr($($args:tt)*) !($($expr:tt)*)) => {
        _does_impl!(@boolexpr($($args)*) $($expr)*).not()
    };
    (@boolexpr($($args:tt)*) ($($left:tt)*) | $($right:tt)*) => {{
        let left = _does_impl!(@boolexpr($($args)*) $($left)*);
        let right = _does_impl!(@boolexpr($($args)*) $($right)*);
        left.or(right)
    }};
    (@boolexpr($($args:tt)*) ($($left:tt)*) & $($right:tt)*) => {{
        let left = _does_impl!(@boolexpr($($args)*) $($left)*);
        let right = _does_impl!(@boolexpr($($args)*) $($right)*);
        left.and(right)
    }};
    (@boolexpr($($args:tt)*) !($($left:tt)*) | $($right:tt)*) => {{
        _does_impl!(@boolexpr($($args)*) (!($($left)*)) | $($right)*)
    }};
    (@boolexpr($($args:tt)*) !($($left:tt)*) & $($right:tt)*) => {{
        _does_impl!(@boolexpr($($args)*) (!($($left)*)) & $($right)*)
    }};
    (@boolexpr($($args:tt)*) !$left:ident | $($right:tt)*) => {{
        _does_impl!(@boolexpr($($args)*) !($left) | $($right)*)
    }};
    (@boolexpr($($args:tt)*) !$left:ident & $($right:tt)*) => {{
        _does_impl!(@boolexpr($($args)*) !($left) & $($right)*)
    }};
    (@boolexpr($($args:tt)*) $left:ident | $($right:tt)*) => {
        _does_impl!(@boolexpr($($args)*) ($left) | $($right)*)
    };
    (@boolexpr($($args:tt)*) $left:ident & $($right:tt)*) => {{
        _does_impl!(@boolexpr($($args)*) ($left) & $($right)*)
    }};
    (@boolexpr($($args:tt)*) !$expr:ident) => {
        _does_impl!(@boolexpr($($args)*) !($expr))
    };
    (@boolexpr($($args:tt)*) !$expr:path) => {
        _does_impl!(@boolexpr($($args)*) !($expr))
    };
    (@boolexpr($($args:tt)*) $expr:ident) => {
        _does_impl!(@base($($args)*) $expr)
    };
    (@boolexpr($($args:tt)*) $expr:path) => {
        _does_impl!(@base($($args)*) $expr)
    };

    (@base($ty:ty, $($args:tt)*) $($trait:tt)*) => {{
        // Base case: computes whether `ty` implements `trait`.
        struct Wrapper<T: ?Sized>(PhantomData<T>);

        #[allow(dead_code)]
        impl<T: ?Sized + $($trait)*> Wrapper<T> {
            const DOES_IMPL: True = True;
        }

        // If `$type: $trait`, the `_does_impl` inherent method on `Wrapper`
        // will be called, and return `True`. Otherwise, the trait method will
        // be called, which returns `False`.
        &<Wrapper<$ty>>::DOES_IMPL
    }};
}
RalfJung commented 12 months ago

That macro looks... interesting:

        const _: () = {
            fn assert_impl<$($generic)*>() {
                // Construct an expression using `True`/`False` and their
                // operators, that corresponds to the provided expression.
                let _: $crate::True = $crate::_does_impl!($ty: $($rest)*);
            }
        };

This is a generic function that is never called. Looks like with this feature we no longer type-check it properly?

I don't know what we guarantee about how much we type-check never-called generic functions.

It would help to distill the behavior you are seeing to a small macro-free example.