Detect when method call on argument could be removed to fulfill failed trait bound #121100

estebank · 2024-02-14T17:49:22Z

When encountering

struct Foo;
struct Bar;
impl From<Bar> for Foo {
    fn from(_: Bar) -> Self { Foo }
}
fn qux(_: impl From<Bar>) {}
fn main() {
    qux(Bar.into());
}

Suggest removing .into():

error[E0283]: type annotations needed
 --> f100.rs:8:13
  |
8 |     qux(Bar.into());
  |     ---     ^^^^
  |     |
  |     required by a bound introduced by this call
  |
  = note: cannot satisfy `_: From<Bar>`
note: required by a bound in `qux`
 --> f100.rs:6:16
  |
6 | fn qux(_: impl From<Bar>) {}
  |                ^^^^^^^^^ required by this bound in `qux`
help: try using a fully qualified path to specify the expected types
  |
8 |     qux(<Bar as Into<T>>::into(Bar));
  |         +++++++++++++++++++++++   ~
help: consider removing this method call, as the receiver has type `Bar` and `Bar: From<Bar>` trivially holds
  |
8 -     qux(Bar.into());
8 +     qux(Bar);
  |

Fix #71252

rustbot · 2024-02-14T17:49:30Z

r? @wesleywiser

rustbot has assigned @wesleywiser.
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

compiler-errors · 2024-02-14T19:15:10Z

compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs

