Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More accurate error for binop errors after identifying RHS type #90006

Closed
wants to merge 14 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions compiler/rustc_middle/src/ty/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,22 @@ pub fn suggest_constraining_type_param(
suggest_removing_unsized_bound(generics, err, param_name, param, def_id);
return true;
}

for bound in param.bounds {
if def_id.is_some() && def_id == bound.trait_ref().and_then(|tr| tr.trait_def_id()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

T: Trait<OneType> + Trait<AnotherType> is a legitimate combination of bounds, so with filtering based on def-ids the suggestions will perpetually recommend replacing Trait<OneType> with Trait<AnotherType> and vice versa in case of one of the bounds missing.

// FIXME: This can happen when we have `Foo<Bar>` when `Foo<Baz>` was intended, but it
// can also happen when using `Foo` where `~const Foo`. Right now the later isn't
// accounted for, but it is still a nightly feature, so I haven't tried to clean it up.
err.span_suggestion_verbose(
bound.span(),
&format!("consider changing this type parameter bound to `{}`", constraint),
constraint.to_string(),
Applicability::MaybeIncorrect,
);
return false;
}
}

let mut suggest_restrict = |span| {
err.span_suggestion_verbose(
span,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_resolve/src/late/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -966,6 +966,7 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {

match (res, source) {
(Res::Def(DefKind::Macro(MacroKind::Bang), _), _) => {
// FIXME: peek at the next token to see if it is one of `(`, `[` or `{`.
estebank marked this conversation as resolved.
Show resolved Hide resolved
err.span_label(span, fallback_label);
err.span_suggestion_verbose(
span.shrink_to_hi(),
Expand Down
51 changes: 32 additions & 19 deletions compiler/rustc_typeck/src/check/demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -844,6 +844,19 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let mut into_suggestion = sugg.clone();
into_suggestion.push((expr.span.shrink_to_hi(), format!("{}.into()", close_paren)));
let mut suffix_suggestion = sugg.clone();

if matches!(
(&expected_ty.kind(), &checked_ty.kind()),
(ty::Float(_), ty::Int(_) | ty::Uint(_))
) {
// Add fractional part from literal, for example `42.0f32` into `42`
estebank marked this conversation as resolved.
Show resolved Hide resolved
let len = src.trim_end_matches(&checked_ty.to_string()).len();
let len = src[..len].trim_end_matches("_").len();
let pos = expr.span.lo() + BytePos(len as u32);
let span = expr.span.with_lo(pos).with_hi(pos);
suffix_suggestion.push((span, ".0".to_string()));
};

suffix_suggestion.push((
if matches!(
(&expected_ty.kind(), &checked_ty.kind()),
Expand Down Expand Up @@ -915,19 +928,19 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
suggestion,
Applicability::MachineApplicable,
);
true
};

let suggest_to_change_suffix_or_into =
|err: &mut DiagnosticBuilder<'_>,
found_to_exp_is_fallible: bool,
exp_to_found_is_fallible: bool| {
let exp_is_lhs =
let exp_is_assign_lhs =
expected_ty_expr.map(|e| self.tcx.hir().is_lhs(e.hir_id)).unwrap_or(false);

if exp_is_lhs {
return;
if exp_is_assign_lhs {
return false;
}

let always_fallible = found_to_exp_is_fallible
&& (exp_to_found_is_fallible || expected_ty_expr.is_none());
let msg = if literal_is_ty_suffixed(expr) {
Expand All @@ -938,10 +951,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// expected type.
let msg = format!("`{}` cannot fit into type `{}`", src, expected_ty);
err.note(&msg);
return;
return false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a comment to fn check_for_cast explaining what the returned bool mean?
It seems weird to fail the cast check, but still modify the error builder.

} else if in_const_context {
// Do not recommend `into` or `try_into` in const contexts.
return;
return false;
} else if found_to_exp_is_fallible {
return suggest_fallible_into_or_lhs_from(err, exp_to_found_is_fallible);
} else {
Expand All @@ -953,6 +966,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
into_suggestion.clone()
};
err.multipart_suggestion_verbose(msg, suggestion, Applicability::MachineApplicable);
true
};

match (&expected_ty.kind(), &checked_ty.kind()) {
Expand All @@ -966,8 +980,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
(None, _) | (_, None) => (true, true),
_ => (false, false),
};
suggest_to_change_suffix_or_into(err, f2e_is_fallible, e2f_is_fallible);
true
suggest_to_change_suffix_or_into(err, f2e_is_fallible, e2f_is_fallible)
}
(&ty::Uint(ref exp), &ty::Uint(ref found)) => {
let (f2e_is_fallible, e2f_is_fallible) = match (exp.bit_width(), found.bit_width())
Expand All @@ -979,8 +992,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
(None, _) | (_, None) => (true, true),
_ => (false, false),
};
suggest_to_change_suffix_or_into(err, f2e_is_fallible, e2f_is_fallible);
true
suggest_to_change_suffix_or_into(err, f2e_is_fallible, e2f_is_fallible)
}
(&ty::Int(exp), &ty::Uint(found)) => {
let (f2e_is_fallible, e2f_is_fallible) = match (exp.bit_width(), found.bit_width())
Expand All @@ -989,8 +1001,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
(None, Some(8)) => (false, true),
_ => (true, true),
};
suggest_to_change_suffix_or_into(err, f2e_is_fallible, e2f_is_fallible);
true
suggest_to_change_suffix_or_into(err, f2e_is_fallible, e2f_is_fallible)
}
(&ty::Uint(exp), &ty::Int(found)) => {
let (f2e_is_fallible, e2f_is_fallible) = match (exp.bit_width(), found.bit_width())
Expand All @@ -999,12 +1010,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
(Some(8), None) => (true, false),
_ => (true, true),
};
suggest_to_change_suffix_or_into(err, f2e_is_fallible, e2f_is_fallible);
true
suggest_to_change_suffix_or_into(err, f2e_is_fallible, e2f_is_fallible)
}
(&ty::Float(ref exp), &ty::Float(ref found)) => {
if found.bit_width() < exp.bit_width() {
suggest_to_change_suffix_or_into(err, false, true);
return suggest_to_change_suffix_or_into(err, false, true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return suggest_to_change_suffix_or_into(err, false, true);
suggest_to_change_suffix_or_into(err, false, true)

?

} else if literal_is_ty_suffixed(expr) {
err.multipart_suggestion_verbose(
&lit_msg,
Expand All @@ -1028,15 +1038,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
suffix_suggestion,
Applicability::MachineApplicable,
);
true
} else if can_cast {
// Missing try_into implementation for `{float}` to `{integer}`
err.multipart_suggestion_verbose(
&format!("{}, rounding the float towards zero", msg),
cast_suggestion,
Applicability::MaybeIncorrect, // lossy conversion
);
true
} else {
false
}
true
}
(&ty::Float(ref exp), &ty::Uint(ref found)) => {
// if `found` is `None` (meaning found is `usize`), don't suggest `.into()`
Expand All @@ -1059,8 +1072,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// Missing try_into implementation for `{integer}` to `{float}`
err.multipart_suggestion_verbose(
&format!(
"{}, producing the floating point representation of the integer,
rounded if necessary",
"{}, producing the floating point representation of the integer, \
rounded if necessary",
cast_msg,
),
cast_suggestion,
Expand Down Expand Up @@ -1091,7 +1104,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
err.multipart_suggestion_verbose(
&format!(
"{}, producing the floating point representation of the integer, \
rounded if necessary",
rounded if necessary",
&msg,
),
cast_suggestion,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -656,7 +656,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
pub(in super::super) fn select_obligations_where_possible(
&self,
fallback_has_occurred: bool,
mutate_fulfillment_errors: impl Fn(&mut Vec<traits::FulfillmentError<'tcx>>),
mut mutate_fulfillment_errors: impl FnMut(&mut Vec<traits::FulfillmentError<'tcx>>),
) {
let result = self
.fulfillment_cx
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_typeck/src/check/method/confirm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> {
def_id: pick.item.def_id,
substs: all_substs,
sig: method_sig.skip_binder(),
trait_def_id: pick.item.container.id(),
};
ConfirmResult { callee, illegal_sized_bound }
}
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_typeck/src/check/method/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ pub struct MethodCallee<'tcx> {
/// substituted, normalized, and has had late-bound
/// lifetimes replaced with inference variables.
pub sig: ty::FnSig<'tcx>,

/// Used only for diagnostics, to skip redundant E0277 errors.
pub trait_def_id: DefId,
}

#[derive(Debug)]
Expand Down Expand Up @@ -432,7 +435,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
ty::Binder::dummy(ty::PredicateKind::WellFormed(method_ty.into())).to_predicate(tcx),
));

let callee = MethodCallee { def_id, substs, sig: fn_sig };
let callee = MethodCallee { def_id, substs, sig: fn_sig, trait_def_id };

debug!("callee = {:?}", callee);

Expand Down
Loading