From ca719954e84033ac928989bc09969829c411d91a Mon Sep 17 00:00:00 2001 From: uellenberg Date: Mon, 11 Nov 2024 19:20:59 -0800 Subject: [PATCH] Suggest using deref in patterns Fixes #132784 --- .../src/check/compare_impl_item.rs | 3 + compiler/rustc_hir_analysis/src/check/mod.rs | 1 + .../rustc_hir_typeck/src/fn_ctxt/checks.rs | 1 + compiler/rustc_hir_typeck/src/pat.rs | 16 +- compiler/rustc_middle/src/traits/mod.rs | 3 + compiler/rustc_passes/src/check_attr.rs | 1 + .../src/error_reporting/infer/mod.rs | 134 +++++++++++-- .../traits/fulfillment_errors.rs | 2 + .../let-else/let-else-deref-coercion.stderr | 12 +- .../suggest-deref-in-match-issue-132784.rs | 64 ++++++ ...suggest-deref-in-match-issue-132784.stderr | 183 ++++++++++++++++++ 11 files changed, 404 insertions(+), 16 deletions(-) create mode 100644 tests/ui/suggestions/suggest-deref-in-match-issue-132784.rs create mode 100644 tests/ui/suggestions/suggest-deref-in-match-issue-132784.stderr diff --git a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs index 77c324183c357..dad05c318bce9 100644 --- a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs +++ b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs @@ -628,6 +628,7 @@ pub(super) fn collect_return_position_impl_trait_in_trait_tys<'tcx>( }))), terr, false, + None, ); return Err(diag.emit()); } @@ -1055,6 +1056,7 @@ fn report_trait_method_mismatch<'tcx>( }))), terr, false, + None, ); diag.emit() @@ -1852,6 +1854,7 @@ fn compare_const_predicate_entailment<'tcx>( }))), terr, false, + None, ); return Err(diag.emit()); }; diff --git a/compiler/rustc_hir_analysis/src/check/mod.rs b/compiler/rustc_hir_analysis/src/check/mod.rs index 375cbfd1c4fb8..a1d471d44bea1 100644 --- a/compiler/rustc_hir_analysis/src/check/mod.rs +++ b/compiler/rustc_hir_analysis/src/check/mod.rs @@ -651,6 +651,7 @@ pub fn check_function_signature<'tcx>( }))), err, false, + None, ); return Err(diag.emit()); } diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs index a6c249da103f3..f5f1f0cfa2441 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs @@ -1119,6 +1119,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { Some(self.param_env.and(trace.values)), e, true, + None, ); } } diff --git a/compiler/rustc_hir_typeck/src/pat.rs b/compiler/rustc_hir_typeck/src/pat.rs index 2a8ed26aa2bee..046016527dd18 100644 --- a/compiler/rustc_hir_typeck/src/pat.rs +++ b/compiler/rustc_hir_typeck/src/pat.rs @@ -10,7 +10,9 @@ use rustc_errors::{ }; use rustc_hir::def::{CtorKind, DefKind, Res}; use rustc_hir::pat_util::EnumerateAndAdjustIterator; -use rustc_hir::{self as hir, BindingMode, ByRef, HirId, LangItem, Mutability, Pat, PatKind}; +use rustc_hir::{ + self as hir, BindingMode, ByRef, HirId, LangItem, Mutability, Pat, PatKind, is_range_literal, +}; use rustc_infer::infer; use rustc_middle::ty::{self, Ty, TypeVisitableExt}; use rustc_middle::{bug, span_bug}; @@ -94,10 +96,22 @@ struct PatInfo<'a, 'tcx> { impl<'a, 'tcx> FnCtxt<'a, 'tcx> { fn pattern_cause(&self, ti: &TopInfo<'tcx>, cause_span: Span) -> ObligationCause<'tcx> { + let needs_parens = ti + .origin_expr + .map(|expr| match expr.kind { + // parenthesize if needed (Issue #46756) + hir::ExprKind::Cast(_, _) | hir::ExprKind::Binary(_, _, _) => true, + // parenthesize borrows of range literals (Issue #54505) + _ if is_range_literal(expr) => true, + _ => false, + }) + .unwrap_or(false); + let code = ObligationCauseCode::Pattern { span: ti.span, root_ty: ti.expected, origin_expr: ti.origin_expr.is_some(), + prefix_suggestion_parentheses: needs_parens, }; self.cause(cause_span, code) } diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index 09731d565b619..3f3be6c6e868f 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -314,6 +314,9 @@ pub enum ObligationCauseCode<'tcx> { root_ty: Ty<'tcx>, /// Whether the `Span` came from an expression or a type expression. origin_expr: bool, + /// If the `Span` came from an expression, does that expression need to be wrapped in + /// parentheses for a prefix suggestion (i.e., dereference) to be valid. + prefix_suggestion_parentheses: bool, }, /// Computing common supertype in an if expression diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs index 64a527ef1061f..4dab5b2d3b755 100644 --- a/compiler/rustc_passes/src/check_attr.rs +++ b/compiler/rustc_passes/src/check_attr.rs @@ -2321,6 +2321,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> { }))), terr, false, + None, ); diag.emit(); self.abort.set(true); diff --git a/compiler/rustc_trait_selection/src/error_reporting/infer/mod.rs b/compiler/rustc_trait_selection/src/error_reporting/infer/mod.rs index 929fa559d750c..e5f0efea7c443 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/infer/mod.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/infer/mod.rs @@ -58,13 +58,15 @@ use rustc_hir::def_id::DefId; use rustc_hir::intravisit::Visitor; use rustc_hir::lang_items::LangItem; use rustc_hir::{self as hir}; +use rustc_infer::infer::DefineOpaqueTypes; +use rustc_infer::traits::Obligation; use rustc_macros::extension; use rustc_middle::bug; use rustc_middle::dep_graph::DepContext; use rustc_middle::ty::error::{ExpectedFound, TypeError, TypeErrorToStringExt}; use rustc_middle::ty::print::{PrintError, PrintTraitRefExt as _, with_forced_trimmed_paths}; use rustc_middle::ty::{ - self, List, Region, Ty, TyCtxt, TypeFoldable, TypeSuperVisitable, TypeVisitable, + self, List, ParamEnv, Region, Ty, TyCtxt, TypeFoldable, TypeSuperVisitable, TypeVisitable, TypeVisitableExt, }; use rustc_span::{BytePos, DesugaringKind, Pos, Span, sym}; @@ -76,8 +78,10 @@ use crate::infer; use crate::infer::relate::{self, RelateResult, TypeRelation}; use crate::infer::{InferCtxt, TypeTrace, ValuePairs}; use crate::solve::deeply_normalize_for_diagnostics; +use crate::traits::query::evaluate_obligation::InferCtxtExt; use crate::traits::{ IfExpressionCause, MatchExpressionArmCause, ObligationCause, ObligationCauseCode, + ObligationCtxt, }; mod note_and_explain; @@ -339,9 +343,15 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { cause: &ObligationCause<'tcx>, exp_found: Option>>, terr: TypeError<'tcx>, + param_env: Option>, ) { match *cause.code() { - ObligationCauseCode::Pattern { origin_expr: true, span: Some(span), root_ty } => { + ObligationCauseCode::Pattern { + origin_expr: true, + span: Some(span), + root_ty, + prefix_suggestion_parentheses, + } => { let ty = self.resolve_vars_if_possible(root_ty); if !matches!(ty.kind(), ty::Infer(ty::InferTy::TyVar(_) | ty::InferTy::FreshTy(_))) { @@ -359,15 +369,35 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { } } if let Some(ty::error::ExpectedFound { found, .. }) = exp_found - && ty.boxed_ty() == Some(found) - && let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span) + && let Ok(mut snippet) = self.tcx.sess.source_map().span_to_snippet(span) { - err.span_suggestion( - span, - "consider dereferencing the boxed value", - format!("*{snippet}"), - Applicability::MachineApplicable, - ); + // Parentheses are needed for cases like as casts. + snippet = if prefix_suggestion_parentheses { + format!("({snippet})") + } else { + snippet + }; + + // Try giving a box suggestion first, as it is a special case of the + // deref suggestion. + if ty.boxed_ty() == Some(found) { + err.span_suggestion( + span, + "consider dereferencing the boxed value", + format!("*{snippet}"), + Applicability::MachineApplicable, + ); + } else if let Some(param_env) = param_env + && let Some(prefix) = + self.should_deref_suggestion_on_mismatch(cause, param_env, found, ty) + { + err.span_suggestion( + span, + "consider dereferencing to access the inner value", + format!("{prefix}{snippet}"), + Applicability::MaybeIncorrect, + ); + } } } ObligationCauseCode::Pattern { origin_expr: false, span: Some(span), .. } => { @@ -524,6 +554,86 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { } } + /// Determines whether deref_to == ::Target, and if so, + /// returns a prefix that should be added to deref_from as a suggestion. + fn should_deref_suggestion_on_mismatch( + &self, + cause: &ObligationCause<'tcx>, + param_env: ParamEnv<'tcx>, + deref_to: Ty<'tcx>, + deref_from: Ty<'tcx>, + ) -> Option { + let tcx = self.infcx.tcx; + + let deref_trait = tcx.lang_items().deref_trait()?; + let deref_mut_trait = tcx.lang_items().deref_mut_trait()?; + let deref_target = tcx.lang_items().deref_target()?; + + // Count the number of references in deref_from, since we need + // that plus 1 to invoke Deref. + // This is also needed to ensure we aren't checking for Deref on a reference. + let mut deref_from_cur = deref_from; + // Extra required dereference to invoke Deref. + let mut num_refs = 1; + while let ty::Ref(_, inner_ty, _) = deref_from_cur.kind() { + deref_from_cur = *inner_ty; + num_refs += 1; + } + + // + let trait_ref = ty::TraitRef::new(tcx, deref_trait, [deref_from_cur]); + let obligation = + Obligation::new(tcx, cause.clone(), param_env, ty::Binder::dummy(trait_ref)); + if !self.infcx.predicate_may_hold(&obligation) { + return None; + } + + let ocx = ObligationCtxt::new(self.infcx); + + // ::Target + let target = Ty::new_projection(tcx, deref_target, [deref_from_cur]); + + let target_normalized = ocx.structurally_normalize(cause, param_env, target).ok()?; + let expected_normalized = ocx.structurally_normalize(cause, param_env, deref_to).ok()?; + + // Determine whether target_normalized == expected_normalized with inference. + let types_eq = self.infcx.probe(|_| { + // FIXME: I don't know what DefineOpaqueTypes should be used here. + self.infcx + .at(cause, param_env) + .eq(DefineOpaqueTypes::No, expected_normalized, target_normalized) + .is_ok() + }); + + if !types_eq { + return None; + } + + let deref_part = "*".repeat(num_refs); + + // FIXME: Edge case: how should this handle double references? + // Currently, they're dereferenced correctly, but only the + // outer reference is added back after the dereferences. + + // Check if we have DerefMut, as it's required to create a mut reference. + let trait_ref_deref_mut = ty::TraitRef::new(tcx, deref_mut_trait, [deref_from_cur]); + let obligation_deref_mut = + Obligation::new(tcx, cause.clone(), param_env, ty::Binder::dummy(trait_ref_deref_mut)); + let has_deref_mut = self.infcx.predicate_may_hold(&obligation_deref_mut); + + // Try to give a suggestion with the same type of reference as the original code. + // For example, if deref_from was a reference, then make the suggestion a reference as well. + let valid_mutability = deref_from + .ref_mutability() + .map(|v| if has_deref_mut { v } else { hir::Mutability::Not }); + + match valid_mutability { + Some(hir::Mutability::Mut) => Some(format!("&mut {deref_part}")), + Some(hir::Mutability::Not) => Some(format!("&{deref_part}")), + None => Some(deref_part), + } + } + /// Given that `other_ty` is the same as a type argument for `name` in `sub`, populate `value` /// highlighting `name` and every type argument that isn't at `pos` (which is `other_ty`), and /// populate `other_value` with `other_ty`. @@ -1209,6 +1319,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { mut values: Option>>, terr: TypeError<'tcx>, prefer_label: bool, + param_env: Option>, ) { let span = cause.span; @@ -1693,7 +1804,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { // It reads better to have the error origin as the final // thing. - self.note_error_origin(diag, cause, exp_found, terr); + self.note_error_origin(diag, cause, exp_found, terr, param_env); debug!(?diag); } @@ -1868,6 +1979,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { Some(param_env.and(trace.values)), terr, false, + Some(param_env), ); diag } diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs index 1109b11d2a71a..b2068971add8f 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs @@ -727,6 +727,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { None, TypeError::Sorts(ty::error::ExpectedFound::new(true, expected_ty, ct_ty)), false, + None, ); diag } @@ -1456,6 +1457,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { }), err, false, + None, ); self.note_obligation_cause(&mut diag, obligation); diag.emit() diff --git a/tests/ui/let-else/let-else-deref-coercion.stderr b/tests/ui/let-else/let-else-deref-coercion.stderr index 143b838bac500..e80b78e832c49 100644 --- a/tests/ui/let-else/let-else-deref-coercion.stderr +++ b/tests/ui/let-else/let-else-deref-coercion.stderr @@ -2,16 +2,20 @@ error[E0308]: mismatched types --> $DIR/let-else-deref-coercion.rs:37:13 | LL | let Bar::Present(z) = self else { - | ^^^^^^^^^^^^^^^ ---- this expression has type `&mut Foo` - | | + | ^^^^^^^^^^^^^^^ ---- + | | | + | | this expression has type `&mut Foo` + | | help: consider dereferencing to access the inner value: `&mut **self` | expected `Foo`, found `Bar` error[E0308]: mismatched types --> $DIR/let-else-deref-coercion.rs:68:13 | LL | let Bar(z) = x; - | ^^^^^^ - this expression has type `&mut irrefutable::Foo` - | | + | ^^^^^^ - + | | | + | | this expression has type `&mut irrefutable::Foo` + | | help: consider dereferencing to access the inner value: `&mut **x` | expected `Foo`, found `Bar` error: aborting due to 2 previous errors diff --git a/tests/ui/suggestions/suggest-deref-in-match-issue-132784.rs b/tests/ui/suggestions/suggest-deref-in-match-issue-132784.rs new file mode 100644 index 0000000000000..cde7b70172165 --- /dev/null +++ b/tests/ui/suggestions/suggest-deref-in-match-issue-132784.rs @@ -0,0 +1,64 @@ +use std::sync::Arc; +fn main() { + let mut x = Arc::new(Some(1)); + match x { + //~^ HELP consider dereferencing to access the inner value + //~| HELP consider dereferencing to access the inner value + Some(_) => {} + //~^ ERROR mismatched types + None => {} + //~^ ERROR mismatched types + } + + let mut y = Box::new(Some(1)); + match y { + //~^ HELP consider dereferencing to access the inner value + //~| HELP consider dereferencing to access the inner value + Some(_) => {} + //~^ ERROR mismatched types + None => {} + //~^ ERROR mismatched types + } + + let mut z = Arc::new(Some(1)); + match z as Arc> { + //~^ HELP consider dereferencing to access the inner value + //~| HELP consider dereferencing to access the inner value + Some(_) => {} + //~^ ERROR mismatched types + None => {} + //~^ ERROR mismatched types + } + + let z_const: &Arc> = &z; + match z_const { + //~^ HELP consider dereferencing to access the inner value + //~| HELP consider dereferencing to access the inner value + Some(_) => {} + //~^ ERROR mismatched types + None => {} + //~^ ERROR mismatched types + } + + // Normal reference because Arc doesn't implement DerefMut. + let z_mut: &mut Arc> = &mut z; + match z_mut { + //~^ HELP consider dereferencing to access the inner value + //~| HELP consider dereferencing to access the inner value + Some(_) => {} + //~^ ERROR mismatched types + None => {} + //~^ ERROR mismatched types + } + + // Mutable reference because Box does implement DerefMut. + let y_mut: &mut Box> = &mut y; + match y_mut { + //~^ HELP consider dereferencing to access the inner value + //~| HELP consider dereferencing to access the inner value + Some(_) => {} + //~^ ERROR mismatched types + None => {} + //~^ ERROR mismatched types + } +} diff --git a/tests/ui/suggestions/suggest-deref-in-match-issue-132784.stderr b/tests/ui/suggestions/suggest-deref-in-match-issue-132784.stderr new file mode 100644 index 0000000000000..e04ad7d63ee44 --- /dev/null +++ b/tests/ui/suggestions/suggest-deref-in-match-issue-132784.stderr @@ -0,0 +1,183 @@ +error[E0308]: mismatched types + --> $DIR/issue-132784-suggest-deref-in-match.rs:7:9 + | +LL | match x { + | - + | | + | this expression has type `Arc>` + | help: consider dereferencing to access the inner value: `*x` +... +LL | Some(_) => {} + | ^^^^^^^ expected `Arc>`, found `Option<_>` + | + = note: expected struct `Arc>` + found enum `Option<_>` + +error[E0308]: mismatched types + --> $DIR/issue-132784-suggest-deref-in-match.rs:9:9 + | +LL | match x { + | - + | | + | this expression has type `Arc>` + | help: consider dereferencing to access the inner value: `*x` +... +LL | None => {} + | ^^^^ expected `Arc>`, found `Option<_>` + | + = note: expected struct `Arc>` + found enum `Option<_>` + +error[E0308]: mismatched types + --> $DIR/issue-132784-suggest-deref-in-match.rs:17:9 + | +LL | match y { + | - + | | + | this expression has type `Box>` + | help: consider dereferencing to access the inner value: `*y` +... +LL | Some(_) => {} + | ^^^^^^^ expected `Box>`, found `Option<_>` + | + = note: expected struct `Box>` + found enum `Option<_>` + +error[E0308]: mismatched types + --> $DIR/issue-132784-suggest-deref-in-match.rs:19:9 + | +LL | match y { + | - + | | + | this expression has type `Box>` + | help: consider dereferencing to access the inner value: `*y` +... +LL | None => {} + | ^^^^ expected `Box>`, found `Option<_>` + | + = note: expected struct `Box>` + found enum `Option<_>` + +error[E0308]: mismatched types + --> $DIR/issue-132784-suggest-deref-in-match.rs:27:9 + | +LL | match z as Arc> { + | --------------------- + | | + | this expression has type `Arc>` + | help: consider dereferencing to access the inner value: `*(z as Arc>)` +... +LL | Some(_) => {} + | ^^^^^^^ expected `Arc>`, found `Option<_>` + | + = note: expected struct `Arc>` + found enum `Option<_>` + +error[E0308]: mismatched types + --> $DIR/issue-132784-suggest-deref-in-match.rs:29:9 + | +LL | match z as Arc> { + | --------------------- + | | + | this expression has type `Arc>` + | help: consider dereferencing to access the inner value: `*(z as Arc>)` +... +LL | None => {} + | ^^^^ expected `Arc>`, found `Option<_>` + | + = note: expected struct `Arc>` + found enum `Option<_>` + +error[E0308]: mismatched types + --> $DIR/issue-132784-suggest-deref-in-match.rs:37:9 + | +LL | match z_const { + | ------- + | | + | this expression has type `&Arc>` + | help: consider dereferencing to access the inner value: `&**z_const` +... +LL | Some(_) => {} + | ^^^^^^^ expected `Arc>`, found `Option<_>` + | + = note: expected struct `Arc>` + found enum `Option<_>` + +error[E0308]: mismatched types + --> $DIR/issue-132784-suggest-deref-in-match.rs:39:9 + | +LL | match z_const { + | ------- + | | + | this expression has type `&Arc>` + | help: consider dereferencing to access the inner value: `&**z_const` +... +LL | None => {} + | ^^^^ expected `Arc>`, found `Option<_>` + | + = note: expected struct `Arc>` + found enum `Option<_>` + +error[E0308]: mismatched types + --> $DIR/issue-132784-suggest-deref-in-match.rs:48:9 + | +LL | match z_mut { + | ----- + | | + | this expression has type `&mut Arc>` + | help: consider dereferencing to access the inner value: `&**z_mut` +... +LL | Some(_) => {} + | ^^^^^^^ expected `Arc>`, found `Option<_>` + | + = note: expected struct `Arc>` + found enum `Option<_>` + +error[E0308]: mismatched types + --> $DIR/issue-132784-suggest-deref-in-match.rs:50:9 + | +LL | match z_mut { + | ----- + | | + | this expression has type `&mut Arc>` + | help: consider dereferencing to access the inner value: `&**z_mut` +... +LL | None => {} + | ^^^^ expected `Arc>`, found `Option<_>` + | + = note: expected struct `Arc>` + found enum `Option<_>` + +error[E0308]: mismatched types + --> $DIR/issue-132784-suggest-deref-in-match.rs:59:9 + | +LL | match y_mut { + | ----- + | | + | this expression has type `&mut Box>` + | help: consider dereferencing to access the inner value: `&mut **y_mut` +... +LL | Some(_) => {} + | ^^^^^^^ expected `Box>`, found `Option<_>` + | + = note: expected struct `Box>` + found enum `Option<_>` + +error[E0308]: mismatched types + --> $DIR/issue-132784-suggest-deref-in-match.rs:61:9 + | +LL | match y_mut { + | ----- + | | + | this expression has type `&mut Box>` + | help: consider dereferencing to access the inner value: `&mut **y_mut` +... +LL | None => {} + | ^^^^ expected `Box>`, found `Option<_>` + | + = note: expected struct `Box>` + found enum `Option<_>` + +error: aborting due to 12 previous errors + +For more information about this error, try `rustc --explain E0308`.