|
| 1 | +- Feature Name: `allow_trivial_constraints` |
| 2 | +- Start Date: 2017-07-05 |
| 3 | +- RFC PR: https://github.com/rust-lang/rfcs/pull/2056 |
| 4 | +- Rust Issue: https://github.com/rust-lang/rust/issues/48214 |
| 5 | + |
| 6 | +# Summary |
| 7 | +[summary]: #summary |
| 8 | + |
| 9 | +Allow constraints to appear in where clauses which are trivially known to either |
| 10 | +always hold or never hold. This would mean that `impl Foo for Bar where i32: |
| 11 | +Iterator` would become valid, and the impl would never be satisfied. |
| 12 | + |
| 13 | +# Motivation |
| 14 | +[motivation]: #motivation |
| 15 | + |
| 16 | +It may seem strange to ever want to include a constraint that is always known to |
| 17 | +hold or not hold. However, as with many of these cases, allowing this would be |
| 18 | +useful for macros. For example, a custom derive may want to add additional |
| 19 | +functionality if two derives are used together. As another more concrete |
| 20 | +example, Diesel allows the use of normal Rust operators to generate the |
| 21 | +equivalent SQL. Due to coherence rules, we can't actually provide a blanket |
| 22 | +impl, but we'd like to automatically implement `std::ops::Add` for columns when |
| 23 | +they are of a type for which `+` is a valid operator. The generated impl would |
| 24 | +look like: |
| 25 | + |
| 26 | +```rust |
| 27 | +impl<T> std::ops::Add<T> for my_column |
| 28 | +where |
| 29 | + my_column::SqlType: diesel::types::ops::Add, |
| 30 | + T: AsExpression<<my_column::SqlType as diesel::types::ops::Add>::Rhs>, |
| 31 | +{ |
| 32 | + // ... |
| 33 | +} |
| 34 | +``` |
| 35 | + |
| 36 | +One would never write this impl normally since we always know the type of |
| 37 | +`my_column::SqlType`. However, when you consider the use case of a macro, we |
| 38 | +can't always easily know whether that constraint would hold or not at the time |
| 39 | +when we're generating code. |
| 40 | + |
| 41 | +# Detailed design |
| 42 | +[design]: #detailed-design |
| 43 | + |
| 44 | +Concretely implementing this means the removal of [`E0193`]. Interestingly, as of |
| 45 | +Rust 1.7, that error never actually appears. Instead the current behavior is |
| 46 | +that something like `impl Foo for Bar where i32: Copy` (e.g. anywhere that the |
| 47 | +constraint always holds) compiles fine, and `impl Foo for Bar where i32: |
| 48 | +Iterator` fails to compile by complaining that `i32` does not implement |
| 49 | +`Iterator`. The original error message explicitly forbidding this case does not |
| 50 | +seem to ever appear. |
| 51 | + |
| 52 | +The obvious complication that comes to mind when implementing this feature is |
| 53 | +that it would allow nonsensical projections to appear in the where clause as |
| 54 | +well. For example, when `i32: IntoIterator` appears in a where clause, we would |
| 55 | +also need to allow `i32::Item: SomeTrait` to appear in the same clause, and even |
| 56 | +allow `for _ in 1` to appear in item bodies, and have it all successfully |
| 57 | +compile. |
| 58 | + |
| 59 | +Since code that was caught by this error is usually nonsense outside of macros, |
| 60 | +it would be valuable for the error to continue to live on as a lint. The lint |
| 61 | +`trivial_constraints` would be added, matching the pre-1.7 semantics of E0193, |
| 62 | +and would be set to warn by default. |
| 63 | + |
| 64 | +[`E0193`]: https://doc.rust-lang.org/error-index.html#E0193 |
| 65 | + |
| 66 | +# How We Teach This |
| 67 | +[how-we-teach-this]: #how-we-teach-this |
| 68 | + |
| 69 | +This feature does not need to be taught explicitly. Knowing the basic rules of |
| 70 | +where clauses, one would naturally already expect this to work. |
| 71 | + |
| 72 | +# Drawbacks |
| 73 | +[drawbacks]: #drawbacks |
| 74 | + |
| 75 | +- The changes to the compiler could potentially increase complexity quite a bit |
| 76 | + |
| 77 | +# Alternatives |
| 78 | +[alternatives]: #alternatives |
| 79 | + |
| 80 | +n/a |
| 81 | + |
| 82 | +# Unresolved questions |
| 83 | +[unresolved]: #unresolved-questions |
| 84 | + |
| 85 | +Should the lint error by default instead of warn? |
0 commit comments