Skip to content

Commit

Permalink
When encountering &SmallImplCopy < &SmallImplCopy, suggest copying
Browse files Browse the repository at this point in the history
When encountering a binary operation for two `T: Copy` where `T` is as
small as a 64bit pointer, suggest dereferencing the expressions so the
binary operation is inlined.

Mitigate the effects of rust-lang#105259, particularly when match ergonomics is
involved.
  • Loading branch information
estebank committed Feb 23, 2023
1 parent fdbc432 commit 8f681d8
Show file tree
Hide file tree
Showing 51 changed files with 372 additions and 69 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_ast/src/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -894,7 +894,7 @@ impl PartialEq for Nonterminal {
fn eq(&self, rhs: &Self) -> bool {
match (self, rhs) {
(NtIdent(ident_lhs, is_raw_lhs), NtIdent(ident_rhs, is_raw_rhs)) => {
ident_lhs == ident_rhs && is_raw_lhs == is_raw_rhs
ident_lhs == ident_rhs && *is_raw_lhs == *is_raw_rhs
}
(NtLifetime(ident_lhs), NtLifetime(ident_rhs)) => ident_lhs == ident_rhs,
// FIXME: Assume that all "complex" nonterminal are not equal, we can't compare them
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ast/src/tokenstream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ impl TokenTree {
match (self, other) {
(TokenTree::Token(token, _), TokenTree::Token(token2, _)) => token.kind == token2.kind,
(TokenTree::Delimited(_, delim, tts), TokenTree::Delimited(_, delim2, tts2)) => {
delim == delim2 && tts.eq_unspanned(tts2)
*delim == *delim2 && tts.eq_unspanned(tts2)
}
_ => false,
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
self.impl_trait_defs = current_impl_trait_defs;
self.impl_trait_bounds = current_impl_trait_bounds;

debug_assert!(!self.children.iter().any(|(id, _)| id == &def_id));
debug_assert!(!self.children.iter().any(|(id, _)| *id == def_id));
self.children.push((def_id, hir::MaybeOwner::Owner(info)));
}

Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_ast_passes/src/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -688,8 +688,9 @@ fn check_incompatible_features(sess: &Session) {
.iter()
.filter(|&&(f1, f2)| features.enabled(f1) && features.enabled(f2))
{
if let Some((f1_name, f1_span)) = declared_features.clone().find(|(name, _)| name == f1) {
if let Some((f2_name, f2_span)) = declared_features.clone().find(|(name, _)| name == f2)
if let Some((f1_name, f1_span)) = declared_features.clone().find(|(name, _)| *name == *f1) {
if let Some((f2_name, f2_span)) =
declared_features.clone().find(|(name, _)| *name == *f2)
{
let spans = vec![f1_span, f2_span];
sess.struct_span_err(
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2825,7 +2825,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let mut arguments = Vec::new();
for (index, argument) in sig.inputs().skip_binder().iter().enumerate() {
if let ty::Ref(argument_region, _, _) = argument.kind() {
if argument_region == return_region {
if *argument_region == *return_region {
// Need to use the `rustc_middle::ty` types to compare against the
// `return_region`. Then use the `rustc_hir` type to get only
// the lifetime span.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -694,7 +694,7 @@ pub fn codegen_crate<B: ExtraBackendMethods>(
let cgus: Vec<_> = cgu_reuse
.iter()
.enumerate()
.filter(|&(_, reuse)| reuse == &CguReuse::No)
.filter(|&(_, reuse)| *reuse == CguReuse::No)
.take(tcx.sess.threads())
.collect();

Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_const_eval/src/interpret/terminator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,15 +258,15 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
(PassMode::Pair(a1, b1), PassMode::Pair(a2, b2)) => {
arg_attr_compat(a1, a2) && arg_attr_compat(b1, b2)
}
(PassMode::Cast(c1, pad1), PassMode::Cast(c2, pad2)) => c1 == c2 && pad1 == pad2,
(PassMode::Cast(c1, pad1), PassMode::Cast(c2, pad2)) => c1 == c2 && *pad1 == *pad2,
(
PassMode::Indirect { attrs: a1, extra_attrs: None, on_stack: s1 },
PassMode::Indirect { attrs: a2, extra_attrs: None, on_stack: s2 },
) => arg_attr_compat(a1, a2) && s1 == s2,
) => arg_attr_compat(a1, a2) && *s1 == *s2,
(
PassMode::Indirect { attrs: a1, extra_attrs: Some(e1), on_stack: s1 },
PassMode::Indirect { attrs: a2, extra_attrs: Some(e2), on_stack: s2 },
) => arg_attr_compat(a1, a2) && arg_attr_compat(e1, e2) && s1 == s2,
) => arg_attr_compat(a1, a2) && arg_attr_compat(e1, e2) && *s1 == *s2,
_ => false,
};

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_errors/src/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ pub trait Emitter: Translate {
if let Some((macro_kind, name)) = has_macro_spans.first() {
// Mark the actual macro this originates from
let and_then = if let Some((macro_kind, last_name)) = has_macro_spans.last()
&& last_name != name
&& *last_name != *name
{
let descr = macro_kind.descr();
format!(
Expand Down Expand Up @@ -2753,7 +2753,7 @@ pub fn is_case_difference(sm: &SourceMap, suggested: &str, sp: Span) -> bool {
let ascii_confusables = &['c', 'f', 'i', 'k', 'o', 's', 'u', 'v', 'w', 'x', 'y', 'z'];
// All the chars that differ in capitalization are confusable (above):
let confusable = iter::zip(found.chars(), suggested.chars())
.filter(|(f, s)| f != s)
.filter(|(f, s)| *f != *s)
.all(|(f, s)| (ascii_confusables.contains(&f) || ascii_confusables.contains(&s)));
confusable && found.to_lowercase() == suggested.to_lowercase()
// FIXME: We sometimes suggest the same thing we already have, which is a
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_expand/src/mbe/transcribe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ pub(super) fn transcribe<'a>(
if let Frame::Sequence { idx, sep, .. } = stack.last_mut().unwrap() {
let (repeat_idx, repeat_len) = repeats.last_mut().unwrap();
*repeat_idx += 1;
if repeat_idx < repeat_len {
if *repeat_idx < *repeat_len {
*idx = 0;
if let Some(sep) = sep {
result.push(TokenTree::Token(sep.clone(), Spacing::Alone));
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,7 @@ fn gather_gat_bounds<'tcx, T: TypeFoldable<TyCtxt<'tcx>>>(
for (region_b, region_b_idx) in &regions {
// Again, skip `'static` because it outlives everything. Also, we trivially
// know that a region outlives itself.
if ty::ReStatic == **region_b || region_a == region_b {
if ty::ReStatic == **region_b || *region_a == *region_b {
continue;
}
if region_known_to_outlive(
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_pretty/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2092,7 +2092,7 @@ impl<'a> State<'a> {

match bound {
GenericBound::Trait(tref, modifier) => {
if modifier == &TraitBoundModifier::Maybe {
if *modifier == TraitBoundModifier::Maybe {
self.word("?");
}
self.print_poly_trait_ref(tref);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// Remove one layer of references to account for `&mut self` and
// `&self`, so that we can compare it against the binding.
let (ty, def_self_ty) = match (ty.kind(), def_self_ty.kind()) {
(ty::Ref(_, ty, a), ty::Ref(_, self_ty, b)) if a == b => (*ty, *self_ty),
(ty::Ref(_, ty, a), ty::Ref(_, self_ty, b)) if *a == *b => (*ty, *self_ty),
_ => (ty, def_self_ty),
};
let mut param_args = FxHashMap::default();
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1739,7 +1739,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.check_expr_has_type_or_error(base_expr, adt_ty, |_| {
let base_ty = self.typeck_results.borrow().expr_ty(*base_expr);
let same_adt = match (adt_ty.kind(), base_ty.kind()) {
(ty::Adt(adt, _), ty::Adt(base_adt, _)) if adt == base_adt => true,
(ty::Adt(adt, _), ty::Adt(base_adt, _)) if *adt == *base_adt => true,
_ => false,
};
if self.tcx.sess.is_nightly_build() && same_adt {
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -815,7 +815,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
_ => None,
})
.map(|(ty, bounds)| match ty.kind() {
ty::Param(param_ty) if param_ty == expected_ty_as_param => Ok(Some(bounds)),
ty::Param(param_ty) if *param_ty == *expected_ty_as_param => Ok(Some(bounds)),
// check whether there is any predicate that contains our `T`, like `Option<T>: Send`
_ => match ty.contains(expected) {
true => Err(()),
Expand Down Expand Up @@ -848,7 +848,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

let ty_param_used_in_fn_params = fn_parameters.iter().any(|param| {
let ty = self.astconv().ast_ty_to_ty( param);
matches!(ty.kind(), ty::Param(fn_param_ty_param) if expected_ty_as_param == fn_param_ty_param)
matches!(ty.kind(), ty::Param(fn_param_ty_param) if *expected_ty_as_param == *fn_param_ty_param)
});

if ty_param_used_in_fn_params {
Expand Down Expand Up @@ -1008,7 +1008,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
) -> bool {
let ty::Adt(adt_def, substs) = expr_ty.kind() else { return false; };
let ty::Adt(expected_adt_def, expected_substs) = expected_ty.kind() else { return false; };
if adt_def != expected_adt_def {
if *adt_def != *expected_adt_def {
return false;
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_index/src/interval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ impl<I: Idx> IntervalSet<I> {
fn check_invariants(&self) -> bool {
let mut current: Option<u32> = None;
for (start, end) in &self.map {
if start > end || current.map_or(false, |x| x + 1 >= *start) {
if *start > *end || current.map_or(false, |x| x + 1 >= *start) {
return false;
}
current = Some(*end);
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1144,7 +1144,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
let remainder2: Vec<_> = sub2.types().skip(common_len).collect();
let common_default_params =
iter::zip(remainder1.iter().rev(), remainder2.iter().rev())
.filter(|(a, b)| a == b)
.filter(|(a, b)| **a == **b)
.count();
let len = sub1.len() - common_default_params;
let consts_offset = len - sub1.consts().count();
Expand Down Expand Up @@ -2028,7 +2028,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
}),
..
} = s
&& init_span == &self.span {
&& *init_span == self.span {
self.result = Some(*array_ty);
}
}
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_infer/src/infer/error_reporting/suggest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
(expected.kind(), found.kind())
{
if let ty::Adt(found_def, found_substs) = *found_ty.kind() {
if exp_def == &found_def {
if *exp_def == found_def {
let have_as_ref = &[
(
sym::Option,
Expand Down Expand Up @@ -581,7 +581,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
(
ty::Alias(ty::Opaque, ty::AliasTy { def_id: last_def_id, .. }),
ty::Alias(ty::Opaque, ty::AliasTy { def_id: exp_def_id, .. }),
) if last_def_id == exp_def_id => StatementAsExpression::CorrectType,
) if *last_def_id == *exp_def_id => StatementAsExpression::CorrectType,
(
ty::Alias(ty::Opaque, ty::AliasTy { def_id: last_def_id, substs: last_bounds, .. }),
ty::Alias(ty::Opaque, ty::AliasTy { def_id: exp_def_id, substs: exp_bounds, .. }),
Expand All @@ -607,14 +607,14 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
hir::GenericBound::Trait(tl, ml),
hir::GenericBound::Trait(tr, mr),
) if tl.trait_ref.trait_def_id() == tr.trait_ref.trait_def_id()
&& ml == mr =>
&& *ml == *mr =>
{
true
}
(
hir::GenericBound::LangItemTrait(langl, _, _, argsl),
hir::GenericBound::LangItemTrait(langr, _, _, argsr),
) if langl == langr => {
) if *langl == *langr => {
// FIXME: consider the bounds!
debug!("{:?} {:?}", argsl, argsr);
true
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_lint/locales/en-US.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ lint_array_into_iter =
.use_explicit_into_iter_suggestion =
or use `IntoIterator::into_iter(..)` instead of `.into_iter()` to explicitly iterate by value
lint_ref_binop_on_copy_type =
binary operation on reference to `Copy` type `{$ty}`
.note = `{$ty}` takes `{$bytes}` bytes of memory; copying the value instead of referencing it might avoid unnecessary pointer indirections
.suggestion = dereferencing the expressions will allow the compiler to more consistently optimize these binary operations
lint_enum_intrinsics_mem_discriminant =
the return value of `mem::discriminant` is unspecified when called with a non-enum type
.note = the argument to `discriminant` should be a reference to an enum, but it was passed a reference to a `{$ty_param}`, which is not an enum.
Expand Down
117 changes: 115 additions & 2 deletions compiler/rustc_lint/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ use crate::{
BuiltinUngatedAsyncFnTrackCaller, BuiltinUnnameableTestItems, BuiltinUnpermittedTypeInit,
BuiltinUnpermittedTypeInitSub, BuiltinUnreachablePub, BuiltinUnsafe,
BuiltinUnstableFeatures, BuiltinUnusedDocComment, BuiltinUnusedDocCommentSub,
BuiltinWhileTrue, SuggestChangingAssocTypes,
BuiltinWhileTrue, RefBinopOnCopyTypeDiag, RefBinopOnCopyTypeSuggestion,
SuggestChangingAssocTypes,
},
types::{transparent_newtype_field, CItemKind},
EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext,
Expand Down Expand Up @@ -2888,7 +2889,7 @@ impl ClashingExternDeclarations {
}
(Ref(_a_region, a_ty, a_mut), Ref(_b_region, b_ty, b_mut)) => {
// For structural sameness, we don't need the region to be same.
a_mut == b_mut
*a_mut == *b_mut
&& structurally_same_type_impl(seen_types, cx, *a_ty, *b_ty, ckind)
}
(FnDef(..), FnDef(..)) => {
Expand Down Expand Up @@ -3307,6 +3308,118 @@ impl EarlyLintPass for SpecialModuleName {
}
}

declare_lint! {
/// The `ref_binop_on_copy_type` lint detects borrowed `Copy` types being passed to binary
/// operations that have unnecessary borrows.
///
/// ### Example
///
/// ```rust
/// # #![allow(unused)]
/// pub fn slice_of_ints(input: &[(usize, usize, usize, usize)]) -> usize {
/// input
/// .iter()
/// .filter(|(a, b, c, d)| a <= c && d <= b || c <= a && b <= d)
/// .count()
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// When making comparisons (or other binary operations) between two reference values that can
/// be copied instead, the compiler will not always remove the references, making the execution
/// of the binary operation slower than it otherwise could be by making the machine code "chase
/// pointers" before actually finding the underlying value. If the `Copy` value is as small or
/// smaller than a 64 bit pointer, we suggest dereferencing the value so the compiler will have
/// a better chance of producing optimal instructions.
REF_BINOP_ON_COPY_TYPE,
Warn,
"detects binary operations on references to `Copy` types like `&42 < &50`",
}

declare_lint_pass!(RefBinopOnCopyType => [REF_BINOP_ON_COPY_TYPE]);

impl<'tcx> LateLintPass<'tcx> for RefBinopOnCopyType {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>) {
let hir::ExprKind::Binary(op, left, right) = expr.kind else { return; };
let left_ty = cx.typeck_results().expr_ty_adjusted(left);
let right_ty = cx.typeck_results().expr_ty_adjusted(right);
if let ty::Ref(_, left_ty, _) = left_ty.kind()
&& let ty::Ref(_, left_ty, _) = left_ty.kind()
&& let left_ty = left_ty.peel_refs()
&& left_ty.is_copy_modulo_regions(cx.tcx, cx.param_env)
&& let Some(size) = cx.tcx.layout_of(cx.param_env.and(left_ty)).ok().map(|layout| {
layout.size.bytes()
})
&& let ty::Ref(_, right_ty, _) = right_ty.kind()
&& let ty::Ref(_, right_ty, _) = right_ty.kind()
&& let right_ty = right_ty.peel_refs()
&& right_ty.is_copy_modulo_regions(cx.tcx, cx.param_env)
&& let Some(size_r) = cx.tcx.layout_of(cx.param_env.and(right_ty)).ok().map(|layout| {
layout.size.bytes()
})
&& size <= 8
&& size_r <= 8
{
let left_start_base = left.span.shrink_to_lo();
let left_peeled = left.peel_borrows();
let left_start = left_start_base.to(left_peeled.span.shrink_to_lo());
let left_sugg = if left_start != left_start_base
&& !matches!(cx.typeck_results().expr_ty_adjusted(left_peeled).kind(), ty::Ref(..))
{
""
} else {
"*"
};
let (left_brace_start, left_brace_end, left_end) =
if left.precedence().order() < ast::ExprPrecedence::Unary.order() {
("(", ")", Some(left.span.shrink_to_hi()))
} else {
("", "", None)
};
let right_start_base = right.span.shrink_to_lo();
let right_peeled = right.peel_borrows();
let right_start = right_start_base.to(right_peeled.span.shrink_to_lo());
let right_sugg = if right_start != right_start_base
&& !matches!(cx.typeck_results().expr_ty_adjusted(right_peeled).kind(), ty::Ref(..))
{
""
} else {
"*"
};
let (right_brace_start, right_brace_end, right_end) =
if right.precedence().order() < ast::ExprPrecedence::Unary.order() {
("(", ")", Some(right.span.shrink_to_hi()))
} else {
("", "", None)
};
cx.emit_spanned_lint(
REF_BINOP_ON_COPY_TYPE,
op.span,
RefBinopOnCopyTypeDiag {
ty: with_no_trimmed_paths!(left_ty.to_string()),
bytes: size,
note: Some(()),
suggestion: RefBinopOnCopyTypeSuggestion {
left_brace_start,
left_brace_end,
left_sugg,
left_start,
left_end,
right_brace_start,
right_brace_end,
right_sugg,
right_start,
right_end,
},
},
);
}
}
}

pub use rustc_session::lint::builtin::UNEXPECTED_CFGS;

declare_lint_pass!(UnexpectedCfgs => [UNEXPECTED_CFGS]);
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ late_lint_methods!(
ImproperCTypesDefinitions: ImproperCTypesDefinitions,
VariantSizeDifferences: VariantSizeDifferences,
BoxPointers: BoxPointers,
RefBinopOnCopyType: RefBinopOnCopyType,
PathStatements: PathStatements,
LetUnderscore: LetUnderscore,
// Depends on referenced function signatures in expressions
Expand Down
Loading

0 comments on commit 8f681d8

Please sign in to comment.