Silence unnecessary "missing dyn" errors and tweak E0746 suggestions

[estebank]

Rework impl Trait and Box return type suggestions in E0746

• If the return value types diverge, mention it and do not suggest impl Trait
• If the returned dyn Trait is not object safe, mention it and do not suggest Box<dyn Trait> and boxing the returned values
• If the function has arguments with borrowed types, suggest &dyn Trait
• Tweak wording of E0782

error[E0746]: return type cannot have an unboxed trait object
  --> $DIR/dyn-trait-return-should-be-impl-trait.rs:13:13
   |
LL | fn bap() -> Trait { Struct }
   |             ^^^^^ doesn't have a size known at compile-time
   |
help: return an `impl Trait` instead of a `dyn Trait`
   |
LL | fn bap() -> impl Trait { Struct }
   |             ++++
help: alternatively, box the return type to make a boxed trait object, and wrap all of the returned values in `Box::new`
   |
LL | fn bap() -> Box<dyn Trait> { Box::new(Struct) }
   |             +++++++      +   +++++++++      +

error[E0746]: return type cannot have an unboxed trait object
  --> $DIR/object-unsafe-trait-in-return-position-dyn-trait.rs:36:18
   |
LL | fn can(x: &A) -> dyn NotObjectSafe {
   |                  ^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time
   |
help: trait `NotObjectSafe` is not object safe, so you can't return a boxed trait object `Box<dyn NotObjectSafe>`
  --> $DIR/object-unsafe-trait-in-return-position-dyn-trait.rs:36:22
   |
LL | fn can(x: &A) -> dyn NotObjectSafe {
   |                      ^^^^^^^^^^^^^
help: return an `impl Trait` instead of a `dyn Trait`
   |
LL | fn can(x: &A) -> impl NotObjectSafe {
   |                  ~~~~
help: alternatively, you might be able to borrow from the function's argument
   |
LL | fn can(x: &A) -> &dyn NotObjectSafe {
   |                  +

error[E0782]: trait objects must include the `dyn` keyword
  --> $DIR/not-on-bare-trait-2021.rs:12:19
   |
LL | fn bar(x: Foo) -> Foo {
   |                   ^^^
   |
help: use `impl Foo` to return an opaque type, as long as you return a single underlying type
   |
LL | fn bar(x: Foo) -> impl Foo {
   |                   ++++
help: alternatively, you can return a boxed trait object
   |
LL | fn bar(x: Foo) -> Box<dyn Foo> {
   |                   +++++++    +

Silence "missing dyn" error if "?Sized type" error is already emitted.

Account for type Alias = dyn Trait; in unsized return suggestion:

error[E0746]: return type cannot have an unboxed trait object
 --> tests/ui/unsized/issue-91801.rs:8:77
  |
8 | fn or<'a>(first: &'static Validator<'a>, second: &'static Validator<'a>) -> Validator<'a> {
  |                                                                             ^^^^^^^^^^^^^ doesn't have a size known at compile-time
-Ztrack-diagnostics: created at compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs:566:39
  |
help: box the return type to make a boxed trait object, and wrap all of the returned values in `Box::new`
  |
8 | fn or<'a>(first: &'static Validator<'a>, second: &'static Validator<'a>) -> Box<Validator<'a>> {
  |                                                                             ++++             +
help: alternatively, you might be able to borrow from one of the function's arguments
  |
8 | fn or<'a>(first: &'static Validator<'a>, second: &'static Validator<'a>) -> &Validator<'a> {
  |                                                                             +

Use more complex logic for E0277 deduplication of FullfillmentErrors, accounting and customizing the span for SizedArgument obligations and functions with unsized return types. We only emit one unsized local error whenever possible.

Point at the let binding type or init expression instead of the pattern when possible for ?Sized errors.

error[E0277]: the size for values of type `[u8]` cannot be known at compilation time
  --> $DIR/unsized-locals-using-unsized-fn-params.rs:13:15
   |
LL |     let _foo: [u8] = *foo;
   |               ^^^^ doesn't have a size known at compile-time
   |
   = help: the trait `Sized` is not implemented for `[u8]`
   = note: all local variables must have a statically known size

Fix #121037. Fix #105753.

[rustbot]

r? @fee1-dead

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

[rustbot]

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

[rustbot]

HIR ty lowering was modified

cc @fmease

[fee1-dead]

I don't think I have the capacity to review this right now.

r? compiler

[compiler-errors]

I feel like it's incredibly easy to forget to use this function over .obligation.cause.span - can you consider another solution that doesn't give devs more choices that cause subtle inconsistencies when used differently?

[estebank]

The issue about forgetting to call the method is fair, but do notice that we're already duplicating logic to modify that span after the fact in the current code. I'm concerned about properly tracking the span from inception if that will require additional memory or have non-trivial runtime impact on non-diagnostic codepaths. I'll see what options we have available to us that don't have those problems.

The reason I wasn't too concerned about this approach is that the resulting behavior when forgetting to call this method is "just" a different span, in a different error, which tbqh felt "fine".

[estebank]

I had to still use a closure for the deduplication logic (as it is very local to this one E0277 error for Sized locals), but otherwise the obligation span is now the one we want for all errors.

[estebank]

Apologies for dumping more commits to the same PR. Each commit is locally independent (they could be their own PRs), but each individual step is building a tiny bit more on the previous one. I believe that there are may be a handful of additional things that could be done here (particularly, reduce redundancy between E0038 and E0277), but likely nothing more that should be added to this one. If the load is too much, feel free to review as many commits in a row as you feel comfortable, and I'll split the rest to their own PR(s).

[estebank]

Blocker: this needs to compile-pass

error[E0277]: the size for values of type `str` cannot be known at compilation time
 --> f108.rs:2:16
  |
2 |     let ref x: str = *"";
  |                ^^^ doesn't have a size known at compile-time
  |
  = help: the trait `Sized` is not implemented for `str`
  = note: all local variables must have a statically known size
  = help: unsized locals are gated as an unstable feature
help: consider borrowing here
  |
2 |     let ref x: &str = *"";
  |                +

Thanks @eddyb for flagging this.

Edit: addressed in the last commit. Only the top level ref ident pattern in a let binding needs to be accounted for, as any other let binding type that isn't sized already fails on the type itself.

[estebank]

This is weird: it should point at the type, not the init expr. If I modify the test to make this the only thing here, then it does the right thing. I'm not too concerned because of the close locality but it is technically incorrect (as the expr has type &[u8], the binding is [u8]).

[estebank]

r? @oli-obk

This is the PR we talked about earlier today. You can review each commit independently and I'll split them out to their own PR.

[oli-obk]

The function is called suggest_impl_trait, but this doesn't do that. Needs a name update and doc update.

Please split up this function into three functions (impl Trait, Box<dyn Trait>, and &dyn Trait).

[alex-semenyuk]

@estebank
Ping from triage. Could you please post your status on this PR? This PR has not been updated in more than 15 days.
It has merge conflicts and not addressed comment

[fmease]

They are on vacation rn and will return soon