Skip to content

Commit

Permalink
suggest a float literal when dividing a floating-point type by {integer}
Browse files Browse the repository at this point in the history
fix a message

implement a rustfix-applicable suggestion

implement `suggest_floating_point_literal`

add `ObligationCauseCode::BinOp`

remove duplicate code

fix function names in uitests

use `Diagnostic` instead of `DiagnosticBuilder`
  • Loading branch information
TaKO8Ki committed Feb 26, 2022
1 parent 12b71ed commit c60bae7
Show file tree
Hide file tree
Showing 12 changed files with 543 additions and 18 deletions.
5 changes: 5 additions & 0 deletions compiler/rustc_middle/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,11 @@ pub enum ObligationCauseCode<'tcx> {

/// From `match_impl`. The cause for us having to match an impl, and the DefId we are matching against.
MatchImpl(ObligationCause<'tcx>, DefId),

BinOp {
rhs_span: Option<Span>,
is_lit: bool,
},
}

/// The 'location' at which we try to perform HIR-based wf checking.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
err.span_label(enclosing_scope_span, s.as_str());
}

self.suggest_floating_point_literal(&obligation, &mut err, &trait_ref);
self.suggest_dereferences(&obligation, &mut err, trait_predicate);
self.suggest_fn_call(&obligation, &mut err, trait_predicate);
self.suggest_remove_reference(&obligation, &mut err, trait_predicate);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,13 @@ pub trait InferCtxtExt<'tcx> {
trait_pred: ty::PolyTraitPredicate<'tcx>,
span: Span,
);

fn suggest_floating_point_literal(
&self,
obligation: &PredicateObligation<'tcx>,
err: &mut Diagnostic,
trait_ref: &ty::PolyTraitRef<'tcx>,
);
}

fn predicate_constraint(generics: &hir::Generics<'_>, pred: String) -> (Span, String) {
Expand Down Expand Up @@ -1910,8 +1917,9 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
| ObligationCauseCode::AwaitableExpr(_)
| ObligationCauseCode::ForLoopIterator
| ObligationCauseCode::QuestionMark
| ObligationCauseCode::CheckAssociatedTypeBounds { .. }
| ObligationCauseCode::LetElse
| ObligationCauseCode::CheckAssociatedTypeBounds { .. } => {}
| ObligationCauseCode::BinOp { .. } => {}
ObligationCauseCode::SliceOrArrayElem => {
err.note("slice and array elements must have `Sized` type");
}
Expand Down Expand Up @@ -2497,6 +2505,32 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
}
}
}

fn suggest_floating_point_literal(
&self,
obligation: &PredicateObligation<'tcx>,
err: &mut Diagnostic,
trait_ref: &ty::PolyTraitRef<'tcx>,
) {
let rhs_span = match obligation.cause.code() {
ObligationCauseCode::BinOp { rhs_span: Some(span), is_lit } if *is_lit => span,
_ => return,
};
match (
trait_ref.skip_binder().self_ty().kind(),
trait_ref.skip_binder().substs.type_at(1).kind(),
) {
(ty::Float(_), ty::Infer(InferTy::IntVar(_))) => {
err.span_suggestion_verbose(
rhs_span.shrink_to_hi(),
"consider using a floating-point literal by writing it with `.0`",
String::from(".0"),
Applicability::MaybeIncorrect,
);
}
_ => {}
}
}
}

/// Collect all the returned expressions within the input expression.
Expand Down
24 changes: 24 additions & 0 deletions compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,30 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
)
}

pub(in super::super) fn normalize_op_associated_types_in_as_infer_ok<T>(
&self,
span: Span,
value: T,
opt_input_expr: Option<&hir::Expr<'_>>,
) -> InferOk<'tcx, T>
where
T: TypeFoldable<'tcx>,
{
self.inh.partially_normalize_associated_types_in(
ObligationCause::new(
span,
self.body_id,
traits::BinOp {
rhs_span: opt_input_expr.map(|expr| expr.span),
is_lit: opt_input_expr
.map_or(false, |expr| matches!(expr.kind, ExprKind::Lit(_))),
},
),
self.param_env,
value,
)
}

pub fn require_type_meets(
&self,
ty: Ty<'tcx>,
Expand Down
128 changes: 119 additions & 9 deletions compiler/rustc_typeck/src/check/method/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,15 +333,57 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
)
}

