Skip to content

Account for bare tuples and Pin methods in field searching logic #144649

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Account for bare tuples and Pin methods in field searching logic #144649

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3b2c8c0
Account for bare tuples in field searching logic
estebank Jul 29, 2025
53eb650
Do not suggest pinning missing `.get_ref()`
estebank Jul 29, 2025
775e287
Do not provide field typo suggestions for tuples and tuple structs
estebank Jul 30, 2025
f47df01
review comment
estebank Jul 30, 2025
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
55 changes: 35 additions & 20 deletions compiler/rustc_hir_typeck/src/expr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2745,6 +2745,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let available_field_names = self.available_field_names(variant, expr, skip_fields);
if let Some(field_name) =
find_best_match_for_name(&available_field_names, field.ident.name, None)
&& !(field.ident.name.as_str().parse::<usize>().is_ok()
&& field_name.as_str().parse::<usize>().is_ok())
{
err.span_label(field.ident.span, "unknown field");
err.span_suggestion_verbose(
Expand Down Expand Up @@ -3321,18 +3323,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
} else {
(base_ty, "")
};
for (found_fields, args) in
for found_fields in
self.get_field_candidates_considering_privacy_for_diag(span, ty, mod_id, expr.hir_id)
{
let field_names = found_fields.iter().map(|field| field.name).collect::<Vec<_>>();
let field_names = found_fields.iter().map(|field| field.0.name).collect::<Vec<_>>();
let mut candidate_fields: Vec<_> = found_fields
.into_iter()
.filter_map(|candidate_field| {
self.check_for_nested_field_satisfying_condition_for_diag(
span,
&|candidate_field, _| candidate_field.ident(self.tcx()) == field,
&|candidate_field, _| candidate_field == field,
candidate_field,
args,
vec![],
mod_id,
expr.hir_id,
Expand Down Expand Up @@ -3361,6 +3362,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
);
} else if let Some(field_name) =
find_best_match_for_name(&field_names, field.name, None)
&& !(field.name.as_str().parse::<usize>().is_ok()
&& field_name.as_str().parse::<usize>().is_ok())
{
err.span_suggestion_verbose(
field.span,
Expand Down Expand Up @@ -3396,7 +3399,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
base_ty: Ty<'tcx>,
mod_id: DefId,
hir_id: HirId,
) -> Vec<(Vec<&'tcx ty::FieldDef>, GenericArgsRef<'tcx>)> {
) -> Vec<Vec<(Ident, Ty<'tcx>)>> {
debug!("get_field_candidates(span: {:?}, base_t: {:?}", span, base_ty);

let mut autoderef = self.autoderef(span, base_ty).silence_errors();
Expand All @@ -3422,7 +3425,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
if fields.iter().all(|field| !field.vis.is_accessible_from(mod_id, tcx)) {
return None;
}
return Some((
return Some(
fields
.iter()
.filter(move |field| {
Expand All @@ -3431,9 +3434,25 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
})
// For compile-time reasons put a limit on number of fields we search
.take(100)
.map(|field_def| {
(
field_def.ident(self.tcx).normalize_to_macros_2_0(),
field_def.ty(self.tcx, args),
)
})
.collect::<Vec<_>>(),
);
}
ty::Tuple(types) => {
return Some(
types
.iter()
.enumerate()
// For compile-time reasons put a limit on number of fields we search
.take(100)
Comment on lines +3451 to +3452
Copy link
Contributor

Choose a reason for hiding this comment

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

can you think of any (even-non practical) test where this matters? If there are enough fields for a linear search to cause performance issues, I'd assume that we've already got performance issues due to other reasons, e.g. trying to print the full type name to a file

Copy link
Contributor Author

@estebank estebank Jul 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

  1. the check was present already
  2. I think it is good form to make diagnostics not incur additional time during compiles, within reason
  3. Ithis logic could trigger multiple times, up to 3 levels deep: if you have 3 levels of nesting where all of the types have 100 fields, this is already going to be slow (1M ops); making it unbounded could end up with the compiler hanging in a case where it otherwise wouldn't

.map(|(i, ty)| (Ident::from_str(&i.to_string()), ty))
.collect::<Vec<_>>(),
*args,
));
);
}
_ => None,
}
Expand All @@ -3446,9 +3465,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
pub(crate) fn check_for_nested_field_satisfying_condition_for_diag(
&self,
span: Span,
matches: &impl Fn(&ty::FieldDef, Ty<'tcx>) -> bool,
candidate_field: &ty::FieldDef,
subst: GenericArgsRef<'tcx>,
matches: &impl Fn(Ident, Ty<'tcx>) -> bool,
candidate_field: (Ident, Ty<'tcx>),
mut field_path: Vec<Ident>,
mod_id: DefId,
hir_id: HirId,
Expand All @@ -3463,24 +3481,21 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// up to a depth of three
None
} else {
field_path.push(candidate_field.ident(self.tcx).normalize_to_macros_2_0());
let field_ty = candidate_field.ty(self.tcx, subst);
if matches(candidate_field, field_ty) {
field_path.push(candidate_field.0);
let field_ty = candidate_field.1;
if matches(candidate_field.0, field_ty) {
return Some(field_path);
} else {
for (nested_fields, subst) in self
.get_field_candidates_considering_privacy_for_diag(
span, field_ty, mod_id, hir_id,
)
{
for nested_fields in self.get_field_candidates_considering_privacy_for_diag(
span, field_ty, mod_id, hir_id,
) {
// recursively search fields of `candidate_field` if it's a ty::Adt
for field in nested_fields {
if let Some(field_path) = self
.check_for_nested_field_satisfying_condition_for_diag(
span,
matches,
field,
subst,
field_path.clone(),
mod_id,
hir_id,
Expand Down
14 changes: 9 additions & 5 deletions compiler/rustc_hir_typeck/src/method/suggest.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2792,7 +2792,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
) {
if let SelfSource::MethodCall(expr) = source {
let mod_id = self.tcx.parent_module(expr.hir_id).to_def_id();
for (fields, args) in self.get_field_candidates_considering_privacy_for_diag(
for fields in self.get_field_candidates_considering_privacy_for_diag(
span,
actual,
mod_id,
Expand Down Expand Up @@ -2831,7 +2831,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
})
},
candidate_field,
args,
vec![],
mod_id,
expr.hir_id,
Expand Down Expand Up @@ -3671,7 +3670,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
)
{
debug!("try_alt_rcvr: pick candidate {:?}", pick);
let did = Some(pick.item.container_id(self.tcx));
let did = pick.item.trait_container(self.tcx);
// We don't want to suggest a container type when the missing
// method is `.clone()` or `.deref()` otherwise we'd suggest
// `Arc::new(foo).clone()`, which is far from what the user wants.
Expand Down Expand Up @@ -3720,8 +3719,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&& !alt_rcvr_sugg
// `T: !Unpin`
&& !unpin
// The method isn't `as_ref`, as it would provide a wrong suggestion for `Pin`.
&& sym::as_ref != item_name.name
// Either `Pin::as_ref` or `Pin::as_mut`.
&& let Some(pin_call) = pin_call
// Search for `item_name` as a method accessible on `Pin<T>`.
Expand All @@ -3735,6 +3732,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// We skip some common traits that we don't want to consider because autoderefs
// would take care of them.
&& !skippable.contains(&Some(pick.item.container_id(self.tcx)))
// Do not suggest pinning when the method is directly on `Pin`.
&& pick.item.impl_container(self.tcx).map_or(true, |did| {
match self.tcx.type_of(did).instantiate_identity().kind() {
ty::Adt(def, _) => Some(def.did()) != self.tcx.lang_items().pin_type(),
_ => true,
}
})
// We don't want to go through derefs.
&& pick.autoderefs == 0
// Check that the method of the same name that was found on the new `Pin<T>`
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/coherence/coherence-tuple-conflict.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ error[E0609]: no field `dummy` on type `&(A, B)`
|
LL | fn get(&self) -> usize { self.dummy }
| ^^^^^ unknown field
|
= note: available fields are: `0`, `1`

error: aborting due to 2 previous errors

Expand Down
2 changes: 2 additions & 0 deletions tests/ui/consts/issue-19244-1.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ error[E0609]: no field `1` on type `(usize,)`
|
LL | let a: [isize; TUP.1];
| ^ unknown field
|
= note: available field is: `0`

error: aborting due to 1 previous error

Expand Down
1 change: 1 addition & 0 deletions tests/ui/diagnostic-width/long-E0609.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ error[E0609]: no field `field` on type `(..., ..., ..., ...)`
LL | x.field;
| ^^^^^ unknown field
|
= note: available fields are: `0`, `1`, `2`, `3`
= note: the full name for the type has been written to '$TEST_BUILD_DIR/long-E0609.long-type-$LONG_TYPE_HASH.txt'
= note: consider using `--verbose` to print the full type name to the console

Expand Down
6 changes: 1 addition & 5 deletions tests/ui/error-codes/ex-E0612.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,7 @@ error[E0609]: no field `1` on type `Foo`
LL | y.1;
| ^ unknown field
|
help: a field with a similar name exists
|
LL - y.1;
LL + y.0;
|
= note: available field is: `0`

error: aborting due to 1 previous error

Expand Down
6 changes: 1 addition & 5 deletions tests/ui/issues/issue-47073-zero-padded-tuple-struct-indices.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,7 @@ error[E0609]: no field `00` on type `Verdict`
LL | let _condemned = justice.00;
| ^^ unknown field
|
help: a field with a similar name exists
|
LL - let _condemned = justice.00;
LL + let _condemned = justice.0;
|
= note: available fields are: `0`, `1`

error[E0609]: no field `001` on type `Verdict`
--> $DIR/issue-47073-zero-padded-tuple-struct-indices.rs:10:31
Expand Down
36 changes: 36 additions & 0 deletions tests/ui/offset-of/offset-of-tuple-field.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,60 +15,96 @@ error[E0609]: no field `_0` on type `(u8, u8)`
|
LL | offset_of!((u8, u8), _0);
| ^^
|
help: a field with a similar name exists
|
LL - offset_of!((u8, u8), _0);
LL + offset_of!((u8, u8), 0);
|

error[E0609]: no field `01` on type `(u8, u8)`
--> $DIR/offset-of-tuple-field.rs:7:26
|
LL | offset_of!((u8, u8), 01);
| ^^
|
= note: available fields are: `0`, `1`

error[E0609]: no field `1e2` on type `(u8, u8)`
--> $DIR/offset-of-tuple-field.rs:8:26
|
LL | offset_of!((u8, u8), 1e2);
| ^^^
|
= note: available fields are: `0`, `1`

error[E0609]: no field `1_` on type `(u8, u8)`
--> $DIR/offset-of-tuple-field.rs:9:26
|
LL | offset_of!((u8, u8), 1_u8);
| ^^^^
|
help: a field with a similar name exists
|
LL - offset_of!((u8, u8), 1_u8);
LL + offset_of!((u8, u8), 1);
|

error[E0609]: no field `1e2` on type `(u8, u8)`
--> $DIR/offset-of-tuple-field.rs:12:35
|
LL | builtin # offset_of((u8, u8), 1e2);
| ^^^
|
= note: available fields are: `0`, `1`

error[E0609]: no field `_0` on type `(u8, u8)`
--> $DIR/offset-of-tuple-field.rs:13:35
|
LL | builtin # offset_of((u8, u8), _0);
| ^^
|
help: a field with a similar name exists
|
LL - builtin # offset_of((u8, u8), _0);
LL + builtin # offset_of((u8, u8), 0);
|

error[E0609]: no field `01` on type `(u8, u8)`
--> $DIR/offset-of-tuple-field.rs:14:35
|
LL | builtin # offset_of((u8, u8), 01);
| ^^
|
= note: available fields are: `0`, `1`

error[E0609]: no field `1_` on type `(u8, u8)`
--> $DIR/offset-of-tuple-field.rs:15:35
|
LL | builtin # offset_of((u8, u8), 1_u8);
| ^^^^
|
help: a field with a similar name exists
|
LL - builtin # offset_of((u8, u8), 1_u8);
LL + builtin # offset_of((u8, u8), 1);
|

error[E0609]: no field `2` on type `(u8, u16)`
--> $DIR/offset-of-tuple-field.rs:18:47
|
LL | offset_of!(((u8, u16), (u32, u16, u8)), 0.2);
| ^
|
= note: available fields are: `0`, `1`

error[E0609]: no field `1e2` on type `(u8, u16)`
--> $DIR/offset-of-tuple-field.rs:19:47
|
LL | offset_of!(((u8, u16), (u32, u16, u8)), 0.1e2);
| ^^^
|
= note: available fields are: `0`, `1`

error[E0609]: no field `0` on type `u8`
--> $DIR/offset-of-tuple-field.rs:21:49
Expand Down
6 changes: 6 additions & 0 deletions tests/ui/parser/float-field.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,8 @@ error[E0609]: no field `1e1` on type `(u8, u8)`
|
LL | { s.1.1e1; }
| ^^^ unknown field
|
= note: available fields are: `0`, `1`

error[E0609]: no field `0x1e1` on type `S`
--> $DIR/float-field.rs:34:9
Expand Down Expand Up @@ -343,12 +345,16 @@ error[E0609]: no field `f32` on type `(u8, u8)`
|
LL | { s.1.f32; }
| ^^^ unknown field
|
= note: available fields are: `0`, `1`

error[E0609]: no field `1e1` on type `(u8, u8)`
--> $DIR/float-field.rs:71:9
|
LL | { s.1.1e1f32; }
| ^^^^^^^^ unknown field
|
= note: available fields are: `0`, `1`

error: aborting due to 57 previous errors

Expand Down
4 changes: 4 additions & 0 deletions tests/ui/traits/well-formed-recursion-limit.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,16 @@ error[E0609]: no field `ab` on type `(Box<(dyn Fn(Option<A>) -> Option<B> + 'sta
|
LL | let (ab, ba) = (i.ab, i.ba);
| ^^ unknown field
|
= note: available fields are: `0`, `1`

error[E0609]: no field `ba` on type `(Box<(dyn Fn(Option<A>) -> Option<B> + 'static)>, Box<(dyn Fn(Option<B>) -> Option<A> + 'static)>)`
--> $DIR/well-formed-recursion-limit.rs:12:29
|
LL | let (ab, ba) = (i.ab, i.ba);
| ^^ unknown field
|
= note: available fields are: `0`, `1`

error[E0275]: overflow assigning `_` to `Option<_>`
--> $DIR/well-formed-recursion-limit.rs:15:33
Expand Down
6 changes: 6 additions & 0 deletions tests/ui/tuple/index-invalid.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,24 @@ error[E0609]: no field `1` on type `(((),),)`
|
LL | let _ = (((),),).1.0;
| ^ unknown field
|
= note: available field is: `0`

error[E0609]: no field `1` on type `((),)`
--> $DIR/index-invalid.rs:4:24
|
LL | let _ = (((),),).0.1;
| ^ unknown field
|
= note: available field is: `0`

error[E0609]: no field `000` on type `(((),),)`
--> $DIR/index-invalid.rs:6:22
|
LL | let _ = (((),),).000.000;
| ^^^ unknown field
|
= note: available field is: `0`

error: aborting due to 3 previous errors

Expand Down
Loading
Loading