+                    expr.span.with_lo(rcvr.span.hi()),
+                    format!(
+                        "consider removing this method call, as the receiver has type `{ty}` and \
+                         `{pred}` can be fulfilled",


Suggested change

`{pred}` can be fulfilled",

`{pred}` is trivial",

or "trivially holds"

@compiler-errors do we ever use that wording in user-facing messages? AFAICT we only use that wording for other compiler devs. (We can argue that "can be fulfilled" has the same problem.) Maybe "{pred} is met"?

I could say the same about "fulfilled". 😆 don't believe the original wording gets across the fact that the method call is redundant, which is what I think this should be pointing out.

The point that I'm trying to make is that the wording should explain to the user why removing the method call is beneficial, which I don't believe it is doing right now; rn it's just stating facts

@compiler-errors yeah, that makes sense. Initially I was thinking "well, there could be cases where the predicate is actually more complex and the Self is something like an assoc type or impl Trait", but thinking about it for just a minute was enough to realize that you're right and "yes, every case where this would work would be a trivial obligation".

I like "is trivial" better than "can be fulfilled" 👍

I wonder if something even simpler would work ("is valid" or "is true" perhaps)?

compiler-errors

r=me after nits

compiler-errors · 2024-02-14T23:31:33Z

compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs

+            if let hir::Expr { kind: hir::ExprKind::MethodCall(_, rcvr, _, _), .. } = expr
+                && let Some(ty) = typeck_results.node_type_opt(rcvr.hir_id)
+                && let Some(failed_pred) = failed_pred.to_opt_poly_trait_pred()
+                && let pred = failed_pred.skip_binder().with_self_ty(tcx, ty)


use map_bound here. otherwise, it's gonna ICE because you're skipping binders.

compiler-errors · 2024-02-14T23:32:02Z

compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs

+                && let Some(failed_pred) = failed_pred.to_opt_poly_trait_pred()
+                && let pred = failed_pred.skip_binder().with_self_ty(tcx, ty)
+                && self
+                    .evaluate_obligation_no_overflow(&Obligation::misc(


why did you use evaluate_obligation_no_overflow here? please use predicate_may_hold_modulo_regions.

estebank · 2024-02-16T03:28:43Z

@bors r=compiler-errors

bors · 2024-02-16T03:28:46Z

📌 Commit 5a1c454 has been approved by compiler-errors

It is now in the queue for this repository.

compiler-errors · 2024-02-16T03:31:42Z

Why did you use predicate_may_hold? That can hold even if there is inference that still needs to be made. Could you use predicate_may_hold_modulo_regions? False positives here feel like they would be really confusing.

@bors r-

compiler-errors · 2024-02-16T03:43:48Z

Also, is there a reason why you left out the test from the PR description? None of the UI tests that were changed are trait ambiguity errors, just regular trait failures. Please add that test, too.

…d trait bound When encountering ```rust struct Foo; struct Bar; impl From<Bar> for Foo { fn from(_: Bar) -> Self { Foo } } fn qux(_: impl From<Bar>) {} fn main() { qux(Bar.into()); } ``` Suggest removing `.into()`: ``` error[E0283]: type annotations needed --> f100.rs:8:13 | 8 | qux(Bar.into()); | --- ^^^^ | | | required by a bound introduced by this call | = note: cannot satisfy `_: From<Bar>` note: required by a bound in `qux` --> f100.rs:6:16 | 6 | fn qux(_: impl From<Bar>) {} | ^^^^^^^^^ required by this bound in `qux` help: try using a fully qualified path to specify the expected types | 8 | qux(<Bar as Into<T>>::into(Bar)); | +++++++++++++++++++++++ ~ help: consider removing this method call, as the receiver has type `Bar` and `Bar: From<Bar>` can be fulfilled | 8 - qux(Bar.into()); 8 + qux(Bar); | ``` Fix rust-lang#71252

estebank · 2024-02-16T18:33:26Z

Updated

@bors r=compiler-errors

bors · 2024-02-16T18:33:29Z

📌 Commit 9b3fcf9 has been approved by compiler-errors

It is now in the queue for this repository.

estebank · 2024-02-16T18:35:42Z

Why did you use predicate_may_hold? That can hold even if there is inference that still needs to be made. Could you use predicate_may_hold_modulo_regions?

I got confused between predicate_may_hold and predicate_must_hold_modulo_regions. That's what I get for multitasking paying half attention.

…rrors Detect when method call on argument could be removed to fulfill failed trait bound When encountering ```rust struct Foo; struct Bar; impl From<Bar> for Foo { fn from(_: Bar) -> Self { Foo } } fn qux(_: impl From<Bar>) {} fn main() { qux(Bar.into()); } ``` Suggest removing `.into()`: ``` error[E0283]: type annotations needed --> f100.rs:8:13 | 8 | qux(Bar.into()); | --- ^^^^ | | | required by a bound introduced by this call | = note: cannot satisfy `_: From<Bar>` note: required by a bound in `qux` --> f100.rs:6:16 | 6 | fn qux(_: impl From<Bar>) {} | ^^^^^^^^^ required by this bound in `qux` help: try using a fully qualified path to specify the expected types | 8 | qux(<Bar as Into<T>>::into(Bar)); | +++++++++++++++++++++++ ~ help: consider removing this method call, as the receiver has type `Bar` and `Bar: From<Bar>` trivially holds | 8 - qux(Bar.into()); 8 + qux(Bar); | ``` Fix rust-lang#71252

…llaumeGomez Rollup of 7 pull requests Successful merges: - rust-lang#118264 (Optimize `VecDeque::drain` for (half-)open ranges) - rust-lang#121079 (distribute tool documentations and avoid file conflicts on `x install`) - rust-lang#121100 (Detect when method call on argument could be removed to fulfill failed trait bound) - rust-lang#121160 (rustdoc: fix and refactor HTML rendering a bit) - rust-lang#121198 (Add more checks for `unnamed_fields` during HIR analysis) - rust-lang#121221 (AstConv: Refactor lowering of associated item bindings a bit) - rust-lang#121237 (Use better heuristic for printing Cargo specific diagnostics) r? `@ghost` `@rustbot` modify labels: rollup

Rollup of 7 pull requests Successful merges: - rust-lang#120526 (rustdoc: Correctly handle long crate names on mobile) - rust-lang#121100 (Detect when method call on argument could be removed to fulfill failed trait bound) - rust-lang#121160 (rustdoc: fix and refactor HTML rendering a bit) - rust-lang#121198 (Add more checks for `unnamed_fields` during HIR analysis) - rust-lang#121218 (Fix missing trait impls for type in rustc docs) - rust-lang#121221 (AstConv: Refactor lowering of associated item bindings a bit) - rust-lang#121237 (Use better heuristic for printing Cargo specific diagnostics) r? `@ghost` `@rustbot` modify labels: rollup

…rrors Detect when method call on argument could be removed to fulfill failed trait bound When encountering ```rust struct Foo; struct Bar; impl From<Bar> for Foo { fn from(_: Bar) -> Self { Foo } } fn qux(_: impl From<Bar>) {} fn main() { qux(Bar.into()); } ``` Suggest removing `.into()`: ``` error[E0283]: type annotations needed --> f100.rs:8:13 | 8 | qux(Bar.into()); | --- ^^^^ | | | required by a bound introduced by this call | = note: cannot satisfy `_: From<Bar>` note: required by a bound in `qux` --> f100.rs:6:16 | 6 | fn qux(_: impl From<Bar>) {} | ^^^^^^^^^ required by this bound in `qux` help: try using a fully qualified path to specify the expected types | 8 | qux(<Bar as Into<T>>::into(Bar)); | +++++++++++++++++++++++ ~ help: consider removing this method call, as the receiver has type `Bar` and `Bar: From<Bar>` trivially holds | 8 - qux(Bar.into()); 8 + qux(Bar); | ``` Fix rust-lang#71252

Rollup merge of rust-lang#121100 - estebank:issue-71252, r=compiler-errors Detect when method call on argument could be removed to fulfill failed trait bound When encountering ```rust struct Foo; struct Bar; impl From<Bar> for Foo { fn from(_: Bar) -> Self { Foo } } fn qux(_: impl From<Bar>) {} fn main() { qux(Bar.into()); } ``` Suggest removing `.into()`: ``` error[E0283]: type annotations needed --> f100.rs:8:13 | 8 | qux(Bar.into()); | --- ^^^^ | | | required by a bound introduced by this call | = note: cannot satisfy `_: From<Bar>` note: required by a bound in `qux` --> f100.rs:6:16 | 6 | fn qux(_: impl From<Bar>) {} | ^^^^^^^^^ required by this bound in `qux` help: try using a fully qualified path to specify the expected types | 8 | qux(<Bar as Into<T>>::into(Bar)); | +++++++++++++++++++++++ ~ help: consider removing this method call, as the receiver has type `Bar` and `Bar: From<Bar>` trivially holds | 8 - qux(Bar.into()); 8 + qux(Bar); | ``` Fix rust-lang#71252

rustbot assigned wesleywiser Feb 14, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 14, 2024

compiler-errors reviewed Feb 14, 2024

View reviewed changes

estebank force-pushed the issue-71252 branch from 9a1d514 to a71a218 Compare February 14, 2024 19:41

compiler-errors requested changes Feb 14, 2024

View reviewed changes

compiler-errors assigned compiler-errors and unassigned wesleywiser Feb 15, 2024

compiler-errors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 15, 2024

estebank force-pushed the issue-71252 branch from a71a218 to 5a1c454 Compare February 16, 2024 03:28

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 16, 2024

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 16, 2024

This comment has been minimized.

Sign in to view

estebank force-pushed the issue-71252 branch from 5a1c454 to 9b3fcf9 Compare February 16, 2024 04:37

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 16, 2024

Nadrieril mentioned this pull request Feb 17, 2024

Rollup of 7 pull requests #121222

Closed

GuillaumeGomez mentioned this pull request Feb 17, 2024

Rollup of 7 pull requests #121244

Closed

fmease mentioned this pull request Feb 18, 2024

Rollup of 7 pull requests #121252

Merged

CKingX mentioned this pull request Feb 18, 2024

Rollup of 6 pull requests #121254

Closed

bors merged commit 6499eb5 into rust-lang:master Feb 18, 2024
11 checks passed

rustbot added this to the 1.78.0 milestone Feb 18, 2024

estebank mentioned this pull request Mar 7, 2024

Suggest removing .into() for argument that already accepts Into<_> #52837

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Detect when method call on argument could be removed to fulfill failed trait bound #121100

Detect when method call on argument could be removed to fulfill failed trait bound #121100

estebank commented Feb 14, 2024 •

edited

Loading

rustbot commented Feb 14, 2024

compiler-errors Feb 14, 2024

estebank Feb 14, 2024 •

edited

Loading

compiler-errors Feb 14, 2024 •

edited

Loading

compiler-errors Feb 14, 2024

estebank Feb 14, 2024

wesleywiser Feb 14, 2024

compiler-errors left a comment

compiler-errors Feb 14, 2024

compiler-errors Feb 14, 2024

estebank commented Feb 16, 2024

bors commented Feb 16, 2024

compiler-errors commented Feb 16, 2024

compiler-errors commented Feb 16, 2024

This comment has been minimized.

estebank commented Feb 16, 2024

bors commented Feb 16, 2024

estebank commented Feb 16, 2024

Detect when method call on argument could be removed to fulfill failed trait bound #121100

Detect when method call on argument could be removed to fulfill failed trait bound #121100

Conversation

estebank commented Feb 14, 2024 • edited Loading

rustbot commented Feb 14, 2024

compiler-errors Feb 14, 2024

Choose a reason for hiding this comment

estebank Feb 14, 2024 • edited Loading

Choose a reason for hiding this comment

compiler-errors Feb 14, 2024 • edited Loading

Choose a reason for hiding this comment

compiler-errors Feb 14, 2024

Choose a reason for hiding this comment

estebank Feb 14, 2024

Choose a reason for hiding this comment

wesleywiser Feb 14, 2024

Choose a reason for hiding this comment

compiler-errors left a comment

Choose a reason for hiding this comment

compiler-errors Feb 14, 2024

Choose a reason for hiding this comment

compiler-errors Feb 14, 2024

Choose a reason for hiding this comment

estebank commented Feb 16, 2024

bors commented Feb 16, 2024

compiler-errors commented Feb 16, 2024

compiler-errors commented Feb 16, 2024

This comment has been minimized.

estebank commented Feb 16, 2024

bors commented Feb 16, 2024

estebank commented Feb 16, 2024

estebank commented Feb 14, 2024 •

edited

Loading

estebank Feb 14, 2024 •

edited

Loading

compiler-errors Feb 14, 2024 •

edited

Loading