Skip to content
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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Note that the caller chooses a type for type param #122195

Merged
merged 1 commit into from
Mar 23, 2024
Merged

Note that the caller chooses a type for type param #122195

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions compiler/rustc_hir_typeck/messages.ftl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ hir_typeck_no_associated_item = no {$item_kind} named `{$item_name}` found for {
*[other] {" "}in the current scope
}

hir_typeck_note_caller_chooses_ty_for_ty_param = the caller chooses a type for `{$ty_param_name}` which can be different from `{$found_ty}`

hir_typeck_note_edition_guide = for more on editions, read https://doc.rust-lang.org/edition-guide

hir_typeck_option_result_asref = use `{$def_path}::as_ref` to convert `{$expected_ty}` to `{$expr_ty}`
Expand Down
9 changes: 8 additions & 1 deletion compiler/rustc_hir_typeck/src/errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use rustc_middle::ty::Ty;
use rustc_span::{
edition::{Edition, LATEST_STABLE_EDITION},
symbol::Ident,
Span,
Span, Symbol,
};

#[derive(Diagnostic)]
Expand Down Expand Up @@ -614,3 +614,10 @@ pub struct SuggestConvertViaMethod<'tcx> {
pub expected: Ty<'tcx>,
pub found: Ty<'tcx>,
}

#[derive(Subdiagnostic)]
#[note(hir_typeck_note_caller_chooses_ty_for_ty_param)]
pub struct NoteCallerChoosesTyForTyParam<'tcx> {
pub ty_param_name: Symbol,
pub found_ty: Ty<'tcx>,
}
20 changes: 19 additions & 1 deletion compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -889,7 +889,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.dcx(),
errors::ExpectedReturnTypeLabel::Other { span: hir_ty.span, expected },
);
self.try_suggest_return_impl_trait(err, expected, ty, fn_id);
self.try_suggest_return_impl_trait(err, expected, found, fn_id);
Copy link
Member

@fmease fmease Mar 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I wonder if we should add this note in more cases, not just in cases where we suggest RPITs. For example here:

fn f<T>() -> (T,) {
    (0,)
}

Copy link
Member Author

@jieyouxu jieyouxu Mar 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait how do u undo a r= lol (if you want me to add it in more places in this PR)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, let's do this!

self.note_caller_chooses_ty_for_ty_param(err, expected, found);
return true;
}
}
Expand All @@ -899,6 +900,23 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
false
}

fn note_caller_chooses_ty_for_ty_param(
&self,
diag: &mut Diag<'_>,
expected: Ty<'tcx>,
found: Ty<'tcx>,
) {
if let ty::Param(expected_ty_as_param) = expected.kind() {
diag.subdiagnostic(
self.dcx(),
errors::NoteCallerChoosesTyForTyParam {
ty_param_name: expected_ty_as_param.name,
found_ty: found,
},
);
}
}

/// check whether the return type is a generic type with a trait bound
/// only suggest this if the generic param is not present in the arguments
/// if this is true, hint them towards changing the return type to `impl Trait`
Expand Down
1 change: 1 addition & 0 deletions src/tools/clippy/tests/ui/builtin_type_shadow.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ LL | 42
|
= note: expected type parameter `u32`
found type `{integer}`
= note: the caller chooses a type for `u32` which can be different from `i32`

error: aborting due to 2 previous errors

Expand Down
4 changes: 4 additions & 0 deletions tests/ui/return/return-impl-trait-bad.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ LL | "this should not suggest impl Trait"
|
= note: expected type parameter `T`
found reference `&'static str`
= note: the caller chooses a type for `T` which can be different from `&'static str`

error[E0308]: mismatched types
--> $DIR/return-impl-trait-bad.rs:9:5
Expand All @@ -23,6 +24,7 @@ LL | "this will not suggest it, because that would probably be wrong"
|
= note: expected type parameter `T`
found reference `&'static str`
= note: the caller chooses a type for `T` which can be different from `&'static str`

error[E0308]: mismatched types
--> $DIR/return-impl-trait-bad.rs:17:5
Expand All @@ -37,6 +39,7 @@ LL | "don't suggest this, because Option<T> places additional constraints"
|
= note: expected type parameter `T`
found reference `&'static str`
= note: the caller chooses a type for `T` which can be different from `&'static str`

error[E0308]: mismatched types
--> $DIR/return-impl-trait-bad.rs:28:5
Expand All @@ -53,6 +56,7 @@ LL | "don't suggest this, because the generic param is used in the bound."
|
= note: expected type parameter `T`
found reference `&'static str`
= note: the caller chooses a type for `T` which can be different from `&'static str`

error: aborting due to 4 previous errors

Expand Down
2 changes: 2 additions & 0 deletions tests/ui/return/return-impl-trait.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ LL | ()
|
= note: expected type parameter `T`
found unit type `()`
= note: the caller chooses a type for `T` which can be different from `()`

error[E0308]: mismatched types
--> $DIR/return-impl-trait.rs:23:5
Expand All @@ -28,6 +29,7 @@ LL | ()
|
= note: expected type parameter `T`
found unit type `()`
= note: the caller chooses a type for `T` which can be different from `()`

