Skip to content

Commit 774012d

Browse files
uellenberguellenberg
committed
Suggest using deref in patterns
Fixes #132784
1 parent 42b2496 commit 774012d

File tree

6 files changed

+363
-22
lines changed

6 files changed

+363
-22
lines changed

‎compiler/rustc_hir_typeck/src/pat.rs

+15-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@ use rustc_errors::{
1010
};
1111
use rustc_hir::def::{CtorKind, DefKind, Res};
1212
use rustc_hir::pat_util::EnumerateAndAdjustIterator;
13-
use rustc_hir::{self as hir, BindingMode, ByRef, HirId, LangItem, Mutability, Pat, PatKind};
13+
use rustc_hir::{
14+
self as hir, BindingMode, ByRef, HirId, LangItem, Mutability, Pat, PatKind, is_range_literal,
15+
};
1416
use rustc_infer::infer;
1517
use rustc_middle::ty::{self, Ty, TypeVisitableExt};
1618
use rustc_middle::{bug, span_bug};
@@ -94,10 +96,22 @@ struct PatInfo<'a, 'tcx> {
9496

9597
impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
9698
fn pattern_cause(&self, ti: &TopInfo<'tcx>, cause_span: Span) -> ObligationCause<'tcx> {
99+
let needs_parens = ti
100+
.origin_expr
101+
.map(|expr| match expr.kind {
102+
// parenthesize if needed (Issue #46756)
103+
hir::ExprKind::Cast(_, _) | hir::ExprKind::Binary(_, _, _) => true,
104+
// parenthesize borrows of range literals (Issue #54505)
105+
_ if is_range_literal(expr) => true,
106+
_ => false,
107+
})
108+
.unwrap_or(false);
109+
97110
let code = ObligationCauseCode::Pattern {
98111
span: ti.span,
99112
root_ty: ti.expected,
100113
origin_expr: ti.origin_expr.is_some(),
114+
prefix_suggestion_parentheses: needs_parens,
101115
};
102116
self.cause(cause_span, code)
103117
}

‎compiler/rustc_middle/src/traits/mod.rs

+3
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,9 @@ pub enum ObligationCauseCode<'tcx> {
314314
root_ty: Ty<'tcx>,
315315
/// Whether the `Span` came from an expression or a type expression.
316316
origin_expr: bool,
317+
/// If the `Span` came from an expression, does that expression need to be wrapped in
318+
/// parentheses for a prefix suggestion (i.e., dereference) to be valid.
319+
prefix_suggestion_parentheses: bool,
317320
},
318321

319322
/// Computing common supertype in an if expression

‎compiler/rustc_trait_selection/src/error_reporting/infer/mod.rs

+78-17
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ use rustc_middle::dep_graph::DepContext;
6464
use rustc_middle::ty::error::{ExpectedFound, TypeError, TypeErrorToStringExt};
6565
use rustc_middle::ty::print::{PrintError, PrintTraitRefExt as _, with_forced_trimmed_paths};
6666
use rustc_middle::ty::{
67-
self, List, Region, Ty, TyCtxt, TypeFoldable, TypeSuperVisitable, TypeVisitable,
67+
self, List, ParamEnv, Region, Ty, TyCtxt, TypeFoldable, TypeSuperVisitable, TypeVisitable,
6868
TypeVisitableExt,
6969
};
7070
use rustc_span::{BytePos, DesugaringKind, Pos, Span, sym};
@@ -74,7 +74,7 @@ use crate::error_reporting::TypeErrCtxt;
7474
use crate::errors::{ObligationCauseFailureCode, TypeErrorAdditionalDiags};
7575
use crate::infer;
7676
use crate::infer::relate::{self, RelateResult, TypeRelation};
77-
use crate::infer::{InferCtxt, TypeTrace, ValuePairs};
77+
use crate::infer::{InferCtxt, InferCtxtExt as _, TypeTrace, ValuePairs};
7878
use crate::solve::deeply_normalize_for_diagnostics;
7979
use crate::traits::{
8080
IfExpressionCause, MatchExpressionArmCause, ObligationCause, ObligationCauseCode,
@@ -339,9 +339,15 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
339339
cause: &ObligationCause<'tcx>,
340340
exp_found: Option<ty::error::ExpectedFound<Ty<'tcx>>>,
341341
terr: TypeError<'tcx>,
342+
param_env: Option<ParamEnv<'tcx>>,
342343
) {
343344
match *cause.code() {
344-
ObligationCauseCode::Pattern { origin_expr: true, span: Some(span), root_ty } => {
345+
ObligationCauseCode::Pattern {
346+
origin_expr: true,
347+
span: Some(span),
348+
root_ty,
349+
prefix_suggestion_parentheses,
350+
} => {
345351
let ty = self.resolve_vars_if_possible(root_ty);
346352
if !matches!(ty.kind(), ty::Infer(ty::InferTy::TyVar(_) | ty::InferTy::FreshTy(_)))
347353
{
@@ -358,16 +364,39 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
358364
err.span_label(span, format!("this expression has type `{ty}`"));
359365
}
360366
}
361-
if let Some(ty::error::ExpectedFound { found, .. }) = exp_found
362-
&& ty.boxed_ty() == Some(found)
363-
&& let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span)
364-
{
365-
err.span_suggestion(
366-
span,
367-
"consider dereferencing the boxed value",
368-
format!("*{snippet}"),
369-
Applicability::MachineApplicable,
370-
);
367+
if let Some(ty::error::ExpectedFound { found, .. }) = exp_found {
368+
// Parentheses are needed for cases like as casts.
369+
let mut snippet = if prefix_suggestion_parentheses {
370+
vec![
371+
(span.shrink_to_lo(), "(".to_string()),
372+
(span.shrink_to_hi(), ")".to_string()),
373+
]
374+
} else {
375+
vec![(span.shrink_to_lo(), "".to_string())]
376+
};
377+
378+
// Try giving a box suggestion first, as it is a special case of the
379+
// deref suggestion.
380+
if ty.boxed_ty() == Some(found) {
381+
snippet[0].1 = "*".to_string() + &snippet[0].1;
382+
383+
err.multipart_suggestion_verbose(
384+
"consider dereferencing the boxed value",
385+
snippet,
386+
Applicability::MachineApplicable,
387+
);
388+
} else if let Some(param_env) = param_env
389+
&& let Some(prefix) =
390+
self.should_deref_suggestion_on_mismatch(param_env, found, ty)
391+
{
392+
snippet[0].1 = prefix + &snippet[0].1;
393+
394+
err.multipart_suggestion_verbose(
395+
"consider dereferencing to access the inner value",
396+
snippet,
397+
Applicability::MaybeIncorrect,
398+
);
399+
}
371400
}
372401
}
373402
ObligationCauseCode::Pattern { origin_expr: false, span: Some(span), .. } => {
@@ -524,6 +553,38 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
524553
}
525554
}
526555

556+
/// Determines whether deref_to == <deref_from as Deref>::Target, and if so,
557+
/// returns a prefix that should be added to deref_from as a suggestion.
558+
fn should_deref_suggestion_on_mismatch(
559+
&self,
560+
param_env: ParamEnv<'tcx>,
561+
deref_to: Ty<'tcx>,
562+
deref_from: Ty<'tcx>,
563+
) -> Option<String> {
564+
// Find a way to autoderef from deraf_from to deref_to.
565+
let Some((num_derefs, (after_deref_ty, _))) = (self.autoderef_steps)(deref_from)
566+
.into_iter()
567+
.enumerate()
568+
.find(|(_, (ty, _))| self.infcx.can_eq(param_env, *ty, deref_to))
569+
else {
570+
return None;
571+
};
572+
573+
if num_derefs == 0 {
574+
return None;
575+
}
576+
577+
let deref_part = "*".repeat(num_derefs);
578+
579+
// Try to give a suggestion with the same type of reference/non-reference as the original code.
580+
// For example, if deref_from was a reference, then make the suggestion a reference as well.
581+
if deref_from.is_ref() && !after_deref_ty.is_ref() {
582+
Some(format!("&{deref_part}"))
583+
} else {
584+
Some(deref_part)
585+
}
586+
}
587+
527588
/// Given that `other_ty` is the same as a type argument for `name` in `sub`, populate `value`
528589
/// highlighting `name` and every type argument that isn't at `pos` (which is `other_ty`), and
529590
/// populate `other_value` with `other_ty`.
@@ -1312,8 +1373,8 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
13121373
Variable(ty::error::ExpectedFound<Ty<'a>>),
13131374
Fixed(&'static str),
13141375
}
1315-
let (expected_found, exp_found, is_simple_error, values) = match values {
1316-
None => (None, Mismatch::Fixed("type"), false, None),
1376+
let (expected_found, exp_found, is_simple_error, values, param_env) = match values {
1377+
None => (None, Mismatch::Fixed("type"), false, None, None),
13171378
Some(ty::ParamEnvAnd { param_env, value: values }) => {
13181379
let mut values = self.resolve_vars_if_possible(values);
13191380
if self.next_trait_solver() {
@@ -1365,7 +1426,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
13651426
diag.downgrade_to_delayed_bug();
13661427
return;
13671428
};
1368-
(Some(vals), exp_found, is_simple_error, Some(values))
1429+
(Some(vals), exp_found, is_simple_error, Some(values), Some(param_env))
13691430
}
13701431
};
13711432

@@ -1693,7 +1754,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
16931754

16941755
// It reads better to have the error origin as the final
16951756
// thing.
1696-
self.note_error_origin(diag, cause, exp_found, terr);
1757+
self.note_error_origin(diag, cause, exp_found, terr, param_env);
16971758

16981759
debug!(?diag);
16991760
}

‎tests/ui/let-else/let-else-deref-coercion.stderr

+8-4
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,20 @@ error[E0308]: mismatched types
22
--> $DIR/let-else-deref-coercion.rs:37:13
33
|
44
LL | let Bar::Present(z) = self else {
5-
| ^^^^^^^^^^^^^^^ ---- this expression has type `&mut Foo`
6-
| |
5+
| ^^^^^^^^^^^^^^^ ----
6+
| | |
7+
| | this expression has type `&mut Foo`
8+
| | help: consider dereferencing to access the inner value: `&mut **self`
79
| expected `Foo`, found `Bar`
810

911
error[E0308]: mismatched types
1012
--> $DIR/let-else-deref-coercion.rs:68:13
1113
|
1214
LL | let Bar(z) = x;
13-
| ^^^^^^ - this expression has type `&mut irrefutable::Foo`
14-
| |
15+
| ^^^^^^ -
16+
| | |
17+
| | this expression has type `&mut irrefutable::Foo`
18+
| | help: consider dereferencing to access the inner value: `&mut **x`
1519
| expected `Foo`, found `Bar`
1620

1721
error: aborting due to 2 previous errors

‎tests/ui/suggestions/suggest-deref-in-match-issue-132784.rs

+64
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
use std::sync::Arc;
2+
fn main() {
3+
let mut x = Arc::new(Some(1));
4+
match x {
5+
//~^ HELP consider dereferencing to access the inner value
6+
//~| HELP consider dereferencing to access the inner value
7+
Some(_) => {}
8+
//~^ ERROR mismatched types
9+
None => {}
10+
//~^ ERROR mismatched types
11+
}
12+
13+
let mut y = Box::new(Some(1));
14+
match y {
15+
//~^ HELP consider dereferencing to access the inner value
16+
//~| HELP consider dereferencing to access the inner value
17+
Some(_) => {}
18+
//~^ ERROR mismatched types
19+
None => {}
20+
//~^ ERROR mismatched types
21+
}
22+
23+
let mut z = Arc::new(Some(1));
24+
match z as Arc<Option<i32>> {
25+
//~^ HELP consider dereferencing to access the inner value
26+
//~| HELP consider dereferencing to access the inner value
27+
Some(_) => {}
28+
//~^ ERROR mismatched types
29+
None => {}
30+
//~^ ERROR mismatched types
31+
}
32+
33+
let z_const: &Arc<Option<i32>> = &z;
34+
match z_const {
35+
//~^ HELP consider dereferencing to access the inner value
36+
//~| HELP consider dereferencing to access the inner value
37+
Some(_) => {}
38+
//~^ ERROR mismatched types
39+
None => {}
40+
//~^ ERROR mismatched types
41+
}
42+
43+
// Normal reference because Arc doesn't implement DerefMut.
44+
let z_mut: &mut Arc<Option<i32>> = &mut z;
45+
match z_mut {
46+
//~^ HELP consider dereferencing to access the inner value
47+
//~| HELP consider dereferencing to access the inner value
48+
Some(_) => {}
49+
//~^ ERROR mismatched types
50+
None => {}
51+
//~^ ERROR mismatched types
52+
}
53+
54+
// Mutable reference because Box does implement DerefMut.
55+
let y_mut: &mut Box<Option<i32>> = &mut y;
56+
match y_mut {
57+
//~^ HELP consider dereferencing to access the inner value
58+
//~| HELP consider dereferencing to access the inner value
59+
Some(_) => {}
60+
//~^ ERROR mismatched types
61+
None => {}
62+
//~^ ERROR mismatched types
63+
}
64+
}

0 commit comments

Comments
 (0)