From a103393bf1db287fe45e8f58293c9b2cc51f3638 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Wed, 5 Jul 2017 07:14:43 -0400 Subject: [PATCH 1/4] Allow trivial constraints to appear in where clauses --- ...-allow-trivial-where-clause-constraints.md | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 text/0000-allow-trivial-where-clause-constraints.md diff --git a/text/0000-allow-trivial-where-clause-constraints.md b/text/0000-allow-trivial-where-clause-constraints.md new file mode 100644 index 00000000000..45b749819a1 --- /dev/null +++ b/text/0000-allow-trivial-where-clause-constraints.md @@ -0,0 +1,80 @@ +- Feature Name: `allow_trivial_constraints` +- Start Date: 2017-07-05 +- RFC PR: (leave this empty) +- Rust Issue: (leave this empty) + +# Summary +[summary]: #summary + +Allow constraints to appear in where clauses which are trivially known to either +always hold or never hold. This would mean that `impl Foo for Bar where i32: +Iterator` would become valid, and the impl would never be satisfied. + +# Motivation +[motivation]: #motivation + +It may seem strange to ever want to include a constraint that is always known to +hold or not hold. However, as with many of these cases, allowing this would be +useful for macros. For example, a custom derive may want to add additional +functionality if two derives are used together. As another more concrete +example, Diesel allows the use of normal Rust operators to generate the +equivalent SQL. Due to coherence rules, we can't actually provide a blanket +impl, but we'd like to automatically implement `std::ops::Add` for columns when +they are of a type for which `+` is a valid operator. The generated impl would +look like: + +```rust +impl std::ops::Add for my_column +where + my_column::SqlType: diesel::types::ops::Add, + T: AsExpression<::Rhs>, +{ + // ... +} +``` + +One would never write this impl normally since we always know the type of +`my_column::SqlType`. However, when you consider the use case of a macro, we +can't always easily know whether that constraint would hold or not at the time +when we're generating code. + +# Detailed design +[design]: #detailed-design + +Concretely implementing this means the removal of `E0193`. Interestingly, as of +Rust 1.7, that error never actually appears. Instead the current behavior is +that something like `impl Foo for Bar where i32: Copy` (e.g. anywhere that the +constraint always holds) compiles fine, and `impl Foo for Bar where i32: +Iterator` fails to compile by complaining that `i32` does not implement +`Iterator`. The original error message explicitly forbidding this case does not +seem to ever appear. + +The obvious complication that comes to mind when implementing this feature is +that it would allow nonsensical projections to appear in the where clause as +well. For example, when `i32: IntoIterator` appears in a where clause, we would +also need to allow `i32::Item: SomeTrait` to appear in the same clause, and even +allow `for _ in 1` to appear in item bodies, and have it all successfully +compile. + +# How We Teach This +[how-we-teach-this]: #how-we-teach-this + +This feature does not need to be taught explicitly. Knowing the basic rules of +where clauses, one would naturally already expect this to work. + +# Drawbacks +[drawbacks]: #drawbacks + +- Code that is pretty obviously nonsense outside of the context of a macro or + derive would become valid. +- The changes to the compiler could potentially increase complexity quite a bit + +# Alternatives +[alternatives]: #alternatives + +n/a + +# Unresolved questions +[unresolved]: #unresolved-questions + +n/a From e4b0d2c2491473b709c200746d5f46c2cd100669 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Wed, 5 Jul 2017 07:16:39 -0400 Subject: [PATCH 2/4] Link to the error message for E0193 --- text/0000-allow-trivial-where-clause-constraints.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/text/0000-allow-trivial-where-clause-constraints.md b/text/0000-allow-trivial-where-clause-constraints.md index 45b749819a1..b42769fa0d5 100644 --- a/text/0000-allow-trivial-where-clause-constraints.md +++ b/text/0000-allow-trivial-where-clause-constraints.md @@ -41,7 +41,7 @@ when we're generating code. # Detailed design [design]: #detailed-design -Concretely implementing this means the removal of `E0193`. Interestingly, as of +Concretely implementing this means the removal of [`E0193`]. Interestingly, as of Rust 1.7, that error never actually appears. Instead the current behavior is that something like `impl Foo for Bar where i32: Copy` (e.g. anywhere that the constraint always holds) compiles fine, and `impl Foo for Bar where i32: @@ -56,6 +56,8 @@ also need to allow `i32::Item: SomeTrait` to appear in the same clause, and even allow `for _ in 1` to appear in item bodies, and have it all successfully compile. +[`E0193`]: https://doc.rust-lang.org/error-index.html#E0193 + # How We Teach This [how-we-teach-this]: #how-we-teach-this From 8c459173caea1352bc479a4274964f74aa0676f4 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Thu, 6 Jul 2017 08:06:13 -0400 Subject: [PATCH 3/4] Amend the RFC to specify that the error is replaced by a lint --- text/0000-allow-trivial-where-clause-constraints.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/text/0000-allow-trivial-where-clause-constraints.md b/text/0000-allow-trivial-where-clause-constraints.md index b42769fa0d5..1b919ea47c9 100644 --- a/text/0000-allow-trivial-where-clause-constraints.md +++ b/text/0000-allow-trivial-where-clause-constraints.md @@ -56,6 +56,11 @@ also need to allow `i32::Item: SomeTrait` to appear in the same clause, and even allow `for _ in 1` to appear in item bodies, and have it all successfully compile. +Since code that was caught by this error is usually nonsense outside of macros, +it would be valuable for the error to continue to live on as a lint. The lint +`trivial_constraints` would be added, matching the pre-1.7 semantics of E0193, +and would be set to warn by default. + [`E0193`]: https://doc.rust-lang.org/error-index.html#E0193 # How We Teach This @@ -67,8 +72,6 @@ where clauses, one would naturally already expect this to work. # Drawbacks [drawbacks]: #drawbacks -- Code that is pretty obviously nonsense outside of the context of a macro or - derive would become valid. - The changes to the compiler could potentially increase complexity quite a bit # Alternatives @@ -79,4 +82,4 @@ n/a # Unresolved questions [unresolved]: #unresolved-questions -n/a +Should the lint error by default instead of warn? From d82c89a4d3abfd24fce7d2025601acc870b053cc Mon Sep 17 00:00:00 2001 From: Aaron Turon Date: Wed, 14 Feb 2018 08:50:35 -0800 Subject: [PATCH 4/4] RFC 2056 --- ...ints.md => 2056-allow-trivial-where-clause-constraints.md} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename text/{0000-allow-trivial-where-clause-constraints.md => 2056-allow-trivial-where-clause-constraints.md} (96%) diff --git a/text/0000-allow-trivial-where-clause-constraints.md b/text/2056-allow-trivial-where-clause-constraints.md similarity index 96% rename from text/0000-allow-trivial-where-clause-constraints.md rename to text/2056-allow-trivial-where-clause-constraints.md index 1b919ea47c9..32dd09c46b8 100644 --- a/text/0000-allow-trivial-where-clause-constraints.md +++ b/text/2056-allow-trivial-where-clause-constraints.md @@ -1,7 +1,7 @@ - Feature Name: `allow_trivial_constraints` - Start Date: 2017-07-05 -- RFC PR: (leave this empty) -- Rust Issue: (leave this empty) +- RFC PR: https://github.com/rust-lang/rfcs/pull/2056 +- Rust Issue: https://github.com/rust-lang/rust/issues/48214 # Summary [summary]: #summary