Skip to content

Commit 85f5b8a

Browse files
malezjaamalezjaa
committed
resolve reviews
1 parent 572d1dc commit 85f5b8a

1 file changed

Lines changed: 69 additions & 84 deletions

File tree

  • compiler/rustc_hir_typeck/src

‎compiler/rustc_hir_typeck/src/op.rs‎

Lines changed: 69 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//! Code related to processing overloaded binary and unary operators.
22
3-
use rustc_ast as ast;
3+
use rustc_ast::{self as ast, AssignOp, BinOp};
44
use rustc_data_structures::packed::Pu128;
55
use rustc_errors::codes::*;
66
use rustc_errors::{Applicability, Diag, struct_span_code_err};
@@ -284,7 +284,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
284284
Ok(method) => {
285285
let by_ref_binop = !op.is_by_value();
286286

287-
if op.is_assign_op() || by_ref_binop {
287+
if matches!(op, Op::AssignOp(_)) || by_ref_binop {
288288
if let ty::Ref(_, _, mutbl) = method.sig.inputs()[0].kind() {
289289
let mutbl = AutoBorrowMutability::new(*mutbl, AllowTwoPhase::Yes);
290290
let autoref = Adjustment {
@@ -322,13 +322,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
322322
Err(_) if lhs_ty.references_error() || rhs_ty.references_error() => {
323323
Ty::new_misc_error(self.tcx)
324324
}
325-
Err(errors) => self.handle_binop_fulfilment_errors(
325+
Err(errors) => self.report_binop_fulfillment_errors(
326326
expr, lhs_expr, rhs_expr, op, expected, lhs_ty, rhs_ty, errors,
327327
),
328328
}
329329
}
330330

331-
fn handle_binop_fulfilment_errors(
331+
fn report_binop_fulfillment_errors(
332332
&self,
333333
expr: &'tcx Expr<'tcx>,
334334
lhs_expr: &'tcx Expr<'tcx>,
@@ -470,6 +470,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
470470
op_ok || self.can_eq(self.param_env, lhs_ty, rhs_ty)
471471
};
472472

473+
// We should suggest `a + b` => `*a + b` if `a` is copy, and suggest
474+
// `a += b` => `*a += b` if a is a mut ref.
473475
self.suggest_deref_or_call_for_binop_error(
474476
lhs_expr,
475477
rhs_expr,
@@ -484,8 +486,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
484486
if let Some(missing_trait) =
485487
trait_def_id.map(|def_id| with_no_trimmed_paths!(self.tcx.def_path_str(def_id)))
486488
{
487-
if op.is_add_op()
488-
&& self.check_str_addition(lhs_expr, rhs_expr, lhs_ty, rhs_ty, &mut err, op)
489+
if matches!(
490+
op,
491+
Op::BinOp(BinOp { node: BinOpKind::Add, .. })
492+
| Op::AssignOp(AssignOp { node: AssignOpKind::AddAssign, .. })
493+
) && self.check_str_addition(lhs_expr, rhs_expr, lhs_ty, rhs_ty, &mut err, op)
489494
{
490495
// This has nothing here because it means we did string
491496
// concatenation (e.g., "Hello " + "World!"). This means
@@ -528,6 +533,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
528533
}
529534
}
530535

536+
// Suggest using `add`, `offset` or `offset_from` for pointer - {integer},
537+
// pointer + {integer} or pointer - pointer.
531538
self.suggest_raw_ptr_binop_arithmetic(lhs_expr, rhs_expr, op, lhs_ty, rhs_ty, &mut err);
532539

533540
let lhs_name_str = match lhs_expr.kind {
@@ -571,14 +578,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
571578
return;
572579
}
573580

574-
// We should suggest `a + b` => `*a + b` if `a` is copy, and suggest
575-
// `a += b` => `*a += b` if a is a mut ref.
576581
if let Some(lhs_deref_ty) = self.deref_once_mutably_for_diagnostic(lhs_ty)
577-
&& op.is_assign_op()
582+
&& matches!(op, Op::AssignOp(_))
578583
{
579584
self.suggest_deref_binop(lhs_expr, rhs_expr, op, expected, rhs_ty, err, lhs_deref_ty);
580585
} else if let ty::Ref(region, lhs_deref_ty, mutbl) = lhs_ty.kind()
581-
&& op.is_bin_op()
586+
&& matches!(op, Op::BinOp(_))
582587
{
583588
if self.type_is_copy_modulo_regions(self.param_env, *lhs_deref_ty) {
584589
self.suggest_deref_binop(
@@ -668,39 +673,44 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
668673
return;
669674
}
670675

671-
if op.is_bin_op() && op.is_add_op() && lhs_ty.is_raw_ptr() && rhs_ty.is_integral() {
672-
err.multipart_suggestion(
673-
"consider using `wrapping_add` or `add` for pointer + {integer}",
674-
vec![
675-
(lhs_expr.span.between(rhs_expr.span), ".wrapping_add(".to_owned()),
676-
(rhs_expr.span.shrink_to_hi(), ")".to_owned()),
677-
],
678-
Applicability::MaybeIncorrect,
679-
);
680-
} else if op.is_bin_op() && op.is_sub_op() {
681-
if lhs_ty.is_raw_ptr() && rhs_ty.is_integral() {
676+
match op {
677+
Op::BinOp(BinOp { node: BinOpKind::Add, .. })
678+
if lhs_ty.is_raw_ptr() && rhs_ty.is_integral() =>
679+
{
682680
err.multipart_suggestion(
683-
"consider using `wrapping_sub` or `sub` for pointer - {integer}",
681+
"consider using `wrapping_add` or `add` for pointer + {integer}",
684682
vec![
685-
(lhs_expr.span.between(rhs_expr.span), ".wrapping_sub(".to_owned()),
683+
(lhs_expr.span.between(rhs_expr.span), ".wrapping_add(".to_owned()),
686684
(rhs_expr.span.shrink_to_hi(), ")".to_owned()),
687685
],
688686
Applicability::MaybeIncorrect,
689687
);
690688
}
691-
692-
if lhs_ty.is_raw_ptr() && rhs_ty.is_raw_ptr() {
693-
err.multipart_suggestion(
694-
"consider using `offset_from` for pointer - pointer if the \
695-
pointers point to the same allocation",
696-
vec![
697-
(lhs_expr.span.shrink_to_lo(), "unsafe { ".to_owned()),
698-
(lhs_expr.span.between(rhs_expr.span), ".offset_from(".to_owned()),
699-
(rhs_expr.span.shrink_to_hi(), ") }".to_owned()),
700-
],
701-
Applicability::MaybeIncorrect,
702-
);
689+
Op::BinOp(BinOp { node: BinOpKind::Sub, .. }) => {
690+
if lhs_ty.is_raw_ptr() && rhs_ty.is_integral() {
691+
err.multipart_suggestion(
692+
"consider using `wrapping_sub` or `sub` for pointer - {integer}",
693+
vec![
694+
(lhs_expr.span.between(rhs_expr.span), ".wrapping_sub(".to_owned()),
695+
(rhs_expr.span.shrink_to_hi(), ")".to_owned()),
696+
],
697+
Applicability::MaybeIncorrect,
698+
);
699+
}
700+
if lhs_ty.is_raw_ptr() && rhs_ty.is_raw_ptr() {
701+
err.multipart_suggestion(
702+
"consider using `offset_from` for pointer - pointer if the \
703+
pointers point to the same allocation",
704+
vec![
705+
(lhs_expr.span.shrink_to_lo(), "unsafe { ".to_owned()),
706+
(lhs_expr.span.between(rhs_expr.span), ".offset_from(".to_owned()),
707+
(rhs_expr.span.shrink_to_hi(), ") }".to_owned()),
708+
],
709+
Applicability::MaybeIncorrect,
710+
);
711+
}
703712
}
713+
_ => {}
704714
}
705715
}
706716

@@ -715,34 +725,32 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
715725
err: &mut Diag<'_>,
716726
) {
717727
if !op.span().can_be_used_for_suggestions()
718-
|| !op.is_assign_op()
728+
|| !matches!(op, Op::AssignOp(_))
719729
|| !lhs_ty.is_raw_ptr()
720730
|| !rhs_ty.is_integral()
721731
{
722732
return;
723733
}
724734

725-
if op.is_add_op() {
726-
err.multipart_suggestion(
727-
"consider using `add` or `wrapping_add` to do pointer arithmetic",
728-
vec![
729-
(lhs_expr.span.shrink_to_lo(), format!("{} = ", lhs_name_str)),
730-
(lhs_expr.span.between(rhs_expr.span), ".wrapping_add(".to_owned()),
731-
(rhs_expr.span.shrink_to_hi(), ")".to_owned()),
732-
],
733-
Applicability::MaybeIncorrect,
734-
);
735-
} else if op.is_sub_op() {
736-
err.multipart_suggestion(
737-
"consider using `sub` or `wrapping_sub` to do pointer arithmetic",
738-
vec![
739-
(lhs_expr.span.shrink_to_lo(), format!("{} = ", lhs_name_str)),
740-
(lhs_expr.span.between(rhs_expr.span), ".wrapping_sub(".to_owned()),
741-
(rhs_expr.span.shrink_to_hi(), ")".to_owned()),
742-
],
743-
Applicability::MaybeIncorrect,
744-
);
745-
}
735+
let (msg, method) = match op {
736+
Op::AssignOp(AssignOp { node: AssignOpKind::AddAssign, .. }) => {
737+
("consider using `add` or `wrapping_add` to do pointer arithmetic", "wrapping_add")
738+
}
739+
Op::AssignOp(AssignOp { node: AssignOpKind::SubAssign, .. }) => {
740+
("consider using `sub` or `wrapping_sub` to do pointer arithmetic", "wrapping_sub")
741+
}
742+
_ => return,
743+
};
744+
745+
err.multipart_suggestion(
746+
msg,
747+
vec![
748+
(lhs_expr.span.shrink_to_lo(), format!("{} = ", lhs_name_str)),
749+
(lhs_expr.span.between(rhs_expr.span), format!(".{method}(")),
750+
(rhs_expr.span.shrink_to_hi(), ")".to_owned()),
751+
],
752+
Applicability::MaybeIncorrect,
753+
);
746754
}
747755

748756
fn suggest_different_borrow(
@@ -896,7 +904,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
896904
if is_str_like(r_ty)
897905
|| matches!(r_ty.kind(), ty::Ref(_, inner, _) if *inner.kind() == ty::Str) =>
898906
{
899-
if op.is_bin_op() {
907+
// Do not supply this message if `&str += &str`
908+
if let Op::BinOp(_) = op {
900909
err.span_label(
901910
op.span(),
902911
"`+` cannot be used to concatenate two `&str` strings",
@@ -922,7 +931,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
922931
op.span(),
923932
"`+` cannot be used to concatenate a `&str` with a `String`",
924933
);
925-
if op.is_bin_op() {
934+
if matches!(op, Op::BinOp(_)) {
926935
let (lhs_span, lhs_replacement) = lhs_owned_sugg(lhs_expr);
927936
let (sugg_msg, lhs_replacement) = match lhs_replacement {
928937
None => (
@@ -942,7 +951,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
942951
],
943952
Applicability::MachineApplicable,
944953
);
945-
} else if op.is_assign_op() {
954+
} else if matches!(op, Op::AssignOp(_)) {
946955
err.note(str_concat_note);
947956
}
948957
true
@@ -1289,30 +1298,6 @@ impl Op {
12891298
Op::AssignOp(op) => op.node.is_by_value(),
12901299
}
12911300
}
1292-
1293-
fn is_assign_op(&self) -> bool {
1294-
matches!(self, Op::AssignOp(_))
1295-
}
1296-
1297-
fn is_bin_op(&self) -> bool {
1298-
matches!(self, Op::BinOp(_))
1299-
}
1300-
1301-
fn is_add_op(&self) -> bool {
1302-
matches!(
1303-
self,
1304-
Op::BinOp(hir::BinOp { node: hir::BinOpKind::Add, .. })
1305-
| Op::AssignOp(hir::AssignOp { node: hir::AssignOpKind::AddAssign, .. })
1306-
)
1307-
}
1308-
1309-
fn is_sub_op(&self) -> bool {
1310-
matches!(
1311-
self,
1312-
Op::BinOp(hir::BinOp { node: hir::BinOpKind::Sub, .. })
1313-
| Op::AssignOp(hir::AssignOp { node: hir::AssignOpKind::SubAssign, .. })
1314-
)
1315-
}
13161301
}
13171302

13181303
/// Dereferences a single level of immutable referencing.

0 commit comments

Comments
 (0)