pub(super) fn obligation_for_op_method(
&self,
span: Span,
trait_def_id: DefId,
self_ty: Ty<'tcx>,
opt_input_type: Option<Ty<'tcx>>,
opt_input_expr: Option<&'tcx hir::Expr<'tcx>>,
) -> (traits::Obligation<'tcx, ty::Predicate<'tcx>>, &'tcx ty::List<ty::subst::GenericArg<'tcx>>)
{
// Construct a trait-reference `self_ty : Trait<input_tys>`
let substs = InternalSubsts::for_item(self.tcx, trait_def_id, |param, _| {
match param.kind {
GenericParamDefKind::Lifetime | GenericParamDefKind::Const { .. } => {}
GenericParamDefKind::Type { .. } => {
if param.index == 0 {
return self_ty.into();
} else if let Some(input_type) = opt_input_type {
return input_type.into();
}
}
}
self.var_for_def(span, param)
});

let trait_ref = ty::TraitRef::new(trait_def_id, substs);

// Construct an obligation
let poly_trait_ref = ty::Binder::dummy(trait_ref);
(
traits::Obligation::new(
traits::ObligationCause::new(
span,
self.body_id,
traits::BinOp {
rhs_span: opt_input_expr.map(|expr| expr.span),
is_lit: opt_input_expr
.map_or(false, |expr| matches!(expr.kind, hir::ExprKind::Lit(_))),
},
),
self.param_env,
poly_trait_ref.without_const().to_predicate(self.tcx),
),
substs,
)
}

/// `lookup_method_in_trait` is used for overloaded operators.
/// It does a very narrow slice of what the normal probe/confirm path does.
/// In particular, it doesn't really do any probing: it simply constructs
/// an obligation for a particular trait with the given self type and checks
/// whether that trait is implemented.
//
// FIXME(#18741): it seems likely that we can consolidate some of this
// code with the other method-lookup code. In particular, the second half
// of this method is basically the same as confirmation.
#[instrument(level = "debug", skip(self, span, opt_input_types))]
pub(super) fn lookup_method_in_trait(
&self,
Expand All @@ -358,7 +400,57 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

let (obligation, substs) =
self.obligation_for_method(span, trait_def_id, self_ty, opt_input_types);
self.construct_obligation_for_trait(
span,
m_name,
trait_def_id,
obligation,
substs,
None,
false,
)
}

pub(super) fn lookup_op_method_in_trait(
&self,
span: Span,
m_name: Ident,
trait_def_id: DefId,
self_ty: Ty<'tcx>,
opt_input_type: Option<Ty<'tcx>>,
opt_input_expr: Option<&'tcx hir::Expr<'tcx>>,
) -> Option<InferOk<'tcx, MethodCallee<'tcx>>> {
let (obligation, substs) = self.obligation_for_op_method(
span,
trait_def_id,
self_ty,
opt_input_type,
opt_input_expr,
);
self.construct_obligation_for_trait(
span,
m_name,
trait_def_id,
obligation,
substs,
opt_input_expr,
true,
)
}

// FIXME(#18741): it seems likely that we can consolidate some of this
// code with the other method-lookup code. In particular, the second half
// of this method is basically the same as confirmation.
fn construct_obligation_for_trait(
&self,
span: Span,
m_name: Ident,
trait_def_id: DefId,
obligation: traits::PredicateObligation<'tcx>,
substs: &'tcx ty::List<ty::subst::GenericArg<'tcx>>,
opt_input_expr: Option<&'tcx hir::Expr<'tcx>>,
is_op: bool,
) -> Option<InferOk<'tcx, MethodCallee<'tcx>>> {
debug!(?obligation);

// Now we want to know if this can be matched
Expand Down Expand Up @@ -395,8 +487,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let fn_sig = self.replace_bound_vars_with_fresh_vars(span, infer::FnCall, fn_sig).0;
let fn_sig = fn_sig.subst(self.tcx, substs);

let InferOk { value, obligations: o } =
self.normalize_associated_types_in_as_infer_ok(span, fn_sig);
let InferOk { value, obligations: o } = if is_op {
self.normalize_op_associated_types_in_as_infer_ok(span, fn_sig, opt_input_expr)
} else {
self.normalize_associated_types_in_as_infer_ok(span, fn_sig)
};
let fn_sig = {
obligations.extend(o);
value
Expand All @@ -412,16 +507,31 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// any late-bound regions appearing in its bounds.
let bounds = self.tcx.predicates_of(def_id).instantiate(self.tcx, substs);

let InferOk { value, obligations: o } =
self.normalize_associated_types_in_as_infer_ok(span, bounds);
let InferOk { value, obligations: o } = if is_op {
self.normalize_op_associated_types_in_as_infer_ok(span, bounds, opt_input_expr)
} else {
self.normalize_associated_types_in_as_infer_ok(span, bounds)
};
let bounds = {
obligations.extend(o);
value
};

assert!(!bounds.has_escaping_bound_vars());

let cause = traits::ObligationCause::misc(span, self.body_id);
let cause = if is_op {
ObligationCause::new(
span,
self.body_id,
traits::BinOp {
rhs_span: opt_input_expr.map(|expr| expr.span),
is_lit: opt_input_expr
.map_or(false, |expr| matches!(expr.kind, hir::ExprKind::Lit(_))),
},
)
} else {
traits::ObligationCause::misc(span, self.body_id)
};
obligations.extend(traits::predicates_for_generics(cause.clone(), self.param_env, bounds));

// Also add an obligation for the method type being well-formed.
Expand Down
38 changes: 30 additions & 8 deletions compiler/rustc_typeck/src/check/op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
span: rhs_expr.span,
});

