Don't suggest restricting bound with unstable traits on stable and mention it's unstable on nightly #133522
+426
−316
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
S-waiting-on-review
| @@ -6,7 +6,7 @@ LL | impl<A, B> FnOnce<A> for CachedFun<A, B> | |||
| | | |||
| note: required by a bound in `FnOnce` | |||
| --> $SRC_DIR/core/src/ops/function.rs:LL:COL | |||
| help: consider further restricting this bound | |||
| help: consider further restricting this bound but it is an `unstable` trait | |||
There was a problem hiding this comment.
nit: unstable is not a keyword, and "but" feels awkward here... maybe put it in parens and use "although" instead?
consider further restricting this bound (although this is an unstable trait)
There was a problem hiding this comment.
maybe "although note that it is unstable" in the parentheses?
There was a problem hiding this comment.
I also think the "it" here is a bit unclear, on first reading I thought "it" referred to T and was then thrown off by T being an unstable trait
Perhaps the trait could be named or something like "the new bound" or "the bound added below" be used instead?
There was a problem hiding this comment.
the "it" is the trait, not really the bound
There was a problem hiding this comment.
"but note that the trait is unstable"?
There was a problem hiding this comment.
sure, but if you're going to use "but" please still put it in parentheses or at least add a comma, since it's still a bit awkward grammatically
There was a problem hiding this comment.
Drive-by comment: perhaps it would be easier if the types were explicitly named? For example:
error[E0277]: functions with the "rust-call" ABI must take a single non-self tuple argument
--> $DIR/rust-call-abi-not-a-tuple-ice-81974.rs:31:5
|
LL | extern "rust-call" fn call_once(mut self, a: A) -> Self::Output {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Tuple` is not implemented for `A`
|
help: consider adding the unstable trait `Tuple` to the bounds of `A`
|
LL | A: Eq + Hash + Clone + std::marker::Tuple,
| ++++++++++++++++++++
Then the stability or non-stability can be added via the phrase "consider adding the trait Tuple to the bounds of A" / "consider adding the unstable trait Tuple to the bounds of A`".
Alternatively, it could be suggested via a separate note:
error[E0277]: functions with the "rust-call" ABI must take a single non-self tuple argument
--> $DIR/rust-call-abi-not-a-tuple-ice-81974.rs:31:5
|
LL | extern "rust-call" fn call_once(mut self, a: A) -> Self::Output {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Tuple` is not implemented for `A`
|
help: consider adding the trait `Tuple` to the bounds of `A`
|
LL | A: Eq + Hash + Clone + std::marker::Tuple,
| ++++++++++++++++++++
note: the trait `Tuple` is unstable
There was a problem hiding this comment.
Changed to "help: consider restricting type parameter T with unstable trait Unstable".
On nightly, we mention the trait is unstable
```
error[E0277]: the trait bound `T: Unstable` is not satisfied
--> $DIR/unstable-trait-suggestion.rs:13:9
|
LL | foo(t)
| --- ^ the trait `Unstable` is not implemented for `T`
| |
| required by a bound introduced by this call
|
note: required by a bound in `foo`
--> $DIR/unstable-trait-suggestion.rs:9:11
|
LL | fn foo<T: Unstable>(_: T) {}
| ^^^^^^^^ required by this bound in `foo`
help: consider restricting type parameter `T` but it is an `unstable` trait
|
LL | pub fn demo<T: Unstable>(t: T) {
| ++++++++++
```
On stable, we don't suggest the trait at all
```
error[E0277]: the trait bound `T: Unstable` is not satisfied
--> $DIR/unstable-trait-suggestion.rs:13:9
|
LL | foo(t)
| --- ^ the trait `Unstable` is not implemented for `T`
| |
| required by a bound introduced by this call
|
note: required by a bound in `foo`
--> $DIR/unstable-trait-suggestion.rs:9:11
|
LL | fn foo<T: Unstable>(_: T) {}
| ^^^^^^^^ required by this bound in `foo`
```
estebank
force-pushed
the
dont-suggest-unstable-trait
branch
from
932462b
to
e3dfae8
Compare
rustbot
added
the
WG-trait-system-refactor
| @@ -4,7 +4,7 @@ error[E0277]: the trait bound `C: Bar<5>` is not satisfied | |||
| LL | pub struct Structure<C: Tec> { | |||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Bar<5>` is not implemented for `C` | |||
| | | |||
| help: consider further restricting this bound | |||
| help: consider further restricting this bound with trait `Bar<5>` | |||
There was a problem hiding this comment.
Could you change this to use print_only_trait_name rather than printing the binder and the args?
I first of all don't think mentioning the name is really that useful, since it's already there in the suggestion, but if we're going to mention it at all, it's probably only worthwhile to suggest only the trait name.
There was a problem hiding this comment.
Rendering the binder and substs runs the risk of printing a really overly long trait ref here, since we may have large types in the args.
There was a problem hiding this comment.
We could only mention the trait for unsafe ones (as I originally intended in the PR, to keep the diff small).
The only issue against using the "name only" is when we have more than one bound being suggested, but we have to handle that anyways...
There was a problem hiding this comment.
I think it's fine either way, was just mentioning that it feels a bit redundant. Name only seems like a good compromise.
This test will break when `Step` gets stabilized, but punt until then.
|
This PR modifies cc @jieyouxu |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
On nightly, we mention the trait is unstable
On stable, we don't suggest the trait at all
Fix #133511.