From 272fb2395c0867e2ab8aa8cdb141b616d1e52c62 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 13 Jan 2022 14:03:56 -0800 Subject: [PATCH] Don't use source-map when detecting struct field shorthand --- compiler/rustc_typeck/src/check/demand.rs | 217 ++++++++---------- .../src/check/fn_ctxt/suggestions.rs | 4 +- .../did_you_mean/compatible-variants.stderr | 2 +- src/test/ui/inference/deref-suggestion.stderr | 2 +- 4 files changed, 104 insertions(+), 121 deletions(-) diff --git a/compiler/rustc_typeck/src/check/demand.rs b/compiler/rustc_typeck/src/check/demand.rs index 4898109fa38..241dcbc64a6 100644 --- a/compiler/rustc_typeck/src/check/demand.rs +++ b/compiler/rustc_typeck/src/check/demand.rs @@ -13,7 +13,7 @@ use rustc_middle::ty::adjustment::AllowTwoPhase; use rustc_middle::ty::error::{ExpectedFound, TypeError}; use rustc_middle::ty::print::with_no_trimmed_paths; use rustc_middle::ty::{self, AssocItem, Ty, TypeAndMut}; -use rustc_span::symbol::sym; +use rustc_span::symbol::{sym, Symbol}; use rustc_span::{BytePos, Span}; use super::method::probe; @@ -24,7 +24,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { pub fn emit_coerce_suggestions( &self, err: &mut DiagnosticBuilder<'_>, - expr: &hir::Expr<'_>, + expr: &hir::Expr<'tcx>, expr_ty: Ty<'tcx>, expected: Ty<'tcx>, expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>, @@ -109,7 +109,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { pub fn demand_coerce( &self, - expr: &hir::Expr<'_>, + expr: &hir::Expr<'tcx>, checked_ty: Ty<'tcx>, expected: Ty<'tcx>, expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>, @@ -129,7 +129,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { /// will be permitted if the diverges flag is currently "always". pub fn demand_coerce_diag( &self, - expr: &hir::Expr<'_>, + expr: &hir::Expr<'tcx>, checked_ty: Ty<'tcx>, expected: Ty<'tcx>, expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>, @@ -338,67 +338,39 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { }) .collect(); - if self.is_hir_id_from_struct_pattern_shorthand_field(expr.hir_id, expr.span) { - if let Ok(code) = self.tcx.sess.source_map().span_to_snippet(expr.span) { - match &compatible_variants[..] { - [] => { /* No variants to format */ } - [variant] => { - // Just a single matching variant. - err.span_suggestion_verbose( - expr.span, - &format!("try wrapping the expression in `{}`", variant), - format!("{}: {}({})", code, variant, code), - Applicability::MaybeIncorrect, - ); - } - _ => { - // More than one matching variant. - err.span_suggestions( - expr.span, - &format!( - "try wrapping the expression in a variant of `{}`", - self.tcx.def_path_str(expected_adt.did) - ), - compatible_variants - .into_iter() - .map(|variant| format!("{}: {}({})", code, variant, code)), - Applicability::MaybeIncorrect, - ); - } - } - } else { - /* Can't format this without a snippet */ + let prefix = match self.maybe_get_struct_pattern_shorthand_field(expr) { + Some(ident) => format!("{}: ", ident), + None => format!(""), + }; + + match &compatible_variants[..] { + [] => { /* No variants to format */ } + [variant] => { + // Just a single matching variant. + err.multipart_suggestion_verbose( + &format!("try wrapping the expression in `{}`", variant), + vec![ + (expr.span.shrink_to_lo(), format!("{}{}(", prefix, variant)), + (expr.span.shrink_to_hi(), ")".to_string()), + ], + Applicability::MaybeIncorrect, + ); } - } else { - match &compatible_variants[..] { - [] => { /* No variants to format */ } - [variant] => { - // Just a single matching variant. - err.multipart_suggestion_verbose( - &format!("try wrapping the expression in `{}`", variant), + _ => { + // More than one matching variant. + err.multipart_suggestions( + &format!( + "try wrapping the expression in a variant of `{}`", + self.tcx.def_path_str(expected_adt.did) + ), + compatible_variants.into_iter().map(|variant| { vec![ - (expr.span.shrink_to_lo(), format!("{}(", variant)), + (expr.span.shrink_to_lo(), format!("{}{}(", prefix, variant)), (expr.span.shrink_to_hi(), ")".to_string()), - ], - Applicability::MaybeIncorrect, - ); - } - _ => { - // More than one matching variant. - err.multipart_suggestions( - &format!( - "try wrapping the expression in a variant of `{}`", - self.tcx.def_path_str(expected_adt.did) - ), - compatible_variants.into_iter().map(|variant| { - vec![ - (expr.span.shrink_to_lo(), format!("{}(", variant)), - (expr.span.shrink_to_hi(), ")".to_string()), - ] - }), - Applicability::MaybeIncorrect, - ); - } + ] + }), + Applicability::MaybeIncorrect, + ); } } } @@ -520,33 +492,45 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } - crate fn is_hir_id_from_struct_pattern_shorthand_field( + crate fn maybe_get_struct_pattern_shorthand_field( &self, - hir_id: hir::HirId, - sp: Span, - ) -> bool { - let sm = self.sess().source_map(); - let parent_id = self.tcx.hir().get_parent_node(hir_id); - if let Some(parent) = self.tcx.hir().find(parent_id) { - // Account for fields - if let Node::Expr(hir::Expr { kind: hir::ExprKind::Struct(_, fields, ..), .. }) = parent - { - if let Ok(src) = sm.span_to_snippet(sp) { - for field in *fields { - if field.ident.as_str() == src && field.is_shorthand { - return true; - } + expr: &hir::Expr<'_>, + ) -> Option { + let hir = self.tcx.hir(); + let local = match expr { + hir::Expr { + kind: + hir::ExprKind::Path(hir::QPath::Resolved( + None, + hir::Path { + res: hir::def::Res::Local(_), + segments: [hir::PathSegment { ident, .. }], + .. + }, + )), + .. + } => Some(ident), + _ => None, + }?; + + match hir.find(hir.get_parent_node(expr.hir_id))? { + Node::Expr(hir::Expr { kind: hir::ExprKind::Struct(_, fields, ..), .. }) => { + for field in *fields { + if field.ident.name == local.name && field.is_shorthand { + return Some(local.name); } } } + _ => {} } - false + + None } /// If the given `HirId` corresponds to a block with a trailing expression, return that expression - crate fn maybe_get_block_expr(&self, hir_id: hir::HirId) -> Option<&'tcx hir::Expr<'tcx>> { - match self.tcx.hir().find(hir_id)? { - Node::Expr(hir::Expr { kind: hir::ExprKind::Block(block, ..), .. }) => block.expr, + crate fn maybe_get_block_expr(&self, expr: &hir::Expr<'tcx>) -> Option<&'tcx hir::Expr<'tcx>> { + match expr { + hir::Expr { kind: hir::ExprKind::Block(block, ..), .. } => block.expr, _ => None, } } @@ -584,7 +568,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { /// `&mut`!". pub fn check_ref( &self, - expr: &hir::Expr<'_>, + expr: &hir::Expr<'tcx>, checked_ty: Ty<'tcx>, expected: Ty<'tcx>, ) -> Option<(Span, &'static str, String, Applicability, bool /* verbose */)> { @@ -602,9 +586,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { s.strip_prefix(old).map(|stripped| new.to_string() + stripped) }; - let is_struct_pat_shorthand_field = - self.is_hir_id_from_struct_pattern_shorthand_field(expr.hir_id, sp); - // `ExprKind::DropTemps` is semantically irrelevant for these suggestions. let expr = expr.peel_drop_temps(); @@ -698,11 +679,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { false, )); } - let field_name = if is_struct_pat_shorthand_field { - format!("{}: ", sugg_expr) - } else { - String::new() + + let prefix = match self.maybe_get_struct_pattern_shorthand_field(expr) { + Some(ident) => format!("{}: ", ident), + None => format!(""), }; + if let Some(hir::Node::Expr(hir::Expr { kind: hir::ExprKind::Assign(left_expr, ..), .. @@ -732,14 +714,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { hir::Mutability::Mut => ( sp, "consider mutably borrowing here", - format!("{}&mut {}", field_name, sugg_expr), + format!("{}&mut {}", prefix, sugg_expr), Applicability::MachineApplicable, false, ), hir::Mutability::Not => ( sp, "consider borrowing here", - format!("{}&{}", field_name, sugg_expr), + format!("{}&{}", prefix, sugg_expr), Applicability::MachineApplicable, false, ), @@ -883,32 +865,33 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { if self.infcx.type_is_copy_modulo_regions(self.param_env, expected, sp) || checked_ty.is_box() { - if let Ok(code) = sm.span_to_snippet(expr.span) { - let message = if checked_ty.is_box() { - "consider unboxing the value" - } else if checked_ty.is_region_ptr() { - "consider dereferencing the borrow" - } else { - "consider dereferencing the type" - }; - let (span, suggestion) = if is_struct_pat_shorthand_field { - (expr.span, format!("{}: *{}", code, code)) - } else if self.is_else_if_block(expr) { - // Don't suggest nonsense like `else *if` - return None; - } else if let Some(expr) = self.maybe_get_block_expr(expr.hir_id) { - (expr.span.shrink_to_lo(), "*".to_string()) - } else { - (expr.span.shrink_to_lo(), "*".to_string()) - }; - return Some(( - span, - message, - suggestion, - Applicability::MachineApplicable, - true, - )); - } + let message = if checked_ty.is_box() { + "consider unboxing the value" + } else if checked_ty.is_region_ptr() { + "consider dereferencing the borrow" + } else { + "consider dereferencing the type" + }; + let prefix = match self.maybe_get_struct_pattern_shorthand_field(expr) { + Some(ident) => format!("{}: ", ident), + None => format!(""), + }; + let (span, suggestion) = if self.is_else_if_block(expr) { + // Don't suggest nonsense like `else *if` + return None; + } else if let Some(expr) = self.maybe_get_block_expr(expr) { + // prefix should be empty here.. + (expr.span.shrink_to_lo(), "*".to_string()) + } else { + (expr.span.shrink_to_lo(), format!("{}*", prefix)) + }; + return Some(( + span, + message, + suggestion, + Applicability::MachineApplicable, + true, + )); } } } diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs b/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs index e8a0cc946b5..473c848ad8f 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs @@ -208,7 +208,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { pub fn suggest_deref_ref_or_into( &self, err: &mut DiagnosticBuilder<'_>, - expr: &hir::Expr<'_>, + expr: &hir::Expr<'tcx>, expected: Ty<'tcx>, found: Ty<'tcx>, expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>, @@ -231,7 +231,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } else if !self.check_for_cast(err, expr, found, expected, expected_ty_expr) { let is_struct_pat_shorthand_field = - self.is_hir_id_from_struct_pattern_shorthand_field(expr.hir_id, expr.span); + self.maybe_get_struct_pattern_shorthand_field(expr).is_some(); let methods = self.get_conversion_methods(expr.span, expected, found, expr.hir_id); if !methods.is_empty() { if let Ok(expr_text) = self.sess().source_map().span_to_snippet(expr.span) { diff --git a/src/test/ui/did_you_mean/compatible-variants.stderr b/src/test/ui/did_you_mean/compatible-variants.stderr index 1d4448764c1..0dfd8f5c128 100644 --- a/src/test/ui/did_you_mean/compatible-variants.stderr +++ b/src/test/ui/did_you_mean/compatible-variants.stderr @@ -143,7 +143,7 @@ LL | let _ = Foo { bar }; help: try wrapping the expression in `Some` | LL | let _ = Foo { bar: Some(bar) }; - | ~~~~~~~~~~~~~~ + | ++++++++++ + error: aborting due to 9 previous errors diff --git a/src/test/ui/inference/deref-suggestion.stderr b/src/test/ui/inference/deref-suggestion.stderr index da11ba204cb..28c9afaa52c 100644 --- a/src/test/ui/inference/deref-suggestion.stderr +++ b/src/test/ui/inference/deref-suggestion.stderr @@ -87,7 +87,7 @@ LL | let r = R { i }; help: consider dereferencing the borrow | LL | let r = R { i: *i }; - | ~~~~~ + | ++++ error[E0308]: mismatched types --> $DIR/deref-suggestion.rs:46:20