From ae899cc4e2bf91e553ddb369b83deb6425500ff4 Mon Sep 17 00:00:00 2001 From: yukang Date: Thu, 2 Apr 2026 17:57:33 +0800 Subject: [PATCH] Clarify private field autoderef notes --- .../src/error_reporting/traits/suggestions.rs | 125 +++++++++-- ...vate-field-deref-confusion-issue-149546.rs | 51 +---- ...-field-deref-confusion-issue-149546.stderr | 203 +++++++++++++++--- 3 files changed, 285 insertions(+), 94 deletions(-) diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs index e855d51aac1c..4912d5f4582e 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs @@ -22,8 +22,10 @@ expr_needs_parens, }; use rustc_infer::infer::{BoundRegionConversionTime, DefineOpaqueTypes, InferCtxt, InferOk}; +use rustc_infer::traits::ImplSource; use rustc_middle::middle::privacy::Level; use rustc_middle::traits::IsConstable; +use rustc_middle::ty::adjustment::{Adjust, DerefAdjustKind}; use rustc_middle::ty::error::TypeError; use rustc_middle::ty::print::{ PrintPolyTraitPredicateExt as _, PrintPolyTraitRefExt, PrintTraitPredicateExt as _, @@ -49,7 +51,7 @@ use crate::errors; use crate::infer::InferCtxtExt as _; use crate::traits::query::evaluate_obligation::InferCtxtExt as _; -use crate::traits::{ImplDerivedCause, NormalizeExt, ObligationCtxt}; +use crate::traits::{ImplDerivedCause, NormalizeExt, ObligationCtxt, SelectionContext}; #[derive(Debug)] pub enum CoroutineInteriorOrUpvar { @@ -248,46 +250,44 @@ pub fn note_field_shadowed_by_private_candidate_in_cause( cause: &ObligationCause<'tcx>, param_env: ty::ParamEnv<'tcx>, ) { - let mut hir_ids = Vec::new(); - let mut push_hir_id = |hir_id| { - if !hir_ids.contains(&hir_id) { - hir_ids.push(hir_id); - } - }; + let mut hir_ids = FxHashSet::default(); // Walk the parent chain so we can recover // the source expression from whichever layer carries them. let mut next_code = Some(cause.code()); while let Some(cause_code) = next_code { match cause_code { ObligationCauseCode::BinOp { lhs_hir_id, rhs_hir_id, .. } => { - push_hir_id(*lhs_hir_id); - push_hir_id(*rhs_hir_id); + hir_ids.insert(*lhs_hir_id); + hir_ids.insert(*rhs_hir_id); } ObligationCauseCode::FunctionArg { arg_hir_id, .. } | ObligationCauseCode::ReturnValue(arg_hir_id) | ObligationCauseCode::AwaitableExpr(arg_hir_id) | ObligationCauseCode::BlockTailExpression(arg_hir_id, _) | ObligationCauseCode::UnOp { hir_id: arg_hir_id } => { - push_hir_id(*arg_hir_id); + hir_ids.insert(*arg_hir_id); } ObligationCauseCode::OpaqueReturnType(Some((_, hir_id))) => { - push_hir_id(*hir_id); + hir_ids.insert(*hir_id); } _ => {} } next_code = cause_code.parent(); } - if cause.span != DUMMY_SP + if !cause.span.is_dummy() && let Some(body) = self.tcx.hir_maybe_body_owned_by(cause.body_id) { let mut expr_finder = FindExprBySpan::new(cause.span, self.tcx); expr_finder.visit_body(body); if let Some(expr) = expr_finder.result { - push_hir_id(expr.hir_id); + hir_ids.insert(expr.hir_id); } } + // we will sort immediately by source order before emitting any diagnostics + #[allow(rustc::potential_query_instability)] + let mut hir_ids: Vec<_> = hir_ids.into_iter().collect(); let source_map = self.tcx.sess.source_map(); hir_ids.sort_by_cached_key(|hir_id| { let span = self.tcx.hir_span(*hir_id); @@ -326,7 +326,7 @@ pub fn note_field_shadowed_by_private_candidate( } let fn_body_hir_id = self.tcx.local_def_id_to_hir_id(typeck_results.hir_owner.def_id); - let mut private_candidate = None; + let mut private_candidate: Option<(Ty<'tcx>, Ty<'tcx>, Span)> = None; for (deref_base_ty, _) in (self.autoderef_steps)(base_ty) { let ty::Adt(base_def, args) = deref_base_ty.kind() else { @@ -347,20 +347,107 @@ pub fn note_field_shadowed_by_private_candidate( else { continue; }; + let field_span = self + .tcx + .def_ident_span(field_def.did) + .unwrap_or_else(|| self.tcx.def_span(field_def.did)); if field_def.vis.is_accessible_from(def_scope, self.tcx) { let accessible_field_ty = field_def.ty(self.tcx, args); - if let Some((private_base_ty, private_field_ty)) = private_candidate + if let Some((private_base_ty, private_field_ty, private_field_span)) = + private_candidate && !self.can_eq(param_env, private_field_ty, accessible_field_ty) { - err.note(format!( - "there is a field `{field_ident}` on `{private_base_ty}` with type `{private_field_ty}`, but it is private" - )); + let private_struct_span = match private_base_ty.kind() { + ty::Adt(private_base_def, _) => self + .tcx + .def_ident_span(private_base_def.did()) + .unwrap_or_else(|| self.tcx.def_span(private_base_def.did())), + _ => DUMMY_SP, + }; + let accessible_struct_span = self + .tcx + .def_ident_span(base_def.did()) + .unwrap_or_else(|| self.tcx.def_span(base_def.did())); + let deref_impl_span = (typeck_results + .expr_adjustments(base_expr) + .iter() + .filter(|adj| { + matches!(adj.kind, Adjust::Deref(DerefAdjustKind::Overloaded(_))) + }) + .count() + == 1) + .then(|| { + self.probe(|_| { + let deref_trait_did = + self.tcx.require_lang_item(LangItem::Deref, DUMMY_SP); + let trait_ref = + ty::TraitRef::new(self.tcx, deref_trait_did, [private_base_ty]); + let obligation: Obligation<'tcx, ty::Predicate<'tcx>> = + Obligation::new( + self.tcx, + ObligationCause::dummy(), + param_env, + trait_ref, + ); + let Ok(Some(ImplSource::UserDefined(impl_data))) = + SelectionContext::new(self) + .select(&obligation.with(self.tcx, trait_ref)) + else { + return None; + }; + Some(self.tcx.def_span(impl_data.impl_def_id)) + }) + }) + .flatten(); + + let mut note_spans: MultiSpan = private_struct_span.into(); + if private_struct_span != DUMMY_SP { + note_spans.push_span_label(private_struct_span, "in this struct"); + } + if private_field_span != DUMMY_SP { + note_spans.push_span_label( + private_field_span, + "if this field wasn't private, it would be accessible", + ); + } + if accessible_struct_span != DUMMY_SP { + note_spans.push_span_label( + accessible_struct_span, + "this struct is accessible through auto-deref", + ); + } + if field_span != DUMMY_SP { + note_spans + .push_span_label(field_span, "this is the field that was accessed"); + } + if let Some(deref_impl_span) = deref_impl_span + && deref_impl_span != DUMMY_SP + { + note_spans.push_span_label( + deref_impl_span, + "the field was accessed through this `Deref`", + ); + } + + err.span_note( + note_spans, + format!( + "there is a field `{field_ident}` on `{private_base_ty}` with type `{private_field_ty}` but it is private; `{field_ident}` from `{deref_base_ty}` was accessed through auto-deref instead" + ), + ); } + + // we finally get to the accessible field, + // so we can return early without checking the rest of the autoderef candidates return; } - private_candidate.get_or_insert((deref_base_ty, field_def.ty(self.tcx, args))); + private_candidate.get_or_insert(( + deref_base_ty, + field_def.ty(self.tcx, args), + field_span, + )); } } diff --git a/tests/ui/privacy/private-field-deref-confusion-issue-149546.rs b/tests/ui/privacy/private-field-deref-confusion-issue-149546.rs index e9013990d74c..ec26c879d4eb 100644 --- a/tests/ui/privacy/private-field-deref-confusion-issue-149546.rs +++ b/tests/ui/privacy/private-field-deref-confusion-issue-149546.rs @@ -1,6 +1,7 @@ // Field lookup still resolves to the public field on the Deref target, but // follow-up diagnostics should explain that the original type has a same-named // private field with a different type. +//@ dont-require-annotations: ERROR mod structs { pub struct A { @@ -24,18 +25,14 @@ fn deref(&self) -> &Self::Target { use structs::A; fn takes_usize(_: usize) {} -//~^ NOTE function defined here trait Marker {} impl Marker for usize {} -//~^ HELP the trait `Marker` is implemented for `usize` struct Wrapper(i32); impl std::ops::Add for Wrapper { - //~^ NOTE required for `Wrapper` to implement `Add` - //~| NOTE unsatisfied trait bound introduced here type Output = (); fn add(self, _: T) {} @@ -43,95 +40,49 @@ fn add(self, _: T) {} fn by_value(a: A) { a.field + 5; - //~^ ERROR cannot add `{integer}` to `bool` - //~| NOTE bool - //~| NOTE {integer} - //~| NOTE there is a field `field` on `A` with type `usize`, but it is private } fn by_ref(a: &A) { a.field + 5; - //~^ ERROR cannot add `{integer}` to `bool` - //~| NOTE bool - //~| NOTE {integer} - //~| NOTE there is a field `field` on `A` with type `usize`, but it is private } fn rhs_by_value(a: A) { 5 + a.field; - //~^ ERROR cannot add `bool` to `{integer}` - //~| NOTE no implementation for `{integer} + bool` - //~| HELP the trait `Add` is not implemented for `{integer}` - //~| HELP the following other types implement trait `Add`: - //~| NOTE there is a field `field` on `A` with type `usize`, but it is private } fn rhs_by_ref(a: &A) { 5 + a.field; - //~^ ERROR cannot add `bool` to `{integer}` - //~| NOTE no implementation for `{integer} + bool` - //~| HELP the trait `Add` is not implemented for `{integer}` - //~| HELP the following other types implement trait `Add`: - //~| NOTE there is a field `field` on `A` with type `usize`, but it is private } fn rhs_assign_op_by_value(a: A) { let mut n = 5; n += a.field; - //~^ ERROR cannot add-assign `bool` to `{integer}` - //~| NOTE no implementation for `{integer} += bool` - //~| HELP the trait `AddAssign` is not implemented for `{integer}` - //~| HELP the following other types implement trait `AddAssign`: - //~| NOTE there is a field `field` on `A` with type `usize`, but it is private } fn rhs_assign_op_by_ref(a: &A) { let mut n = 5; n += a.field; - //~^ ERROR cannot add-assign `bool` to `{integer}` - //~| NOTE no implementation for `{integer} += bool` - //~| HELP the trait `AddAssign` is not implemented for `{integer}` - //~| HELP the following other types implement trait `AddAssign`: - //~| NOTE there is a field `field` on `A` with type `usize`, but it is private } fn rhs_nested_obligation(a: A) { Wrapper(5) + a.field; - //~^ ERROR the trait bound `bool: Marker` is not satisfied - //~| NOTE the trait `Marker` is not implemented for `bool` - //~| NOTE there is a field `field` on `A` with type `usize`, but it is private } fn method_call(a: A) { a.field.count_ones(); - //~^ ERROR no method named `count_ones` found for type `bool` in the current scope - //~| NOTE method not found in `bool` - //~| NOTE there is a field `field` on `A` with type `usize`, but it is private } fn type_mismatch(a: A) { let value: usize = a.field; - //~^ ERROR mismatched types - //~| NOTE expected `usize`, found `bool` - //~| NOTE expected due to this - //~| NOTE there is a field `field` on `A` with type `usize`, but it is private eprintln!("value: {value}"); } fn function_arg(a: A) { takes_usize(a.field); - //~^ ERROR mismatched types - //~| NOTE expected `usize`, found `bool` - //~| NOTE arguments to this function are incorrect - //~| NOTE there is a field `field` on `A` with type `usize`, but it is private } fn return_value(a: &A) -> usize { - //~^ NOTE expected `usize` because of return type a.field - //~^ ERROR mismatched types - //~| NOTE expected `usize`, found `bool` - //~| NOTE there is a field `field` on `A` with type `usize`, but it is private } fn main() {} diff --git a/tests/ui/privacy/private-field-deref-confusion-issue-149546.stderr b/tests/ui/privacy/private-field-deref-confusion-issue-149546.stderr index d10d9a771b07..9f2bb523913c 100644 --- a/tests/ui/privacy/private-field-deref-confusion-issue-149546.stderr +++ b/tests/ui/privacy/private-field-deref-confusion-issue-149546.stderr @@ -1,31 +1,73 @@ error[E0369]: cannot add `{integer}` to `bool` - --> $DIR/private-field-deref-confusion-issue-149546.rs:45:13 + --> $DIR/private-field-deref-confusion-issue-149546.rs:42:13 | LL | a.field + 5; | ------- ^ - {integer} | | | bool | - = note: there is a field `field` on `A` with type `usize`, but it is private +note: there is a field `field` on `A` with type `usize` but it is private; `field` from `B` was accessed through auto-deref instead + --> $DIR/private-field-deref-confusion-issue-149546.rs:7:16 + | +LL | pub struct A { + | ^ in this struct +LL | field: usize, + | ----- if this field wasn't private, it would be accessible +... +LL | pub struct B { + | - this struct is accessible through auto-deref +LL | pub field: bool, + | ----- this is the field that was accessed +... +LL | impl std::ops::Deref for A { + | -------------------------- the field was accessed through this `Deref` error[E0369]: cannot add `{integer}` to `bool` - --> $DIR/private-field-deref-confusion-issue-149546.rs:53:13 + --> $DIR/private-field-deref-confusion-issue-149546.rs:46:13 | LL | a.field + 5; | ------- ^ - {integer} | | | bool | - = note: there is a field `field` on `A` with type `usize`, but it is private +note: there is a field `field` on `A` with type `usize` but it is private; `field` from `B` was accessed through auto-deref instead + --> $DIR/private-field-deref-confusion-issue-149546.rs:7:16 + | +LL | pub struct A { + | ^ in this struct +LL | field: usize, + | ----- if this field wasn't private, it would be accessible +... +LL | pub struct B { + | - this struct is accessible through auto-deref +LL | pub field: bool, + | ----- this is the field that was accessed +... +LL | impl std::ops::Deref for A { + | -------------------------- the field was accessed through this `Deref` error[E0277]: cannot add `bool` to `{integer}` - --> $DIR/private-field-deref-confusion-issue-149546.rs:61:7 + --> $DIR/private-field-deref-confusion-issue-149546.rs:50:7 | LL | 5 + a.field; | ^ no implementation for `{integer} + bool` | = help: the trait `Add` is not implemented for `{integer}` - = note: there is a field `field` on `A` with type `usize`, but it is private +note: there is a field `field` on `A` with type `usize` but it is private; `field` from `B` was accessed through auto-deref instead + --> $DIR/private-field-deref-confusion-issue-149546.rs:7:16 + | +LL | pub struct A { + | ^ in this struct +LL | field: usize, + | ----- if this field wasn't private, it would be accessible +... +LL | pub struct B { + | - this struct is accessible through auto-deref +LL | pub field: bool, + | ----- this is the field that was accessed +... +LL | impl std::ops::Deref for A { + | -------------------------- the field was accessed through this `Deref` = help: the following other types implement trait `Add`: `&f128` implements `Add` `&f128` implements `Add` @@ -38,13 +80,27 @@ LL | 5 + a.field; and 56 others error[E0277]: cannot add `bool` to `{integer}` - --> $DIR/private-field-deref-confusion-issue-149546.rs:70:7 + --> $DIR/private-field-deref-confusion-issue-149546.rs:54:7 | LL | 5 + a.field; | ^ no implementation for `{integer} + bool` | = help: the trait `Add` is not implemented for `{integer}` - = note: there is a field `field` on `A` with type `usize`, but it is private +note: there is a field `field` on `A` with type `usize` but it is private; `field` from `B` was accessed through auto-deref instead + --> $DIR/private-field-deref-confusion-issue-149546.rs:7:16 + | +LL | pub struct A { + | ^ in this struct +LL | field: usize, + | ----- if this field wasn't private, it would be accessible +... +LL | pub struct B { + | - this struct is accessible through auto-deref +LL | pub field: bool, + | ----- this is the field that was accessed +... +LL | impl std::ops::Deref for A { + | -------------------------- the field was accessed through this `Deref` = help: the following other types implement trait `Add`: `&f128` implements `Add` `&f128` implements `Add` @@ -57,13 +113,27 @@ LL | 5 + a.field; and 56 others error[E0277]: cannot add-assign `bool` to `{integer}` - --> $DIR/private-field-deref-confusion-issue-149546.rs:80:7 + --> $DIR/private-field-deref-confusion-issue-149546.rs:59:7 | LL | n += a.field; | ^^ no implementation for `{integer} += bool` | = help: the trait `AddAssign` is not implemented for `{integer}` - = note: there is a field `field` on `A` with type `usize`, but it is private +note: there is a field `field` on `A` with type `usize` but it is private; `field` from `B` was accessed through auto-deref instead + --> $DIR/private-field-deref-confusion-issue-149546.rs:7:16 + | +LL | pub struct A { + | ^ in this struct +LL | field: usize, + | ----- if this field wasn't private, it would be accessible +... +LL | pub struct B { + | - this struct is accessible through auto-deref +LL | pub field: bool, + | ----- this is the field that was accessed +... +LL | impl std::ops::Deref for A { + | -------------------------- the field was accessed through this `Deref` = help: the following other types implement trait `AddAssign`: `f128` implements `AddAssign<&f128>` `f128` implements `AddAssign` @@ -76,13 +146,27 @@ LL | n += a.field; and 24 others error[E0277]: cannot add-assign `bool` to `{integer}` - --> $DIR/private-field-deref-confusion-issue-149546.rs:90:7 + --> $DIR/private-field-deref-confusion-issue-149546.rs:64:7 | LL | n += a.field; | ^^ no implementation for `{integer} += bool` | = help: the trait `AddAssign` is not implemented for `{integer}` - = note: there is a field `field` on `A` with type `usize`, but it is private +note: there is a field `field` on `A` with type `usize` but it is private; `field` from `B` was accessed through auto-deref instead + --> $DIR/private-field-deref-confusion-issue-149546.rs:7:16 + | +LL | pub struct A { + | ^ in this struct +LL | field: usize, + | ----- if this field wasn't private, it would be accessible +... +LL | pub struct B { + | - this struct is accessible through auto-deref +LL | pub field: bool, + | ----- this is the field that was accessed +... +LL | impl std::ops::Deref for A { + | -------------------------- the field was accessed through this `Deref` = help: the following other types implement trait `AddAssign`: `f128` implements `AddAssign<&f128>` `f128` implements `AddAssign` @@ -95,19 +179,33 @@ LL | n += a.field; and 24 others error[E0277]: the trait bound `bool: Marker` is not satisfied - --> $DIR/private-field-deref-confusion-issue-149546.rs:99:16 + --> $DIR/private-field-deref-confusion-issue-149546.rs:68:16 | LL | Wrapper(5) + a.field; | ^ the trait `Marker` is not implemented for `bool` | - = note: there is a field `field` on `A` with type `usize`, but it is private +note: there is a field `field` on `A` with type `usize` but it is private; `field` from `B` was accessed through auto-deref instead + --> $DIR/private-field-deref-confusion-issue-149546.rs:7:16 + | +LL | pub struct A { + | ^ in this struct +LL | field: usize, + | ----- if this field wasn't private, it would be accessible +... +LL | pub struct B { + | - this struct is accessible through auto-deref +LL | pub field: bool, + | ----- this is the field that was accessed +... +LL | impl std::ops::Deref for A { + | -------------------------- the field was accessed through this `Deref` help: the trait `Marker` is implemented for `usize` --> $DIR/private-field-deref-confusion-issue-149546.rs:31:1 | LL | impl Marker for usize {} | ^^^^^^^^^^^^^^^^^^^^^ note: required for `Wrapper` to implement `Add` - --> $DIR/private-field-deref-confusion-issue-149546.rs:36:17 + --> $DIR/private-field-deref-confusion-issue-149546.rs:35:17 | LL | impl std::ops::Add for Wrapper { | ------ ^^^^^^^^^^^^^^^^ ^^^^^^^ @@ -115,48 +213,103 @@ LL | impl std::ops::Add for Wrapper { | unsatisfied trait bound introduced here error[E0599]: no method named `count_ones` found for type `bool` in the current scope - --> $DIR/private-field-deref-confusion-issue-149546.rs:106:13 + --> $DIR/private-field-deref-confusion-issue-149546.rs:72:13 | LL | a.field.count_ones(); | ^^^^^^^^^^ method not found in `bool` | - = note: there is a field `field` on `A` with type `usize`, but it is private +note: there is a field `field` on `A` with type `usize` but it is private; `field` from `B` was accessed through auto-deref instead + --> $DIR/private-field-deref-confusion-issue-149546.rs:7:16 + | +LL | pub struct A { + | ^ in this struct +LL | field: usize, + | ----- if this field wasn't private, it would be accessible +... +LL | pub struct B { + | - this struct is accessible through auto-deref +LL | pub field: bool, + | ----- this is the field that was accessed +... +LL | impl std::ops::Deref for A { + | -------------------------- the field was accessed through this `Deref` error[E0308]: mismatched types - --> $DIR/private-field-deref-confusion-issue-149546.rs:113:24 + --> $DIR/private-field-deref-confusion-issue-149546.rs:76:24 | LL | let value: usize = a.field; | ----- ^^^^^^^ expected `usize`, found `bool` | | | expected due to this | - = note: there is a field `field` on `A` with type `usize`, but it is private +note: there is a field `field` on `A` with type `usize` but it is private; `field` from `B` was accessed through auto-deref instead + --> $DIR/private-field-deref-confusion-issue-149546.rs:7:16 + | +LL | pub struct A { + | ^ in this struct +LL | field: usize, + | ----- if this field wasn't private, it would be accessible +... +LL | pub struct B { + | - this struct is accessible through auto-deref +LL | pub field: bool, + | ----- this is the field that was accessed +... +LL | impl std::ops::Deref for A { + | -------------------------- the field was accessed through this `Deref` error[E0308]: mismatched types - --> $DIR/private-field-deref-confusion-issue-149546.rs:122:17 + --> $DIR/private-field-deref-confusion-issue-149546.rs:81:17 | LL | takes_usize(a.field); | ----------- ^^^^^^^ expected `usize`, found `bool` | | | arguments to this function are incorrect | - = note: there is a field `field` on `A` with type `usize`, but it is private +note: there is a field `field` on `A` with type `usize` but it is private; `field` from `B` was accessed through auto-deref instead + --> $DIR/private-field-deref-confusion-issue-149546.rs:7:16 + | +LL | pub struct A { + | ^ in this struct +LL | field: usize, + | ----- if this field wasn't private, it would be accessible +... +LL | pub struct B { + | - this struct is accessible through auto-deref +LL | pub field: bool, + | ----- this is the field that was accessed +... +LL | impl std::ops::Deref for A { + | -------------------------- the field was accessed through this `Deref` note: function defined here - --> $DIR/private-field-deref-confusion-issue-149546.rs:26:4 + --> $DIR/private-field-deref-confusion-issue-149546.rs:27:4 | LL | fn takes_usize(_: usize) {} | ^^^^^^^^^^^ -------- error[E0308]: mismatched types - --> $DIR/private-field-deref-confusion-issue-149546.rs:131:5 + --> $DIR/private-field-deref-confusion-issue-149546.rs:85:5 | LL | fn return_value(a: &A) -> usize { | ----- expected `usize` because of return type -LL | LL | a.field | ^^^^^^^ expected `usize`, found `bool` | - = note: there is a field `field` on `A` with type `usize`, but it is private +note: there is a field `field` on `A` with type `usize` but it is private; `field` from `B` was accessed through auto-deref instead + --> $DIR/private-field-deref-confusion-issue-149546.rs:7:16 + | +LL | pub struct A { + | ^ in this struct +LL | field: usize, + | ----- if this field wasn't private, it would be accessible +... +LL | pub struct B { + | - this struct is accessible through auto-deref +LL | pub field: bool, + | ----- this is the field that was accessed +... +LL | impl std::ops::Deref for A { + | -------------------------- the field was accessed through this `Deref` error: aborting due to 11 previous errors