Skip to content

When encountering &SmallImplCopy < &SmallImplCopy, suggest copying #108372

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

Closed
wants to merge 6 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
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
118 changes: 116 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,119 @@ 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)]
/// #![warn(ref_binop_on_copy_type)]
/// 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,
Allow,
"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
Comment on lines +3364 to +3365
Copy link
Member

Choose a reason for hiding this comment

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

Should we hard-code 8 here? Or get the usize-size from the compiler?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We would then give different suggestions depending on what platform the developer happens to be in. That is a departure of what we've been doing so far.

{
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