Skip to content
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.

Sign up for GitHub

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

large_assignments: Lint on specific large args passed to functions #116520

Merged
merged 3 commits into from
Jan 16, 2024
Merged

large_assignments: Lint on specific large args passed to functions #116520

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
924ea05
Implement TypeVisitable and TypeFoldable for Spanned
Enselic Jan 12, 2024
16ba56c
compiler: Lower fn call arg spans down to MIR
Enselic Jan 12, 2024
8f440f0
large_assignments: Lint on specific large args passed to functions
Enselic Oct 7, 2023
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
8 changes: 6 additions & 2 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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);
Enselic marked this conversation as resolved.
Show resolved Hide resolved
}
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_cranelift/src/intrinsics/llvm_aarch64.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
Loading
Loading