Skip to content

Commit

Permalink
Emit suggestions on both lhs and rhs
Browse files Browse the repository at this point in the history
  • Loading branch information
@sjwang05
sjwang05 committed Nov 29, 2023
1 parent bf93a77 commit a68e2e1
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 47 deletions.
42 changes: 26 additions & 16 deletions Cargo.lock
View file
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,7 @@ dependencies = [
"filetime",
"futures",
"if_chain",
"itertools",
"itertools 0.10.5",
"parking_lot 0.12.1",
"quote",
"regex",
Expand Down Expand Up @@ -579,7 +579,7 @@ dependencies = [
"aho-corasick 0.7.20",
"clap",
"indoc",
"itertools",
"itertools 0.10.5",
"opener",
"shell-escape",
"walkdir",
Expand All @@ -594,7 +594,7 @@ dependencies = [
"clippy_config",
"clippy_utils",
"declare_clippy_lint",
"itertools",
"itertools 0.10.5",
"quine-mc_cluskey",
"regex",
"regex-syntax 0.7.2",
Expand All @@ -616,7 +616,7 @@ version = "0.1.76"
dependencies = [
"arrayvec",
"clippy_config",
"itertools",
"itertools 0.10.5",
"rustc-semver",
]

Expand Down Expand Up @@ -999,7 +999,7 @@ checksum = "a0afaad2b26fa326569eb264b1363e8ae3357618c43982b3f285f0774ce76b69"
name = "declare_clippy_lint"
version = "0.1.76"
dependencies = [
"itertools",
"itertools 0.10.5",
"quote",
"syn 2.0.29",
]
Expand Down Expand Up @@ -2067,6 +2067,15 @@ dependencies = [
"either",
]

[[package]]
name = "itertools"
version = "0.12.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "25db6b064527c5d482d0423354fcd07a89a2dfe07b67892e62411946db7f07b0"
dependencies = [
"either",
]

[[package]]
name = "itoa"
version = "1.0.6"
Expand Down Expand Up @@ -3451,7 +3460,7 @@ dependencies = [
name = "rustc_ast_passes"
version = "0.0.0"
dependencies = [
"itertools",
"itertools 0.10.5",
"rustc_ast",
"rustc_ast_pretty",
"rustc_attr",
Expand Down Expand Up @@ -3510,7 +3519,7 @@ name = "rustc_borrowck"
version = "0.0.0"
dependencies = [
"either",
"itertools",
"itertools 0.10.5",
"polonius-engine",
"rustc_data_structures",
"rustc_errors",
Expand Down Expand Up @@ -3565,7 +3574,7 @@ version = "0.0.0"
dependencies = [
"bitflags 1.3.2",
"cstr",
"itertools",
"itertools 0.10.5",
"libc",
"measureme",
"object",
Expand Down Expand Up @@ -3601,7 +3610,7 @@ dependencies = [
"ar_archive_writer",
"bitflags 1.3.2",
"cc",
"itertools",
"itertools 0.10.5",
"jobserver",
"libc",
"object",
Expand Down Expand Up @@ -3670,7 +3679,7 @@ dependencies = [
"elsa",
"ena",
"indexmap",
"itertools",
"itertools 0.10.5",
"jobserver",
"libc",
"measureme",
Expand Down Expand Up @@ -4252,7 +4261,7 @@ version = "0.0.0"
dependencies = [
"coverage_test_macros",
"either",
"itertools",
"itertools 0.10.5",
"rustc_arena",
"rustc_ast",
"rustc_attr",
Expand Down Expand Up @@ -4326,7 +4335,7 @@ dependencies = [
name = "rustc_passes"
version = "0.0.0"
dependencies = [
"itertools",
"itertools 0.10.5",
"rustc_ast",
"rustc_ast_pretty",
"rustc_attr",
Expand Down Expand Up @@ -4552,6 +4561,7 @@ checksum = "8ba09476327c4b70ccefb6180f046ef588c26a24cf5d269a9feba316eb4f029f"
name = "rustc_trait_selection"
version = "0.0.0"
dependencies = [
"itertools 0.12.0",
"rustc_ast",
"rustc_attr",
"rustc_data_structures",
Expand Down Expand Up @@ -4590,7 +4600,7 @@ dependencies = [
name = "rustc_transmute"
version = "0.0.0"
dependencies = [
"itertools",
"itertools 0.10.5",
"rustc_data_structures",
"rustc_hir",
"rustc_infer",
Expand All @@ -4605,7 +4615,7 @@ dependencies = [
name = "rustc_ty_utils"
version = "0.0.0"
dependencies = [
"itertools",
"itertools 0.10.5",
"rustc_data_structures",
"rustc_errors",
"rustc_fluent_macro",
Expand Down Expand Up @@ -4652,7 +4662,7 @@ dependencies = [
"askama",
"expect-test",
"indexmap",
"itertools",
"itertools 0.10.5",
"minifier",
"once_cell",
"regex",
Expand Down Expand Up @@ -4734,7 +4744,7 @@ dependencies = [
"dirs",
"getopts",
"ignore",
"itertools",
"itertools 0.10.5",
"lazy_static",
"regex",
"rustfmt-config_proc_macro",
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_trait_selection/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ edition = "2021"

[dependencies]
# tidy-alphabetical-start
itertools = "0.12.0"
rustc_ast = { path = "../rustc_ast" }
rustc_attr = { path = "../rustc_attr" }
rustc_data_structures = { path = "../rustc_data_structures" }
Expand Down
119 changes: 88 additions & 31 deletions compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ use crate::traits::error_reporting::type_err_ctxt_ext::InferCtxtPrivExt;
use crate::traits::query::evaluate_obligation::InferCtxtExt as _;
use rustc_middle::ty::print::{with_forced_trimmed_paths, with_no_trimmed_paths};

use itertools::EitherOrBoth;
use itertools::Itertools;

#[derive(Debug)]
pub enum CoroutineInteriorOrUpvar {
// span of interior type
Expand Down Expand Up @@ -838,78 +841,132 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
}
}
}
} else if let (ObligationCauseCode::BinOp { lhs_hir_id, rhs_hir_id: Some(_rhs_hir_id), .. }, predicate) =
code.peel_derives_with_predicate()
} else if let (
ObligationCauseCode::BinOp { lhs_hir_id, rhs_hir_id: Some(rhs_hir_id), .. },
predicate,
) = code.peel_derives_with_predicate()
&& let Some(typeck_results) = &self.typeck_results
&& let hir::Node::Expr(lhs) = self.tcx.hir().get(*lhs_hir_id)
&& let hir::Node::Expr(rhs) = self.tcx.hir().get(*rhs_hir_id)
&& let Some(rhs_ty) = typeck_results.expr_ty_opt(rhs)
{
let trait_pred = predicate.unwrap_or(trait_pred);
let lhs_ty = self.tcx.instantiate_bound_regions_with_erased(trait_pred.self_ty());
let lhs_autoderef = (self.autoderef_steps)(lhs_ty);
if let Some(mut steps) =
lhs_autoderef.into_iter().enumerate().find_map(|(steps, (ty, obligations))| {
// Remapping bound vars here
let trait_pred_and_ty =
trait_pred.map_bound(|inner_trait_pred| (inner_trait_pred, ty));
let rhs_autoderef = (self.autoderef_steps)(rhs_ty);
let first_lhs = lhs_autoderef.first().unwrap().clone();
let first_rhs = rhs_autoderef.first().unwrap().clone();
let mut autoderefs = lhs_autoderef
.into_iter()
.enumerate()
.rev()
.zip_longest(rhs_autoderef.into_iter().enumerate().rev())
.map(|t| match t {
EitherOrBoth::Both(a, b) => (a, b),
EitherOrBoth::Left(a) => (a, (0, first_rhs.clone())),
EitherOrBoth::Right(b) => ((0, first_lhs.clone()), b),
})
.rev();
if let Some((lsteps, rsteps)) =
autoderefs.find_map(|((lsteps, (l_ty, _)), (rsteps, (r_ty, _)))| {
let trait_pred_and_ty = trait_pred.map_bound(|inner| {
(
ty::TraitPredicate {
trait_ref: ty::TraitRef::new(
self.tcx,
inner.trait_ref.def_id,
self.tcx.mk_args(&[l_ty.into(), r_ty.into()]),
),
polarity: inner.polarity,
},
l_ty,
)
});
let obligation = self.mk_trait_obligation_with_new_self_ty(
obligation.param_env,
trait_pred_and_ty,
);

let may_hold = obligations
.iter()
.chain([&obligation])
.all(|obligation| self.predicate_may_hold(obligation))
.then_some(steps);

may_hold
self.predicate_may_hold(&obligation).then_some(match (lsteps, rsteps) {
(_, 0) => (Some(lsteps), None),
(0, _) => (None, Some(rsteps)),
_ => (Some(lsteps), Some(rsteps)),
})
})
{
if steps > 0 {
// Suggest `&*` rather than `*&`
let span = lhs.peel_borrows().span;

let mut lhs = lhs;
let mut prefix_span = lhs.span.shrink_to_lo();
let make_sugg = |mut expr: &Expr<'_>, mut steps| {
let mut prefix_span = expr.span.shrink_to_lo();
let mut msg = "consider dereferencing here";
if let hir::ExprKind::AddrOf(_, _, inner) = lhs.kind {
if let hir::ExprKind::AddrOf(_, _, inner) = expr.kind {
msg = "consider removing the borrow and dereferencing instead";
if let hir::ExprKind::AddrOf(..) = inner.kind {
msg = "consider removing the borrows and dereferencing instead";
}
}
while let hir::ExprKind::AddrOf(_, _, inner) = lhs.kind
while let hir::ExprKind::AddrOf(_, _, inner) = expr.kind
&& steps > 0
{
prefix_span = prefix_span.with_hi(inner.span.lo());
lhs = inner;
expr = inner;
steps -= 1;
}
if steps == 0 {
msg = msg.trim_end_matches(" and dereferencing instead");
}

let derefs = "*".repeat(steps);
let needs_parens = steps > 0
&& match lhs.kind {
&& match expr.kind {
hir::ExprKind::Cast(_, _) | hir::ExprKind::Binary(_, _, _) => true,
_ if is_range_literal(lhs) => true,
_ if is_range_literal(expr) => true,
_ => false,
};
let mut suggestion = if needs_parens {
vec![
(span.shrink_to_lo(), format!("{derefs}(")),
(span.shrink_to_hi(), ")".to_string()),
(
expr.span.with_lo(prefix_span.hi()).shrink_to_lo(),
format!("{derefs}("),
),
(expr.span.shrink_to_hi(), ")".to_string()),
]
} else {
vec![(span.shrink_to_lo(), format!("{derefs}"))]
vec![(
expr.span.with_lo(prefix_span.hi()).shrink_to_lo(),
format!("{derefs}"),
)]
};
suggestion.push((prefix_span, "".to_string()));
(msg, suggestion)
};

if let Some(lsteps) = lsteps
&& let Some(rsteps) = rsteps
&& lsteps > 0
&& rsteps > 0
{
let mut suggestion = make_sugg(lhs, lsteps).1;
suggestion.append(&mut make_sugg(rhs, rsteps).1);
err.multipart_suggestion_verbose(
"consider dereferencing both sides",
suggestion,
Applicability::MachineApplicable,
);
} else if let Some(lsteps) = lsteps
&& lsteps > 0
{
let (msg, suggestion) = make_sugg(lhs, lsteps);
err.multipart_suggestion_verbose(
msg,
suggestion,
Applicability::MachineApplicable,
);
} else if let Some(rsteps) = rsteps
&& rsteps > 0
{
let (msg, suggestion) = make_sugg(rhs, rsteps);
err.multipart_suggestion_verbose(
msg,
suggestion,
Applicability::MachineApplicable,
);
return true;
}
}
}
Expand Down

0 comments on commit a68e2e1

Please sign in to comment.