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

[aturon]

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

Steps:

• [x] Implement the RFC – https://github.com/rust-lang/rust/pull/48557
  
  
  
  • [ ] As noted in https://github.com/rust-lang/rust/issues/48214#issuecomment-396038764, Chalk would solve a few issues with this feature, so this is currently blocked on Chalk integration
  
  
• [ ] Adjust documentation (see instructions on forge)
• [ ] Stabilization PR (see instructions on forge)

Unresolved questions:

• [ ] Should the lint error by default instead of warn?

[matthewjasper]

I'm working on this.

[nikomatsakis]

@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]

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]

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]

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]

Not much has changed since my last comment.

[Systemcluster]

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]

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]

@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]

@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]

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]

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]

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]

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