Skip to content

Commit

Permalink
Suggest using deref in patterns
Browse files Browse the repository at this point in the history
Fixes #132784
  • Loading branch information
uellenberg committed Nov 12, 2024
1 parent 42b2496 commit 774012d
Show file tree
Hide file tree
Showing 6 changed files with 363 additions and 22 deletions.
16 changes: 15 additions & 1 deletion compiler/rustc_hir_typeck/src/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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)
}
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_middle/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
95 changes: 78 additions & 17 deletions compiler/rustc_trait_selection/src/error_reporting/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ 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};
Expand All @@ -74,7 +74,7 @@ use crate::error_reporting::TypeErrCtxt;
use crate::errors::{ObligationCauseFailureCode, TypeErrorAdditionalDiags};
use crate::infer;
use crate::infer::relate::{self, RelateResult, TypeRelation};
use crate::infer::{InferCtxt, TypeTrace, ValuePairs};
use crate::infer::{InferCtxt, InferCtxtExt as _, TypeTrace, ValuePairs};
use crate::solve::deeply_normalize_for_diagnostics;
use crate::traits::{
IfExpressionCause, MatchExpressionArmCause, ObligationCause, ObligationCauseCode,
Expand Down Expand Up @@ -339,9 +339,15 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
cause: &ObligationCause<'tcx>,
exp_found: Option<ty::error::ExpectedFound<Ty<'tcx>>>,
terr: TypeError<'tcx>,
param_env: Option<ParamEnv<'tcx>>,
) {
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(_)))
{
Expand All @@ -358,16 +364,39 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
err.span_label(span, format!("this expression has type `{ty}`"));
}
}
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)
{
err.span_suggestion(
span,
"consider dereferencing the boxed value",
format!("*{snippet}"),
Applicability::MachineApplicable,
);
if let Some(ty::error::ExpectedFound { found, .. }) = exp_found {
// Parentheses are needed for cases like as casts.
let mut snippet = if prefix_suggestion_parentheses {
vec![
(span.shrink_to_lo(), "(".to_string()),
(span.shrink_to_hi(), ")".to_string()),
]
} else {
vec![(span.shrink_to_lo(), "".to_string())]
};

// Try giving a box suggestion first, as it is a special case of the
// deref suggestion.
if ty.boxed_ty() == Some(found) {
snippet[0].1 = "*".to_string() + &snippet[0].1;

err.multipart_suggestion_verbose(
"consider dereferencing the boxed value",
snippet,
Applicability::MachineApplicable,
);
} else if let Some(param_env) = param_env
&& let Some(prefix) =
self.should_deref_suggestion_on_mismatch(param_env, found, ty)
{
snippet[0].1 = prefix + &snippet[0].1;

err.multipart_suggestion_verbose(
"consider dereferencing to access the inner value",
snippet,
Applicability::MaybeIncorrect,
);
}
}
}
ObligationCauseCode::Pattern { origin_expr: false, span: Some(span), .. } => {
Expand Down Expand Up @@ -524,6 +553,38 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
}
}

/// Determines whether deref_to == <deref_from as Deref>::Target, and if so,
/// returns a prefix that should be added to deref_from as a suggestion.
fn should_deref_suggestion_on_mismatch(
&self,
param_env: ParamEnv<'tcx>,
deref_to: Ty<'tcx>,
deref_from: Ty<'tcx>,
) -> Option<String> {
// Find a way to autoderef from deraf_from to deref_to.
let Some((num_derefs, (after_deref_ty, _))) = (self.autoderef_steps)(deref_from)
.into_iter()
.enumerate()
.find(|(_, (ty, _))| self.infcx.can_eq(param_env, *ty, deref_to))
else {
return None;
};

if num_derefs == 0 {
return None;
}

let deref_part = "*".repeat(num_derefs);

// Try to give a suggestion with the same type of reference/non-reference as the original code.
// For example, if deref_from was a reference, then make the suggestion a reference as well.
if deref_from.is_ref() && !after_deref_ty.is_ref() {
Some(format!("&{deref_part}"))
} else {
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`.
Expand Down Expand Up @@ -1312,8 +1373,8 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
Variable(ty::error::ExpectedFound<Ty<'a>>),
Fixed(&'static str),
}
let (expected_found, exp_found, is_simple_error, values) = match values {
None => (None, Mismatch::Fixed("type"), false, None),
let (expected_found, exp_found, is_simple_error, values, param_env) = match values {
None => (None, Mismatch::Fixed("type"), false, None, None),
Some(ty::ParamEnvAnd { param_env, value: values }) => {
let mut values = self.resolve_vars_if_possible(values);
if self.next_trait_solver() {
Expand Down Expand Up @@ -1365,7 +1426,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
diag.downgrade_to_delayed_bug();
return;
};
(Some(vals), exp_found, is_simple_error, Some(values))
(Some(vals), exp_found, is_simple_error, Some(values), Some(param_env))
}
};

Expand Down Expand Up @@ -1693,7 +1754,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);
}
Expand Down
12 changes: 8 additions & 4 deletions tests/ui/let-else/let-else-deref-coercion.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
64 changes: 64 additions & 0 deletions tests/ui/suggestions/suggest-deref-in-match-issue-132784.rs
Original file line number Diff line number Diff line change
@@ -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<Option<i32>> {
//~^ 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<Option<i32>> = &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<Option<i32>> = &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<Option<i32>> = &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
}
}
Loading

0 comments on commit 774012d

Please sign in to comment.