Skip to content

Commit

Permalink
Auto merge of rust-lang#116520 - Enselic:large-copy-into-fn, r=oli-obk
Browse files Browse the repository at this point in the history
large_assignments: Lint on specific large args passed to functions

Requires lowering function call arg spans down to MIR, which is done in the second commit.

Part of rust-lang#83518

Also see
* https://rust-lang.zulipchat.com/#narrow/stream/182449-t-compiler.2Fhelp/topic/arg.20Spans.20for.20TerminatorKind.3A.3ACall.3F
* https://rust-lang.zulipchat.com/#narrow/stream/122651-general/topic/move_size_limit.20lint

r? `@oli-obk` (E-mentor)
  • Loading branch information
bors committed Jan 16, 2024
2 parents f9c2421 + 8f440f0 commit 7d98bab
Show file tree
Hide file tree
Showing 52 changed files with 379 additions and 210 deletions.
8 changes: 6 additions & 2 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Expand Up @@ -1257,7 +1257,10 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
return None;
};
debug!("checking call args for uses of inner_param: {:?}", args);
args.contains(&Operand::Move(inner_param)).then_some((loc, term))
args.iter()
.map(|a| &a.node)
.any(|a| a == &Operand::Move(inner_param))
.then_some((loc, term))
})
else {
debug!("no uses of inner_param found as a by-move call arg");
Expand Down Expand Up @@ -3242,7 +3245,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
assigned_to, args
);
for operand in args {
let (Operand::Copy(assigned_from) | Operand::Move(assigned_from)) = operand
let (Operand::Copy(assigned_from) | Operand::Move(assigned_from)) =
&operand.node
else {
continue;
};
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs
Expand Up @@ -691,7 +691,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
);
// Check if one of the arguments to this function is the target place.
let found_target = args.iter().any(|arg| {
if let Operand::Move(place) = arg {
if let Operand::Move(place) = arg.node {
if let Some(potential) = place.as_local() {
potential == target
} else {
Expand Down
7 changes: 4 additions & 3 deletions compiler/rustc_borrowck/src/diagnostics/mod.rs
Expand Up @@ -23,6 +23,7 @@ use rustc_middle::ty::{self, Instance, Ty, TyCtxt};
use rustc_middle::util::{call_kind, CallDesugaringKind};
use rustc_mir_dataflow::move_paths::{InitLocation, LookupResult};
use rustc_span::def_id::LocalDefId;
use rustc_span::source_map::Spanned;
use rustc_span::{symbol::sym, Span, Symbol, DUMMY_SP};
use rustc_target::abi::{FieldIdx, VariantIdx};
use rustc_trait_selection::infer::InferCtxtExt;
Expand Down Expand Up @@ -111,9 +112,9 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
debug!("add_moved_or_invoked_closure_note: id={:?}", id);
if Some(self.infcx.tcx.parent(id)) == self.infcx.tcx.lang_items().fn_once_trait() {
let closure = match args.first() {
Some(Operand::Copy(place) | Operand::Move(place))
if target == place.local_or_deref_local() =>
{
Some(Spanned {
node: Operand::Copy(place) | Operand::Move(place), ..
}) if target == place.local_or_deref_local() => {
place.local_or_deref_local().unwrap()
}
_ => return false,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/lib.rs
Expand Up @@ -703,7 +703,7 @@ impl<'cx, 'tcx, R> rustc_mir_dataflow::ResultsVisitor<'cx, 'tcx, R> for MirBorro
} => {
self.consume_operand(loc, (func, span), flow_state);
for arg in args {
self.consume_operand(loc, (arg, span), flow_state);
self.consume_operand(loc, (&arg.node, arg.span), flow_state);
}
self.mutate_place(loc, (*destination, span), Deep, flow_state);
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/polonius/loan_invalidations.rs
Expand Up @@ -120,7 +120,7 @@ impl<'cx, 'tcx> Visitor<'tcx> for LoanInvalidationsGenerator<'cx, 'tcx> {
} => {
self.consume_operand(location, func);
for arg in args {
self.consume_operand(location, arg);
self.consume_operand(location, &arg.node);
}
self.mutate_place(location, *destination, Deep);
}
Expand Down
9 changes: 5 additions & 4 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Expand Up @@ -36,6 +36,7 @@ use rustc_middle::ty::{
};
use rustc_middle::ty::{GenericArgsRef, UserArgs};
use rustc_span::def_id::CRATE_DEF_ID;
use rustc_span::source_map::Spanned;
use rustc_span::symbol::sym;
use rustc_span::{Span, DUMMY_SP};
use rustc_target::abi::{FieldIdx, FIRST_VARIANT};
Expand Down Expand Up @@ -1359,7 +1360,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
TerminatorKind::Call { func, args, destination, call_source, target, .. } => {
self.check_operand(func, term_location);
for arg in args {
self.check_operand(arg, term_location);
self.check_operand(&arg.node, term_location);
}

let func_ty = func.ty(body, tcx);
Expand Down Expand Up @@ -1580,7 +1581,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
term: &Terminator<'tcx>,
func: &Operand<'tcx>,
sig: &ty::FnSig<'tcx>,
args: &[Operand<'tcx>],
args: &[Spanned<Operand<'tcx>>],
term_location: Location,
call_source: CallSource,
) {
Expand All @@ -1593,7 +1594,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
if self.tcx().is_intrinsic(def_id) {
match self.tcx().item_name(def_id) {
sym::simd_shuffle => {
if !matches!(args[2], Operand::Constant(_)) {
if !matches!(args[2], Spanned { node: Operand::Constant(_), .. }) {
self.tcx()
.dcx()
.emit_err(SimdShuffleLastConst { span: term.source_info.span });
Expand All @@ -1606,7 +1607,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
debug!(?func_ty);

for (n, (fn_arg, op_arg)) in iter::zip(sig.inputs(), args).enumerate() {
let op_arg_ty = op_arg.ty(body, self.tcx());
let op_arg_ty = op_arg.node.ty(body, self.tcx());

let op_arg_ty = self.normalize(op_arg_ty, term_location);
let category = if call_source.from_hir_call() {
Expand Down
13 changes: 7 additions & 6 deletions compiler/rustc_codegen_cranelift/src/abi/mod.rs
Expand Up @@ -11,6 +11,7 @@ use cranelift_module::ModuleError;
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
use rustc_middle::ty::layout::FnAbiOf;
use rustc_session::Session;
use rustc_span::source_map::Spanned;
use rustc_target::abi::call::{Conv, FnAbi};
use rustc_target::spec::abi::Abi;

Expand Down Expand Up @@ -360,7 +361,7 @@ pub(crate) fn codegen_terminator_call<'tcx>(
fx: &mut FunctionCx<'_, '_, 'tcx>,
source_info: mir::SourceInfo,
func: &Operand<'tcx>,
args: &[Operand<'tcx>],
args: &[Spanned<Operand<'tcx>>],
destination: Place<'tcx>,
target: Option<BasicBlock>,
) {
Expand Down Expand Up @@ -415,7 +416,7 @@ pub(crate) fn codegen_terminator_call<'tcx>(

let extra_args = &args[fn_sig.inputs().skip_binder().len()..];
let extra_args = fx.tcx.mk_type_list_from_iter(
extra_args.iter().map(|op_arg| fx.monomorphize(op_arg.ty(fx.mir, fx.tcx))),
extra_args.iter().map(|op_arg| fx.monomorphize(op_arg.node.ty(fx.mir, fx.tcx))),
);
let fn_abi = if let Some(instance) = instance {
RevealAllLayoutCx(fx.tcx).fn_abi_of_instance(instance, extra_args)
Expand All @@ -440,10 +441,10 @@ pub(crate) fn codegen_terminator_call<'tcx>(
// Unpack arguments tuple for closures
let mut args = if fn_sig.abi() == Abi::RustCall {
let (self_arg, pack_arg) = match args {
[pack_arg] => (None, codegen_call_argument_operand(fx, pack_arg)),
[pack_arg] => (None, codegen_call_argument_operand(fx, &pack_arg.node)),
[self_arg, pack_arg] => (
Some(codegen_call_argument_operand(fx, self_arg)),
codegen_call_argument_operand(fx, pack_arg),
Some(codegen_call_argument_operand(fx, &self_arg.node)),
codegen_call_argument_operand(fx, &pack_arg.node),
),
_ => panic!("rust-call abi requires one or two arguments"),
};
Expand All @@ -463,7 +464,7 @@ pub(crate) fn codegen_terminator_call<'tcx>(
}
args
} else {
args.iter().map(|arg| codegen_call_argument_operand(fx, arg)).collect::<Vec<_>>()
args.iter().map(|arg| codegen_call_argument_operand(fx, &arg.node)).collect::<Vec<_>>()
};

// Pass the caller location for `#[track_caller]`.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_cranelift/src/intrinsics/llvm.rs
Expand Up @@ -7,7 +7,7 @@ pub(crate) fn codegen_llvm_intrinsic_call<'tcx>(
fx: &mut FunctionCx<'_, '_, 'tcx>,
intrinsic: &str,
generic_args: GenericArgsRef<'tcx>,
args: &[mir::Operand<'tcx>],
args: &[Spanned<mir::Operand<'tcx>>],
ret: CPlace<'tcx>,
target: Option<BasicBlock>,
span: Span,
Expand Down
Expand Up @@ -7,7 +7,7 @@ pub(crate) fn codegen_aarch64_llvm_intrinsic_call<'tcx>(
fx: &mut FunctionCx<'_, '_, 'tcx>,
intrinsic: &str,
_args: GenericArgsRef<'tcx>,
args: &[mir::Operand<'tcx>],
args: &[Spanned<mir::Operand<'tcx>>],
ret: CPlace<'tcx>,
target: Option<BasicBlock>,
) {
Expand Down
86 changes: 43 additions & 43 deletions compiler/rustc_codegen_cranelift/src/intrinsics/llvm_x86.rs
Expand Up @@ -11,7 +11,7 @@ pub(crate) fn codegen_x86_llvm_intrinsic_call<'tcx>(
fx: &mut FunctionCx<'_, '_, 'tcx>,
intrinsic: &str,
_args: GenericArgsRef<'tcx>,
args: &[mir::Operand<'tcx>],
args: &[Spanned<mir::Operand<'tcx>>],
ret: CPlace<'tcx>,
target: Option<BasicBlock>,
span: Span,
Expand Down Expand Up @@ -175,9 +175,9 @@ pub(crate) fn codegen_x86_llvm_intrinsic_call<'tcx>(
[x, y, kind] => (x, y, kind),
_ => bug!("wrong number of args for intrinsic {intrinsic}"),
};
let x = codegen_operand(fx, x);
let y = codegen_operand(fx, y);
let kind = match kind {
let x = codegen_operand(fx, &x.node);
let y = codegen_operand(fx, &y.node);
let kind = match &kind.node {
Operand::Constant(const_) => crate::constant::eval_mir_constant(fx, const_).0,
Operand::Copy(_) | Operand::Move(_) => unreachable!("{kind:?}"),
};
Expand Down Expand Up @@ -287,8 +287,8 @@ pub(crate) fn codegen_x86_llvm_intrinsic_call<'tcx>(
[a, b] => (a, b),
_ => bug!("wrong number of args for intrinsic {intrinsic}"),
};
let a = codegen_operand(fx, a);
let b = codegen_operand(fx, b);
let a = codegen_operand(fx, &a.node);
let b = codegen_operand(fx, &b.node);

// Based on the pseudocode at https://github.com/rust-lang/stdarch/blob/1cfbca8b38fd9b4282b2f054f61c6ca69fc7ce29/crates/core_arch/src/x86/avx2.rs#L2319-L2332
let zero = fx.bcx.ins().iconst(types::I8, 0);
Expand Down Expand Up @@ -325,9 +325,9 @@ pub(crate) fn codegen_x86_llvm_intrinsic_call<'tcx>(
[a, b, imm8] => (a, b, imm8),
_ => bug!("wrong number of args for intrinsic {intrinsic}"),
};
let a = codegen_operand(fx, a);
let b = codegen_operand(fx, b);
let imm8 = codegen_operand(fx, imm8).load_scalar(fx);
let a = codegen_operand(fx, &a.node);
let b = codegen_operand(fx, &b.node);
let imm8 = codegen_operand(fx, &imm8.node).load_scalar(fx);

let a_low = a.value_typed_lane(fx, fx.tcx.types.u128, 0).load_scalar(fx);
let a_high = a.value_typed_lane(fx, fx.tcx.types.u128, 1).load_scalar(fx);
Expand Down Expand Up @@ -956,14 +956,14 @@ pub(crate) fn codegen_x86_llvm_intrinsic_call<'tcx>(
let b = b.load_scalar(fx);
let lb = lb.load_scalar(fx);

let imm8 = if let Some(imm8) = crate::constant::mir_operand_get_const_val(fx, &args[4])
{
imm8
} else {
fx.tcx
.dcx()
.span_fatal(span, "Index argument for `_mm_cmpestri` is not a constant");
};
let imm8 =
if let Some(imm8) = crate::constant::mir_operand_get_const_val(fx, &args[4].node) {
imm8
} else {
fx.tcx
.dcx()
.span_fatal(span, "Index argument for `_mm_cmpestri` is not a constant");
};

let imm8 = imm8.try_to_u8().unwrap_or_else(|_| panic!("kind not scalar: {:?}", imm8));

Expand Down Expand Up @@ -1009,14 +1009,14 @@ pub(crate) fn codegen_x86_llvm_intrinsic_call<'tcx>(
let b = b.load_scalar(fx);
let lb = lb.load_scalar(fx);

let imm8 = if let Some(imm8) = crate::constant::mir_operand_get_const_val(fx, &args[4])
{
imm8
} else {
fx.tcx
.dcx()
.span_fatal(span, "Index argument for `_mm_cmpestrm` is not a constant");
};
let imm8 =
if let Some(imm8) = crate::constant::mir_operand_get_const_val(fx, &args[4].node) {
imm8
} else {
fx.tcx
.dcx()
.span_fatal(span, "Index argument for `_mm_cmpestrm` is not a constant");
};

let imm8 = imm8.try_to_u8().unwrap_or_else(|_| panic!("kind not scalar: {:?}", imm8));

Expand Down Expand Up @@ -1056,15 +1056,15 @@ pub(crate) fn codegen_x86_llvm_intrinsic_call<'tcx>(
let a = a.load_scalar(fx);
let b = b.load_scalar(fx);

let imm8 = if let Some(imm8) = crate::constant::mir_operand_get_const_val(fx, &args[2])
{
imm8
} else {
fx.tcx.dcx().span_fatal(
span,
"Index argument for `_mm_clmulepi64_si128` is not a constant",
);
};
let imm8 =
if let Some(imm8) = crate::constant::mir_operand_get_const_val(fx, &args[2].node) {
imm8
} else {
fx.tcx.dcx().span_fatal(
span,
"Index argument for `_mm_clmulepi64_si128` is not a constant",
);
};

let imm8 = imm8.try_to_u8().unwrap_or_else(|_| panic!("kind not scalar: {:?}", imm8));

Expand Down Expand Up @@ -1093,15 +1093,15 @@ pub(crate) fn codegen_x86_llvm_intrinsic_call<'tcx>(

let a = a.load_scalar(fx);

let imm8 = if let Some(imm8) = crate::constant::mir_operand_get_const_val(fx, &args[1])
{
imm8
} else {
fx.tcx.dcx().span_fatal(
span,
"Index argument for `_mm_aeskeygenassist_si128` is not a constant",
);
};
let imm8 =
if let Some(imm8) = crate::constant::mir_operand_get_const_val(fx, &args[1].node) {
imm8
} else {
fx.tcx.dcx().span_fatal(
span,
"Index argument for `_mm_aeskeygenassist_si128` is not a constant",
);
};

let imm8 = imm8.try_to_u8().unwrap_or_else(|_| panic!("kind not scalar: {:?}", imm8));

Expand Down
22 changes: 13 additions & 9 deletions compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs
Expand Up @@ -5,7 +5,7 @@ macro_rules! intrinsic_args {
($fx:expr, $args:expr => ($($arg:tt),*); $intrinsic:expr) => {
#[allow(unused_parens)]
let ($($arg),*) = if let [$($arg),*] = $args {
($(codegen_operand($fx, $arg)),*)
($(codegen_operand($fx, &($arg).node)),*)
} else {
$crate::intrinsics::bug_on_incorrect_arg_count($intrinsic);
};
Expand All @@ -22,6 +22,7 @@ use rustc_middle::ty;
use rustc_middle::ty::layout::{HasParamEnv, ValidityRequirement};
use rustc_middle::ty::print::{with_no_trimmed_paths, with_no_visible_paths};
use rustc_middle::ty::GenericArgsRef;
use rustc_span::source_map::Spanned;
use rustc_span::symbol::{kw, sym, Symbol};

pub(crate) use self::llvm::codegen_llvm_intrinsic_call;
Expand Down Expand Up @@ -263,7 +264,7 @@ fn bool_to_zero_or_max_uint<'tcx>(
pub(crate) fn codegen_intrinsic_call<'tcx>(
fx: &mut FunctionCx<'_, '_, 'tcx>,
instance: Instance<'tcx>,
args: &[mir::Operand<'tcx>],
args: &[Spanned<mir::Operand<'tcx>>],
destination: CPlace<'tcx>,
target: Option<BasicBlock>,
source_info: mir::SourceInfo,
Expand Down Expand Up @@ -301,7 +302,7 @@ pub(crate) fn codegen_intrinsic_call<'tcx>(
fn codegen_float_intrinsic_call<'tcx>(
fx: &mut FunctionCx<'_, '_, 'tcx>,
intrinsic: Symbol,
args: &[mir::Operand<'tcx>],
args: &[Spanned<mir::Operand<'tcx>>],
ret: CPlace<'tcx>,
) -> bool {
let (name, arg_count, ty, clif_ty) = match intrinsic {
Expand Down Expand Up @@ -353,18 +354,21 @@ fn codegen_float_intrinsic_call<'tcx>(
let (a, b, c);
let args = match args {
[x] => {
a = [codegen_operand(fx, x).load_scalar(fx)];
a = [codegen_operand(fx, &x.node).load_scalar(fx)];
&a as &[_]
}
[x, y] => {
b = [codegen_operand(fx, x).load_scalar(fx), codegen_operand(fx, y).load_scalar(fx)];
b = [
codegen_operand(fx, &x.node).load_scalar(fx),
codegen_operand(fx, &y.node).load_scalar(fx),
];
&b
}
[x, y, z] => {
c = [
codegen_operand(fx, x).load_scalar(fx),
codegen_operand(fx, y).load_scalar(fx),
codegen_operand(fx, z).load_scalar(fx),
codegen_operand(fx, &x.node).load_scalar(fx),
codegen_operand(fx, &y.node).load_scalar(fx),
codegen_operand(fx, &z.node).load_scalar(fx),
];
&c
}
Expand Down Expand Up @@ -422,7 +426,7 @@ fn codegen_regular_intrinsic_call<'tcx>(
instance: Instance<'tcx>,
intrinsic: Symbol,
generic_args: GenericArgsRef<'tcx>,
args: &[mir::Operand<'tcx>],
args: &[Spanned<mir::Operand<'tcx>>],
ret: CPlace<'tcx>,
destination: Option<BasicBlock>,
source_info: mir::SourceInfo,
Expand Down

0 comments on commit 7d98bab

Please sign in to comment.