error: aborting due to 2 previous errors

Expand Down
21 changes: 21 additions & 0 deletions tests/ui/return/return-ty-mismatch-note.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Checks existence of a note for "a caller chooses ty for ty param" upon return ty mismatch.

fn f<T>() -> (T,) {
(0,) //~ ERROR mismatched types
}

fn g<U, V>() -> (U, V) {
(0, "foo")
//~^ ERROR mismatched types
//~| ERROR mismatched types
}

fn h() -> u8 {
0u8
}

fn main() {
f::<()>();
g::<(), ()>;
let _ = h();
}
36 changes: 36 additions & 0 deletions tests/ui/return/return-ty-mismatch-note.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
error[E0308]: mismatched types
--> $DIR/return-ty-mismatch-note.rs:4:6
|
LL | fn f<T>() -> (T,) {
| - expected this type parameter
LL | (0,)
| ^ expected type parameter `T`, found integer
|
= note: expected type parameter `T`
found type `{integer}`

error[E0308]: mismatched types
--> $DIR/return-ty-mismatch-note.rs:8:6
|
LL | fn g<U, V>() -> (U, V) {
| - expected this type parameter
LL | (0, "foo")
| ^ expected type parameter `U`, found integer
|
= note: expected type parameter `U`
found type `{integer}`

error[E0308]: mismatched types
--> $DIR/return-ty-mismatch-note.rs:8:9
|
LL | fn g<U, V>() -> (U, V) {
| - expected this type parameter
LL | (0, "foo")
| ^^^^^ expected type parameter `V`, found `&str`
|
= note: expected type parameter `V`
found reference `&'static str`

error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0308`.
1 change: 1 addition & 0 deletions tests/ui/suggestions/clone-on-unconstrained-borrowed-type-param.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ LL | t.clone()
|
= note: expected type parameter `_`
found reference `&_`
= note: the caller chooses a type for `T` which can be different from `&T`
note: `T` does not implement `Clone`, so `&T` was cloned instead
--> $DIR/clone-on-unconstrained-borrowed-type-param.rs:3:5
|
Expand Down
1 change: 1 addition & 0 deletions tests/ui/trait-bounds/restrict-assoc-type-of-generic-bound.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ LL | return a.bar();
|
= note: expected type parameter `B`
found associated type `<A as MyTrait>::T`
= note: the caller chooses a type for `B` which can be different from `<A as MyTrait>::T`
help: consider further restricting this bound
|
LL | pub fn foo<A: MyTrait<T = B>, B>(a: A) -> B {
Expand Down
1 change: 1 addition & 0 deletions tests/ui/type/type-parameter-names.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ LL | x
found type parameter `Foo`
= note: a type parameter was expected, but a different one was found; you might be missing a type parameter or trait bound
= note: for more information, visit https://doc.rust-lang.org/book/ch10-02-traits.html#traits-as-parameters
= note: the caller chooses a type for `Bar` which can be different from `Foo`

error: aborting due to 1 previous error

Expand Down
1 change: 1 addition & 0 deletions tests/ui/type/type-params-in-different-spaces-3.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ LL | u
found type parameter `X`
= note: a type parameter was expected, but a different one was found; you might be missing a type parameter or trait bound
= note: for more information, visit https://doc.rust-lang.org/book/ch10-02-traits.html#traits-as-parameters
= note: the caller chooses a type for `Self` which can be different from `X`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, this one is interesting. The note is technically speaking correct but I don't know if it helps the user. Remember that we add the note “the caller chooses …” because the user might not understand that fn f<T>() -> T doesn't mean “can return anything the callee / the author of the function wants to return”. What do you think? Do you think it's useful?

Copy link
Member Author

@jieyouxu jieyouxu Mar 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This note doesn't seem very helpful in this instance, no. This particular case probably needs a separate note suggesting that X might not satisfy X: Sized but Self: Sized or something to that effect. Not entirely sure what help to give here though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not block this PR on this (given the long history of this diagnostic change with several failed PRs), we should probably not emit the note if the param is kw::SelfUpper but that can be done in a follow up.

Sorry for the delay, I was sitting at my desk wondering why we're not suggesting impl-Trait on fn f<T>() -> (T,) { (0,) } etc. (so, fn f() -> (impl Sized,) { (0,) } which works perfectly). I should probably open an issue for that. If we end up making try_suggest_return_impl_trait smarter at some point we can then probably also move the note back into impl-Trait since it does some other checks, too, which we don't do for note_caller_chooses_ty_for_ty_param.

suggesting that X might not satisfy X: Sized but Self: Sized or something to that effect

Nah, that's not relevant here. The Sized bounds only exist to allow the method to return self.


error: aborting due to 1 previous error

Expand Down
1 change: 1 addition & 0 deletions tests/ui/typeck/issue-13853.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ LL | self.iter()
|
= note: expected type parameter `I`
found struct `std::slice::Iter<'_, N>`
= note: the caller chooses a type for `I` which can be different from `std::slice::Iter<'_, N>`

error[E0599]: no method named `iter` found for reference `&G` in the current scope
--> $DIR/issue-13853.rs:27:23
Expand Down
Loading