let result = self.lookup_op_method(lhs_ty, &[rhs_ty_var], Op::Binary(op, is_assign));
let result = self.lookup_op_method(
lhs_ty,
Some(rhs_ty_var),
Some(rhs_expr),
Op::Binary(op, is_assign),
);

// see `NB` above
let rhs_ty = self.check_expr_coercable_to_type(rhs_expr, rhs_ty_var, Some(lhs_expr));
Expand Down Expand Up @@ -382,6 +387,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
lhs_expr.span,
lhs_ty,
rhs_ty,
rhs_expr,
op,
is_assign,
);
Expand All @@ -390,6 +396,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
rhs_expr.span,
rhs_ty,
lhs_ty,
lhs_expr,
op,
is_assign,
);
Expand All @@ -400,7 +407,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
};
if let Ref(_, rty, _) = lhs_ty.kind() {
if self.infcx.type_is_copy_modulo_regions(self.param_env, *rty, lhs_expr.span)
&& self.lookup_op_method(*rty, &[rhs_ty], Op::Binary(op, is_assign)).is_ok()
&& self
.lookup_op_method(
*rty,
Some(rhs_ty),
Some(rhs_expr),
Op::Binary(op, is_assign),
)
.is_ok()
{
if let Ok(lstring) = source_map.span_to_snippet(lhs_expr.span) {
let msg = &format!(
Expand Down Expand Up @@ -443,7 +457,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let needs_bound = self
.lookup_op_method(
eraser.fold_ty(lhs_ty),
&[eraser.fold_ty(rhs_ty)],
Some(eraser.fold_ty(rhs_ty)),
Some(rhs_expr),
Op::Binary(op, is_assign),
)
.is_ok();
Expand Down Expand Up @@ -487,6 +502,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
span: Span,
ty: Ty<'tcx>,
other_ty: Ty<'tcx>,
other_expr: &'tcx hir::Expr<'tcx>,
op: hir::BinOp,
is_assign: IsAssign,
) -> bool /* did we suggest to call a function because of missing parentheses? */ {
Expand All @@ -513,7 +529,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
};

if self
.lookup_op_method(fn_sig.output(), &[other_ty], Op::Binary(op, is_assign))
.lookup_op_method(
fn_sig.output(),
Some(other_ty),
Some(other_expr),
Op::Binary(op, is_assign),
)
.is_ok()
{
let (variable_snippet, applicability) = if !fn_sig.inputs().is_empty() {
Expand Down Expand Up @@ -631,7 +652,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
op: hir::UnOp,
) -> Ty<'tcx> {
assert!(op.is_by_value());
match self.lookup_op_method(operand_ty, &[], Op::Unary(op, ex.span)) {
match self.lookup_op_method(operand_ty, None, None, Op::Unary(op, ex.span)) {
Ok(method) => {
self.write_method_call(ex.hir_id, method);
method.sig.output()
Expand Down Expand Up @@ -705,7 +726,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
fn lookup_op_method(
&self,
lhs_ty: Ty<'tcx>,
other_tys: &[Ty<'tcx>],
other_ty: Option<Ty<'tcx>>,
other_ty_expr: Option<&'tcx hir::Expr<'tcx>>,
op: Op,
) -> Result<MethodCallee<'tcx>, Vec<FulfillmentError<'tcx>>> {
let lang = self.tcx.lang_items();
Expand Down Expand Up @@ -791,7 +813,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

let opname = Ident::with_dummy_span(opname);
let method = trait_did.and_then(|trait_did| {
self.lookup_method_in_trait(span, opname, trait_did, lhs_ty, Some(other_tys))
self.lookup_op_method_in_trait(span, opname, trait_did, lhs_ty, other_ty, other_ty_expr)
});

match (method, trait_did) {
Expand All @@ -803,7 +825,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
(None, None) => Err(vec![]),
(None, Some(trait_did)) => {
let (obligation, _) =
self.obligation_for_method(span, trait_did, lhs_ty, Some(other_tys));
self.obligation_for_op_method(span, trait_did, lhs_ty, other_ty, other_ty_expr);
let mut fulfill = <dyn TraitEngine<'_>>::new(self.tcx);
fulfill.register_predicate_obligation(self, obligation);
Err(fulfill.select_where_possible(&self.infcx))
Expand Down
Loading

0 comments on commit c60bae7

Please sign in to comment.