Don't suggest restricting bound with unstable traits on stable and mention it's unstable on nightly by estebank · Pull Request #133522 · rust-lang/rust (original) (raw)
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation32 Commits10 Checks6 Files changed
Conversation
This file contains hidden or 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
[ Show hidden characters]({{ revealButtonHref }})
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`
Fix #133511.
r? @BoxyUwU
rustbot has assigned @BoxyUwU.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.
Use r? to explicitly pick a reviewer
Status: Awaiting review from the assignee but also interested parties.
Relevant to the compiler team, which will review and decide on the PR/issue.
labels
| @@ -6,7 +6,7 @@ LL | impl<A, B> FnOnce 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 |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe "although note that it is unstable" in the parentheses?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the "it" is the trait, not really the bound
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"but note that the trait is unstable"?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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:📑: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:📑:Tuple,
| ++++++++++++++++++++
note: the trait `Tuple` is unstable
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to "help: consider restricting type parameter T with unstable trait Unstable".
| @@ -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>` |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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...
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's fine either way, was just mentioning that it feels a bit redundant. Name only seems like a good compromise.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed (edit: but not on this file ? edit 2: tests/rustdoc-ui 🤦 edit 3: actually, just github linking to the older version 2x🤦). It adds a bunch of logic, but it does look better.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was just mentioning that it feels a bit redundant
Yeah, but that was a review request though. I feel like it might be useful for the sake of suggestions being presented without context. I don't feel strongly one way or another.
This PR modifies tests/run-make/. If this PR is trying to port a Makefile
run-make test to use rmake.rs, please update the
run-make port tracking issue
so we can track our progress. You can either modify the tracking issue
directly, or you can comment on the tracking issue and link this PR.
cc @jieyouxu
This comment has been minimized.
Some changes occurred to the CTFE machinery
cc @rust-lang/wg-const-eval
This comment has been minimized.
HIR ty lowering was modified
cc @fmease
📌 Commit 05e6714 has been approved by compiler-errors
It is now in the queue for this repository.
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
and removed S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
labels
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request
…it, r=compiler-errors
Don't suggest restricting bound with unstable traits on stable and mention it's unstable on nightly
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`Fix rust-lang#133511.
bors added a commit to rust-lang-ci/rust that referenced this pull request
…iaskrgr
Rollup of 8 pull requests
Successful merges:
- rust-lang#133089 (Stabilize noop_waker)
- rust-lang#133522 (Don't suggest restricting bound with unstable traits on stable and mention it's unstable on nightly)
- rust-lang#133545 (Lint against Symbol::intern on a string literal)
- rust-lang#133558 (Structurally resolve in
probe_adt) - rust-lang#133753 (Reduce false positives on some common cases from if-let-rescope lint)
- rust-lang#133762 (stabilize const_{size,align}_of_val)
- rust-lang#133777 (document -Zrandomize-layout in the unstable book)
- rust-lang#133779 (Use correct
hir_idfor array const arg infers)
r? @ghost
@rustbot modify labels: rollup
jhpratt added a commit to jhpratt/rust that referenced this pull request
…it, r=compiler-errors
Don't suggest restricting bound with unstable traits on stable and mention it's unstable on nightly
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`Fix rust-lang#133511.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request
…it, r=compiler-errors
Don't suggest restricting bound with unstable traits on stable and mention it's unstable on nightly
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`Fix rust-lang#133511.
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`
This test will break when Step gets stabilized, but punt until then.
help: consider restricting type parameter `T` with traits `Copy` and `Trait`
|
LL | fn duplicate_custom<T: Copy + Trait>(t: S<T>) -> (S<T>, S<T>) {
| ++++++++++++++help: consider restricting type parameter `V` with trait `Copy`
|
LL | fn index<'a, K, V: std:📑:Copy>(map: &'a HashMap<K, V>, k: K) -> &'a V {
| +++++++++++++++++++
Pass in an appropriate Option<DefId> in more cases from hir ty lowering.
📌 Commit 25ad047 has been approved by compiler-errors
It is now in the queue for this repository.
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request
…it, r=compiler-errors
Don't suggest restricting bound with unstable traits on stable and mention it's unstable on nightly
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`Fix rust-lang#133511.
bors added a commit to rust-lang-ci/rust that referenced this pull request
…kingjubilee
Rollup of 10 pull requests
Successful merges:
- rust-lang#133522 (Don't suggest restricting bound with unstable traits on stable and mention it's unstable on nightly)
- rust-lang#133733 ( compiletest: show the difference between the normalized output and the actual output for lines which didn't match)
- rust-lang#133861 (Add allocate_bytes and refactor allocate_str in InterpCx for raw byte…)
- rust-lang#133967 ([AIX] Pass -bnoipath when adding rust upstream dynamic crates)
- rust-lang#133976 (Removed Unnecessary Spaces From RELEASES.md)
- rust-lang#133980 ([AIX] Remove option "-n" from AIX "ln" command)
- rust-lang#133987 (Define acronym for thread local storage)
- rust-lang#133992 (Actually walk into lifetimes and attrs in
EarlyContextAndPass) - rust-lang#133993 (Fix: typo in E0751 error explanation)
- rust-lang#133996 (Move most tests for
-land#[link(..)]intotests/ui/link-native-libs)
r? @ghost
@rustbot modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request
…kingjubilee
Rollup of 10 pull requests
Successful merges:
- rust-lang#133522 (Don't suggest restricting bound with unstable traits on stable and mention it's unstable on nightly)
- rust-lang#133733 ( compiletest: show the difference between the normalized output and the actual output for lines which didn't match)
- rust-lang#133861 (Add allocate_bytes and refactor allocate_str in InterpCx for raw byte…)
- rust-lang#133967 ([AIX] Pass -bnoipath when adding rust upstream dynamic crates)
- rust-lang#133976 (Removed Unnecessary Spaces From RELEASES.md)
- rust-lang#133980 ([AIX] Remove option "-n" from AIX "ln" command)
- rust-lang#133987 (Define acronym for thread local storage)
- rust-lang#133992 (Actually walk into lifetimes and attrs in
EarlyContextAndPass) - rust-lang#133993 (Fix: typo in E0751 error explanation)
- rust-lang#133996 (Move most tests for
-land#[link(..)]intotests/ui/link-native-libs)
r? @ghost
@rustbot modify labels: rollup
actually conflict-prone it seems
@bors rollup=iffy
bors added a commit to rust-lang-ci/rust that referenced this pull request
…kingjubilee
Rollup of 5 pull requests
Successful merges:
- rust-lang#131669 (lint: change help for pointers to dyn types in FFI)
- rust-lang#133184 (wasi/fs: Improve stopping condition for ::next)
- rust-lang#133265 (Add a range argument to vec.extract_if)
- rust-lang#133456 (Add licenses + Run
cargo update) - rust-lang#133767 (Add more info on type/trait mismatches for different crate versions)
Failed merges:
- rust-lang#133522 (Don't suggest restricting bound with unstable traits on stable and mention it's unstable on nightly)
r? @ghost
@rustbot modify labels: rollup
@bors p=1 (conflict-prone)
This was referenced
Dec 8, 2024
Finished benchmarking commit (f415c07): comparison URL.
Overall result: ❌ regressions - no action needed
@rustbot label: -perf-regression
Instruction count
This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.
| mean | range | count | |
|---|---|---|---|
| Regressions ❌ (primary) | - | - | 0 |
| Regressions ❌ (secondary) | 2.3% | [2.3%, 2.3%] | 1 |
| Improvements ✅ (primary) | - | - | 0 |
| Improvements ✅ (secondary) | - | - | 0 |
| All ❌✅ (primary) | - | - | 0 |
Max RSS (memory usage)
Results (primary 0.7%)
This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
| mean | range | count | |
|---|---|---|---|
| Regressions ❌ (primary) | 2.8% | [2.8%, 2.8%] | 1 |
| Regressions ❌ (secondary) | - | - | 0 |
| Improvements ✅ (primary) | -1.4% | [-1.4%, -1.4%] | 1 |
| Improvements ✅ (secondary) | - | - | 0 |
| All ❌✅ (primary) | 0.7% | [-1.4%, 2.8%] | 2 |
Cycles
This benchmark run did not return any relevant results for this metric.
Binary size
This benchmark run did not return any relevant results for this metric.
Bootstrap: 766.806s -> 766.757s (-0.01%)
Artifact size: 330.83 MiB -> 330.86 MiB (0.01%)
Labels
Area: port run-make Makefiles to rmake.rs
This PR was explicitly merged by bors.
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Relevant to the compiler team, which will review and decide on the PR/issue.
The Rustc Trait System Refactor Initiative (-Znext-solver)