From 82777a94adc540d479e9536a344b47afdb86612b Mon Sep 17 00:00:00 2001 From: WANG Xuerui Date: Mon, 5 Feb 2024 13:18:32 +0800 Subject: [PATCH 1/8] target: default to the medium code model on LoongArch targets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Rust LoongArch targets have been using the default LLVM code model so far, which is "small" in LLVM-speak and "normal" in LoongArch-speak. As described in the "Code Model" section of LoongArch ELF psABI spec v20231219 [1], one can only make function calls as far as ±128MiB with the "normal" code model; this is insufficient for very large software containing Rust components that needs to be linked into the big text section, such as Chromium. Because: * we do not want to ask users to recompile std if they are to build such software, * objects compiled with larger code models can be linked with those with smaller code models without problems, and * the "medium" code model is comparable to the "small"/"normal" one performance-wise (same data access pattern; each function call becomes 2-insn long and indirect, but this may be relaxed back into the direct 1-insn form in a future LLVM version), but is able to perform function calls within ±128GiB, it is better to just switch the targets to the "medium" code model, which is also "medium" in LLVM-speak. [1]: https://github.com/loongson/la-abi-specs/blob/v2.30/laelf.adoc#code-models Co-authored-by: WANG Rui --- .../src/spec/targets/loongarch64_unknown_linux_gnu.rs | 3 ++- .../src/spec/targets/loongarch64_unknown_linux_musl.rs | 3 ++- .../rustc_target/src/spec/targets/loongarch64_unknown_none.rs | 2 +- .../src/spec/targets/loongarch64_unknown_none_softfloat.rs | 2 +- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_target/src/spec/targets/loongarch64_unknown_linux_gnu.rs b/compiler/rustc_target/src/spec/targets/loongarch64_unknown_linux_gnu.rs index d33c7af92c656..0bcbf655bd89e 100644 --- a/compiler/rustc_target/src/spec/targets/loongarch64_unknown_linux_gnu.rs +++ b/compiler/rustc_target/src/spec/targets/loongarch64_unknown_linux_gnu.rs @@ -1,4 +1,4 @@ -use crate::spec::{base, Target, TargetOptions}; +use crate::spec::{base, CodeModel, Target, TargetOptions}; pub(crate) fn target() -> Target { Target { @@ -13,6 +13,7 @@ pub(crate) fn target() -> Target { data_layout: "e-m:e-p:64:64-i64:64-i128:128-n32:64-S128".into(), arch: "loongarch64".into(), options: TargetOptions { + code_model: Some(CodeModel::Medium), cpu: "generic".into(), features: "+f,+d".into(), llvm_abiname: "lp64d".into(), diff --git a/compiler/rustc_target/src/spec/targets/loongarch64_unknown_linux_musl.rs b/compiler/rustc_target/src/spec/targets/loongarch64_unknown_linux_musl.rs index 5540e71ad4f31..223d979a06fbf 100644 --- a/compiler/rustc_target/src/spec/targets/loongarch64_unknown_linux_musl.rs +++ b/compiler/rustc_target/src/spec/targets/loongarch64_unknown_linux_musl.rs @@ -1,4 +1,4 @@ -use crate::spec::{base, Target, TargetOptions}; +use crate::spec::{base, CodeModel, Target, TargetOptions}; pub(crate) fn target() -> Target { Target { @@ -13,6 +13,7 @@ pub(crate) fn target() -> Target { data_layout: "e-m:e-p:64:64-i64:64-i128:128-n32:64-S128".into(), arch: "loongarch64".into(), options: TargetOptions { + code_model: Some(CodeModel::Medium), cpu: "generic".into(), features: "+f,+d".into(), llvm_abiname: "lp64d".into(), diff --git a/compiler/rustc_target/src/spec/targets/loongarch64_unknown_none.rs b/compiler/rustc_target/src/spec/targets/loongarch64_unknown_none.rs index 5628519026831..db527c8b63697 100644 --- a/compiler/rustc_target/src/spec/targets/loongarch64_unknown_none.rs +++ b/compiler/rustc_target/src/spec/targets/loongarch64_unknown_none.rs @@ -23,7 +23,7 @@ pub(crate) fn target() -> Target { max_atomic_width: Some(64), relocation_model: RelocModel::Static, panic_strategy: PanicStrategy::Abort, - code_model: Some(CodeModel::Small), + code_model: Some(CodeModel::Medium), ..Default::default() }, } diff --git a/compiler/rustc_target/src/spec/targets/loongarch64_unknown_none_softfloat.rs b/compiler/rustc_target/src/spec/targets/loongarch64_unknown_none_softfloat.rs index 7e57715ce7a73..221ca02fe3e3b 100644 --- a/compiler/rustc_target/src/spec/targets/loongarch64_unknown_none_softfloat.rs +++ b/compiler/rustc_target/src/spec/targets/loongarch64_unknown_none_softfloat.rs @@ -24,7 +24,7 @@ pub(crate) fn target() -> Target { max_atomic_width: Some(64), relocation_model: RelocModel::Static, panic_strategy: PanicStrategy::Abort, - code_model: Some(CodeModel::Small), + code_model: Some(CodeModel::Medium), ..Default::default() }, } From 55c9f9626575f93d0f7cb8b51523a8891a065dc4 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 13 Sep 2024 09:49:28 +1000 Subject: [PATCH 2/8] Remove unnecessary `Clone`/`Copy` derives from analyses. No analysis needs `Copy`, and `MaybeBorrowedLocals` is the only analysis that needs `Clone`. In `locals_live_across_suspend_points` it gets cloned so it can be used within a `MaybeRequiresStorage`. --- compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs | 2 +- compiler/rustc_mir_dataflow/src/impls/liveness.rs | 1 - compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs | 2 -- 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs b/compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs index e8e78fb8a89ee..8b082ef2667a3 100644 --- a/compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs +++ b/compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs @@ -11,7 +11,7 @@ use crate::{AnalysisDomain, GenKill, GenKillAnalysis}; /// At present, this is used as a very limited form of alias analysis. For example, /// `MaybeBorrowedLocals` is used to compute which locals are live during a yield expression for /// immovable coroutines. -#[derive(Clone, Copy)] +#[derive(Clone)] pub struct MaybeBorrowedLocals; impl MaybeBorrowedLocals { diff --git a/compiler/rustc_mir_dataflow/src/impls/liveness.rs b/compiler/rustc_mir_dataflow/src/impls/liveness.rs index 24a4b32ceb7c4..1559c131a3783 100644 --- a/compiler/rustc_mir_dataflow/src/impls/liveness.rs +++ b/compiler/rustc_mir_dataflow/src/impls/liveness.rs @@ -217,7 +217,6 @@ impl DefUse { /// This is basically written for dead store elimination and nothing else. /// /// All of the caveats of `MaybeLiveLocals` apply. -#[derive(Clone, Copy)] pub struct MaybeTransitiveLiveLocals<'a> { always_live: &'a BitSet, } diff --git a/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs b/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs index 9f2f0187698a8..6bf54c8db410e 100644 --- a/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs +++ b/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs @@ -7,7 +7,6 @@ use rustc_middle::mir::*; use super::MaybeBorrowedLocals; use crate::{GenKill, ResultsCursor}; -#[derive(Clone)] pub struct MaybeStorageLive<'a> { always_live_locals: Cow<'a, BitSet>, } @@ -80,7 +79,6 @@ impl<'tcx, 'a> crate::GenKillAnalysis<'tcx> for MaybeStorageLive<'a> { } } -#[derive(Clone)] pub struct MaybeStorageDead<'a> { always_live_locals: Cow<'a, BitSet>, } From bb943f93ff093d70012950f4617de8cfe74e5a36 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 13 Sep 2024 16:27:24 +1000 Subject: [PATCH 3/8] Rename `FlowState` as `Domain`. Because that's what it is; no point having a different name for it. --- compiler/rustc_borrowck/src/dataflow.rs | 23 ++- compiler/rustc_borrowck/src/lib.rs | 171 +++++++++--------- .../src/framework/direction.rs | 20 +- .../src/framework/engine.rs | 4 +- .../src/framework/graphviz.rs | 14 +- .../src/framework/visitor.rs | 55 +++--- compiler/rustc_mir_dataflow/src/points.rs | 8 +- compiler/rustc_mir_dataflow/src/rustc_peek.rs | 10 +- compiler/rustc_mir_transform/src/coroutine.rs | 10 +- .../src/dataflow_const_prop.rs | 8 +- 10 files changed, 160 insertions(+), 163 deletions(-) diff --git a/compiler/rustc_borrowck/src/dataflow.rs b/compiler/rustc_borrowck/src/dataflow.rs index 39994ad784a95..6725920746bcf 100644 --- a/compiler/rustc_borrowck/src/dataflow.rs +++ b/compiler/rustc_borrowck/src/dataflow.rs @@ -9,7 +9,7 @@ use rustc_middle::mir::{ use rustc_middle::ty::{RegionVid, TyCtxt}; use rustc_mir_dataflow::fmt::DebugWithContext; use rustc_mir_dataflow::impls::{EverInitializedPlaces, MaybeUninitializedPlaces}; -use rustc_mir_dataflow::{Analysis, AnalysisDomain, GenKill, Results, ResultsVisitable}; +use rustc_mir_dataflow::{Analysis, AnalysisDomain, Forward, GenKill, Results, ResultsVisitable}; use tracing::debug; use crate::{places_conflict, BorrowSet, PlaceConflictBias, PlaceExt, RegionInferenceContext}; @@ -23,26 +23,25 @@ pub(crate) struct BorrowckResults<'a, 'tcx> { /// The transient state of the dataflow analyses used by the borrow checker. #[derive(Debug)] -pub(crate) struct BorrowckFlowState<'a, 'tcx> { +pub(crate) struct BorrowckDomain<'a, 'tcx> { pub(crate) borrows: as AnalysisDomain<'tcx>>::Domain, pub(crate) uninits: as AnalysisDomain<'tcx>>::Domain, pub(crate) ever_inits: as AnalysisDomain<'tcx>>::Domain, } impl<'a, 'tcx> ResultsVisitable<'tcx> for BorrowckResults<'a, 'tcx> { - // All three analyses are forward, but we have to use just one here. - type Direction = as AnalysisDomain<'tcx>>::Direction; - type FlowState = BorrowckFlowState<'a, 'tcx>; + type Direction = Forward; + type Domain = BorrowckDomain<'a, 'tcx>; - fn new_flow_state(&self, body: &mir::Body<'tcx>) -> Self::FlowState { - BorrowckFlowState { + fn bottom_value(&self, body: &mir::Body<'tcx>) -> Self::Domain { + BorrowckDomain { borrows: self.borrows.analysis.bottom_value(body), uninits: self.uninits.analysis.bottom_value(body), ever_inits: self.ever_inits.analysis.bottom_value(body), } } - fn reset_to_block_entry(&self, state: &mut Self::FlowState, block: BasicBlock) { + fn reset_to_block_entry(&self, state: &mut Self::Domain, block: BasicBlock) { state.borrows.clone_from(self.borrows.entry_set_for_block(block)); state.uninits.clone_from(self.uninits.entry_set_for_block(block)); state.ever_inits.clone_from(self.ever_inits.entry_set_for_block(block)); @@ -50,7 +49,7 @@ impl<'a, 'tcx> ResultsVisitable<'tcx> for BorrowckResults<'a, 'tcx> { fn reconstruct_before_statement_effect( &mut self, - state: &mut Self::FlowState, + state: &mut Self::Domain, stmt: &mir::Statement<'tcx>, loc: Location, ) { @@ -61,7 +60,7 @@ impl<'a, 'tcx> ResultsVisitable<'tcx> for BorrowckResults<'a, 'tcx> { fn reconstruct_statement_effect( &mut self, - state: &mut Self::FlowState, + state: &mut Self::Domain, stmt: &mir::Statement<'tcx>, loc: Location, ) { @@ -72,7 +71,7 @@ impl<'a, 'tcx> ResultsVisitable<'tcx> for BorrowckResults<'a, 'tcx> { fn reconstruct_before_terminator_effect( &mut self, - state: &mut Self::FlowState, + state: &mut Self::Domain, term: &mir::Terminator<'tcx>, loc: Location, ) { @@ -83,7 +82,7 @@ impl<'a, 'tcx> ResultsVisitable<'tcx> for BorrowckResults<'a, 'tcx> { fn reconstruct_terminator_effect( &mut self, - state: &mut Self::FlowState, + state: &mut Self::Domain, term: &mir::Terminator<'tcx>, loc: Location, ) { diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index d5f297a591382..17153546d2dc9 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -83,7 +83,7 @@ mod util; pub mod consumers; use borrow_set::{BorrowData, BorrowSet}; -use dataflow::{BorrowIndex, BorrowckFlowState as Flows, BorrowckResults, Borrows}; +use dataflow::{BorrowIndex, BorrowckDomain, BorrowckResults, Borrows}; use nll::PoloniusOutput; use place_ext::PlaceExt; use places_conflict::{places_conflict, PlaceConflictBias}; @@ -602,25 +602,25 @@ struct MirBorrowckCtxt<'a, 'infcx, 'tcx> { impl<'a, 'tcx, R> rustc_mir_dataflow::ResultsVisitor<'a, 'tcx, R> for MirBorrowckCtxt<'a, '_, 'tcx> { - type FlowState = Flows<'a, 'tcx>; + type Domain = BorrowckDomain<'a, 'tcx>; fn visit_statement_before_primary_effect( &mut self, _results: &mut R, - flow_state: &Flows<'a, 'tcx>, + state: &BorrowckDomain<'a, 'tcx>, stmt: &'a Statement<'tcx>, location: Location, ) { - debug!("MirBorrowckCtxt::process_statement({:?}, {:?}): {:?}", location, stmt, flow_state); + debug!("MirBorrowckCtxt::process_statement({:?}, {:?}): {:?}", location, stmt, state); let span = stmt.source_info.span; - self.check_activations(location, span, flow_state); + self.check_activations(location, span, state); match &stmt.kind { StatementKind::Assign(box (lhs, rhs)) => { - self.consume_rvalue(location, (rhs, span), flow_state); + self.consume_rvalue(location, (rhs, span), state); - self.mutate_place(location, (*lhs, span), Shallow(None), flow_state); + self.mutate_place(location, (*lhs, span), Shallow(None), state); } StatementKind::FakeRead(box (_, place)) => { // Read for match doesn't access any memory and is used to @@ -637,11 +637,11 @@ impl<'a, 'tcx, R> rustc_mir_dataflow::ResultsVisitor<'a, 'tcx, R> location, InitializationRequiringAction::Use, (place.as_ref(), span), - flow_state, + state, ); } StatementKind::Intrinsic(box kind) => match kind { - NonDivergingIntrinsic::Assume(op) => self.consume_operand(location, (op, span), flow_state), + NonDivergingIntrinsic::Assume(op) => self.consume_operand(location, (op, span), state), NonDivergingIntrinsic::CopyNonOverlapping(..) => span_bug!( span, "Unexpected CopyNonOverlapping, should only appear after lower_intrinsics", @@ -662,7 +662,7 @@ impl<'a, 'tcx, R> rustc_mir_dataflow::ResultsVisitor<'a, 'tcx, R> (Place::from(*local), span), (Shallow(None), Write(WriteKind::StorageDeadOrDrop)), LocalMutationIsAllowed::Yes, - flow_state, + state, ); } StatementKind::Nop @@ -677,18 +677,18 @@ impl<'a, 'tcx, R> rustc_mir_dataflow::ResultsVisitor<'a, 'tcx, R> fn visit_terminator_before_primary_effect( &mut self, _results: &mut R, - flow_state: &Flows<'a, 'tcx>, + state: &BorrowckDomain<'a, 'tcx>, term: &'a Terminator<'tcx>, loc: Location, ) { - debug!("MirBorrowckCtxt::process_terminator({:?}, {:?}): {:?}", loc, term, flow_state); + debug!("MirBorrowckCtxt::process_terminator({:?}, {:?}): {:?}", loc, term, state); let span = term.source_info.span; - self.check_activations(loc, span, flow_state); + self.check_activations(loc, span, state); match &term.kind { TerminatorKind::SwitchInt { discr, targets: _ } => { - self.consume_operand(loc, (discr, span), flow_state); + self.consume_operand(loc, (discr, span), state); } TerminatorKind::Drop { place, target: _, unwind: _, replace } => { debug!( @@ -704,7 +704,7 @@ impl<'a, 'tcx, R> rustc_mir_dataflow::ResultsVisitor<'a, 'tcx, R> (*place, span), (AccessDepth::Drop, Write(write_kind)), LocalMutationIsAllowed::Yes, - flow_state, + state, ); } TerminatorKind::Call { @@ -716,29 +716,29 @@ impl<'a, 'tcx, R> rustc_mir_dataflow::ResultsVisitor<'a, 'tcx, R> call_source: _, fn_span: _, } => { - self.consume_operand(loc, (func, span), flow_state); + self.consume_operand(loc, (func, span), state); for arg in args { - self.consume_operand(loc, (&arg.node, arg.span), flow_state); + self.consume_operand(loc, (&arg.node, arg.span), state); } - self.mutate_place(loc, (*destination, span), Deep, flow_state); + self.mutate_place(loc, (*destination, span), Deep, state); } TerminatorKind::TailCall { func, args, fn_span: _ } => { - self.consume_operand(loc, (func, span), flow_state); + self.consume_operand(loc, (func, span), state); for arg in args { - self.consume_operand(loc, (&arg.node, arg.span), flow_state); + self.consume_operand(loc, (&arg.node, arg.span), state); } } TerminatorKind::Assert { cond, expected: _, msg, target: _, unwind: _ } => { - self.consume_operand(loc, (cond, span), flow_state); + self.consume_operand(loc, (cond, span), state); if let AssertKind::BoundsCheck { len, index } = &**msg { - self.consume_operand(loc, (len, span), flow_state); - self.consume_operand(loc, (index, span), flow_state); + self.consume_operand(loc, (len, span), state); + self.consume_operand(loc, (index, span), state); } } TerminatorKind::Yield { value, resume: _, resume_arg, drop: _ } => { - self.consume_operand(loc, (value, span), flow_state); - self.mutate_place(loc, (*resume_arg, span), Deep, flow_state); + self.consume_operand(loc, (value, span), state); + self.mutate_place(loc, (*resume_arg, span), Deep, state); } TerminatorKind::InlineAsm { @@ -752,22 +752,17 @@ impl<'a, 'tcx, R> rustc_mir_dataflow::ResultsVisitor<'a, 'tcx, R> for op in operands { match op { InlineAsmOperand::In { reg: _, value } => { - self.consume_operand(loc, (value, span), flow_state); + self.consume_operand(loc, (value, span), state); } InlineAsmOperand::Out { reg: _, late: _, place, .. } => { if let Some(place) = place { - self.mutate_place(loc, (*place, span), Shallow(None), flow_state); + self.mutate_place(loc, (*place, span), Shallow(None), state); } } InlineAsmOperand::InOut { reg: _, late: _, in_value, out_place } => { - self.consume_operand(loc, (in_value, span), flow_state); + self.consume_operand(loc, (in_value, span), state); if let &Some(out_place) = out_place { - self.mutate_place( - loc, - (out_place, span), - Shallow(None), - flow_state, - ); + self.mutate_place(loc, (out_place, span), Shallow(None), state); } } InlineAsmOperand::Const { value: _ } @@ -794,7 +789,7 @@ impl<'a, 'tcx, R> rustc_mir_dataflow::ResultsVisitor<'a, 'tcx, R> fn visit_terminator_after_primary_effect( &mut self, _results: &mut R, - flow_state: &Flows<'a, 'tcx>, + state: &BorrowckDomain<'a, 'tcx>, term: &'a Terminator<'tcx>, loc: Location, ) { @@ -805,7 +800,7 @@ impl<'a, 'tcx, R> rustc_mir_dataflow::ResultsVisitor<'a, 'tcx, R> if self.movable_coroutine { // Look for any active borrows to locals let borrow_set = self.borrow_set.clone(); - for i in flow_state.borrows.iter() { + for i in state.borrows.iter() { let borrow = &borrow_set[i]; self.check_for_local_borrow(borrow, span); } @@ -821,7 +816,7 @@ impl<'a, 'tcx, R> rustc_mir_dataflow::ResultsVisitor<'a, 'tcx, R> // StorageDead, but we don't always emit those (notably on unwind paths), // so this "extra check" serves as a kind of backup. let borrow_set = self.borrow_set.clone(); - for i in flow_state.borrows.iter() { + for i in state.borrows.iter() { let borrow = &borrow_set[i]; self.check_for_invalidation_at_exit(loc, borrow, span); } @@ -989,7 +984,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { place_span: (Place<'tcx>, Span), kind: (AccessDepth, ReadOrWrite), is_local_mutation_allowed: LocalMutationIsAllowed, - flow_state: &Flows<'a, 'tcx>, + state: &BorrowckDomain<'a, 'tcx>, ) { let (sd, rw) = kind; @@ -1020,11 +1015,10 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { place_span, rw, is_local_mutation_allowed, - flow_state, + state, location, ); - let conflict_error = - self.check_access_for_conflict(location, place_span, sd, rw, flow_state); + let conflict_error = self.check_access_for_conflict(location, place_span, sd, rw, state); if conflict_error || mutability_error { debug!("access_place: logging error place_span=`{:?}` kind=`{:?}`", place_span, kind); @@ -1032,14 +1026,14 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { } } - #[instrument(level = "debug", skip(self, flow_state))] + #[instrument(level = "debug", skip(self, state))] fn check_access_for_conflict( &mut self, location: Location, place_span: (Place<'tcx>, Span), sd: AccessDepth, rw: ReadOrWrite, - flow_state: &Flows<'a, 'tcx>, + state: &BorrowckDomain<'a, 'tcx>, ) -> bool { let mut error_reported = false; let borrow_set = Rc::clone(&self.borrow_set); @@ -1054,7 +1048,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { } &polonius_output } else { - &flow_state.borrows + &state.borrows }; each_borrow_involving_path( @@ -1180,17 +1174,17 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { location: Location, place_span: (Place<'tcx>, Span), kind: AccessDepth, - flow_state: &Flows<'a, 'tcx>, + state: &BorrowckDomain<'a, 'tcx>, ) { // Write of P[i] or *P requires P init'd. - self.check_if_assigned_path_is_moved(location, place_span, flow_state); + self.check_if_assigned_path_is_moved(location, place_span, state); self.access_place( location, place_span, (kind, Write(WriteKind::Mutate)), LocalMutationIsAllowed::No, - flow_state, + state, ); } @@ -1198,7 +1192,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { &mut self, location: Location, (rvalue, span): (&'a Rvalue<'tcx>, Span), - flow_state: &Flows<'a, 'tcx>, + state: &BorrowckDomain<'a, 'tcx>, ) { match rvalue { &Rvalue::Ref(_ /*rgn*/, bk, place) => { @@ -1224,7 +1218,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { (place, span), access_kind, LocalMutationIsAllowed::No, - flow_state, + state, ); let action = if bk == BorrowKind::Fake(FakeBorrowKind::Shallow) { @@ -1237,7 +1231,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { location, action, (place.as_ref(), span), - flow_state, + state, ); } @@ -1257,14 +1251,14 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { (place, span), access_kind, LocalMutationIsAllowed::No, - flow_state, + state, ); self.check_if_path_or_subpath_is_moved( location, InitializationRequiringAction::Borrow, (place.as_ref(), span), - flow_state, + state, ); } @@ -1275,7 +1269,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { | Rvalue::UnaryOp(_ /*un_op*/, operand) | Rvalue::Cast(_ /*cast_kind*/, operand, _ /*ty*/) | Rvalue::ShallowInitBox(operand, _ /*ty*/) => { - self.consume_operand(location, (operand, span), flow_state) + self.consume_operand(location, (operand, span), state) } &Rvalue::CopyForDeref(place) => { @@ -1284,7 +1278,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { (place, span), (Deep, Read(ReadKind::Copy)), LocalMutationIsAllowed::No, - flow_state, + state, ); // Finally, check if path was already moved. @@ -1292,7 +1286,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { location, InitializationRequiringAction::Use, (place.as_ref(), span), - flow_state, + state, ); } @@ -1307,19 +1301,19 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { (place, span), (Shallow(af), Read(ReadKind::Copy)), LocalMutationIsAllowed::No, - flow_state, + state, ); self.check_if_path_or_subpath_is_moved( location, InitializationRequiringAction::Use, (place.as_ref(), span), - flow_state, + state, ); } Rvalue::BinaryOp(_bin_op, box (operand1, operand2)) => { - self.consume_operand(location, (operand1, span), flow_state); - self.consume_operand(location, (operand2, span), flow_state); + self.consume_operand(location, (operand1, span), state); + self.consume_operand(location, (operand2, span), state); } Rvalue::NullaryOp(_op, _ty) => { @@ -1349,7 +1343,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { } for operand in operands { - self.consume_operand(location, (operand, span), flow_state); + self.consume_operand(location, (operand, span), state); } } } @@ -1456,7 +1450,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { &mut self, location: Location, (operand, span): (&'a Operand<'tcx>, Span), - flow_state: &Flows<'a, 'tcx>, + state: &BorrowckDomain<'a, 'tcx>, ) { match *operand { Operand::Copy(place) => { @@ -1467,7 +1461,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { (place, span), (Deep, Read(ReadKind::Copy)), LocalMutationIsAllowed::No, - flow_state, + state, ); // Finally, check if path was already moved. @@ -1475,7 +1469,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { location, InitializationRequiringAction::Use, (place.as_ref(), span), - flow_state, + state, ); } Operand::Move(place) => { @@ -1488,7 +1482,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { (place, span), (Deep, Write(WriteKind::Move)), LocalMutationIsAllowed::Yes, - flow_state, + state, ); // Finally, check if path was already moved. @@ -1496,7 +1490,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { location, InitializationRequiringAction::Use, (place.as_ref(), span), - flow_state, + state, ); } Operand::Constant(_) => {} @@ -1576,7 +1570,12 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { } } - fn check_activations(&mut self, location: Location, span: Span, flow_state: &Flows<'a, 'tcx>) { + fn check_activations( + &mut self, + location: Location, + span: Span, + state: &BorrowckDomain<'a, 'tcx>, + ) { // Two-phase borrow support: For each activation that is newly // generated at this statement, check if it interferes with // another borrow. @@ -1595,7 +1594,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { (borrow.borrowed_place, span), (Deep, Activation(WriteKind::MutableBorrow(borrow.kind), borrow_index)), LocalMutationIsAllowed::No, - flow_state, + state, ); // We do not need to call `check_if_path_or_subpath_is_moved` // again, as we already called it when we made the @@ -1739,9 +1738,9 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { location: Location, desired_action: InitializationRequiringAction, place_span: (PlaceRef<'tcx>, Span), - flow_state: &Flows<'a, 'tcx>, + state: &BorrowckDomain<'a, 'tcx>, ) { - let maybe_uninits = &flow_state.uninits; + let maybe_uninits = &state.uninits; // Bad scenarios: // @@ -1844,9 +1843,9 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { location: Location, desired_action: InitializationRequiringAction, place_span: (PlaceRef<'tcx>, Span), - flow_state: &Flows<'a, 'tcx>, + state: &BorrowckDomain<'a, 'tcx>, ) { - let maybe_uninits = &flow_state.uninits; + let maybe_uninits = &state.uninits; // Bad scenarios: // @@ -1863,7 +1862,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { // must have been initialized for the use to be sound. // 6. Move of `a.b.c` then reinit of `a.b.c.d`, use of `a.b.c.d` - self.check_if_full_path_is_moved(location, desired_action, place_span, flow_state); + self.check_if_full_path_is_moved(location, desired_action, place_span, state); if let Some((place_base, ProjectionElem::Subslice { from, to, from_end: false })) = place_span.0.last_projection() @@ -1943,7 +1942,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { &mut self, location: Location, (place, span): (Place<'tcx>, Span), - flow_state: &Flows<'a, 'tcx>, + state: &BorrowckDomain<'a, 'tcx>, ) { debug!("check_if_assigned_path_is_moved place: {:?}", place); @@ -1965,7 +1964,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { ProjectionElem::Deref => { self.check_if_full_path_is_moved( location, InitializationRequiringAction::Use, - (place_base, span), flow_state); + (place_base, span), state); // (base initialized; no need to // recur further) break; @@ -1985,7 +1984,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { ty::Adt(def, _) if def.has_dtor(tcx) => { self.check_if_path_or_subpath_is_moved( location, InitializationRequiringAction::Assignment, - (place_base, span), flow_state); + (place_base, span), state); // (base initialized; no need to // recur further) @@ -1995,7 +1994,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { // Once `let s; s.x = V; read(s.x);`, // is allowed, remove this match arm. ty::Adt(..) | ty::Tuple(..) => { - check_parent_of_field(self, location, place_base, span, flow_state); + check_parent_of_field(self, location, place_base, span, state); } _ => {} @@ -2009,7 +2008,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { location: Location, base: PlaceRef<'tcx>, span: Span, - flow_state: &Flows<'a, 'tcx>, + state: &BorrowckDomain<'a, 'tcx>, ) { // rust-lang/rust#21232: Until Rust allows reads from the // initialized parts of partially initialized structs, we @@ -2042,7 +2041,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { // Shallow so that we'll stop at any dereference; we'll // report errors about issues with such bases elsewhere. - let maybe_uninits = &flow_state.uninits; + let maybe_uninits = &state.uninits; // Find the shortest uninitialized prefix you can reach // without going over a Deref. @@ -2100,7 +2099,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { (place, span): (Place<'tcx>, Span), kind: ReadOrWrite, is_local_mutation_allowed: LocalMutationIsAllowed, - flow_state: &Flows<'a, 'tcx>, + state: &BorrowckDomain<'a, 'tcx>, location: Location, ) -> bool { debug!( @@ -2124,7 +2123,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { }; match self.is_mutable(place.as_ref(), is_local_mutation_allowed) { Ok(root_place) => { - self.add_used_mut(root_place, flow_state); + self.add_used_mut(root_place, state); return false; } Err(place_err) => { @@ -2136,7 +2135,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { Reservation(WriteKind::Mutate) | Write(WriteKind::Mutate) => { match self.is_mutable(place.as_ref(), is_local_mutation_allowed) { Ok(root_place) => { - self.add_used_mut(root_place, flow_state); + self.add_used_mut(root_place, state); return false; } Err(place_err) => { @@ -2194,7 +2193,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { // partial initialization, do not complain about mutability // errors except for actual mutation (as opposed to an attempt // to do a partial initialization). - let previously_initialized = self.is_local_ever_initialized(place.local, flow_state); + let previously_initialized = self.is_local_ever_initialized(place.local, state); // at this point, we have set up the error reporting state. if let Some(init_index) = previously_initialized { @@ -2216,22 +2215,22 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { fn is_local_ever_initialized( &self, local: Local, - flow_state: &Flows<'a, 'tcx>, + state: &BorrowckDomain<'a, 'tcx>, ) -> Option { let mpi = self.move_data.rev_lookup.find_local(local)?; let ii = &self.move_data.init_path_map[mpi]; - ii.into_iter().find(|&&index| flow_state.ever_inits.contains(index)).copied() + ii.into_iter().find(|&&index| state.ever_inits.contains(index)).copied() } /// Adds the place into the used mutable variables set - fn add_used_mut(&mut self, root_place: RootPlace<'tcx>, flow_state: &Flows<'a, 'tcx>) { + fn add_used_mut(&mut self, root_place: RootPlace<'tcx>, state: &BorrowckDomain<'a, 'tcx>) { match root_place { RootPlace { place_local: local, place_projection: [], is_local_mutation_allowed } => { // If the local may have been initialized, and it is now currently being // mutated, then it is justified to be annotated with the `mut` // keyword, since the mutation may be a possible reassignment. if is_local_mutation_allowed != LocalMutationIsAllowed::Yes - && self.is_local_ever_initialized(local, flow_state).is_some() + && self.is_local_ever_initialized(local, state).is_some() { self.used_mut.insert(local); } diff --git a/compiler/rustc_mir_dataflow/src/framework/direction.rs b/compiler/rustc_mir_dataflow/src/framework/direction.rs index ba4a7d7651141..88a9a78f8ad62 100644 --- a/compiler/rustc_mir_dataflow/src/framework/direction.rs +++ b/compiler/rustc_mir_dataflow/src/framework/direction.rs @@ -42,14 +42,14 @@ pub trait Direction { ) where A: GenKillAnalysis<'tcx>; - fn visit_results_in_block<'mir, 'tcx, F, R>( - state: &mut F, + fn visit_results_in_block<'mir, 'tcx, D, R>( + state: &mut D, block: BasicBlock, block_data: &'mir mir::BasicBlockData<'tcx>, results: &mut R, - vis: &mut impl ResultsVisitor<'mir, 'tcx, R, FlowState = F>, + vis: &mut impl ResultsVisitor<'mir, 'tcx, R, Domain = D>, ) where - R: ResultsVisitable<'tcx, FlowState = F>; + R: ResultsVisitable<'tcx, Domain = D>; fn join_state_into_successors_of<'tcx, A>( analysis: &mut A, @@ -186,14 +186,14 @@ impl Direction for Backward { analysis.apply_statement_effect(state, statement, location); } - fn visit_results_in_block<'mir, 'tcx, F, R>( - state: &mut F, + fn visit_results_in_block<'mir, 'tcx, D, R>( + state: &mut D, block: BasicBlock, block_data: &'mir mir::BasicBlockData<'tcx>, results: &mut R, - vis: &mut impl ResultsVisitor<'mir, 'tcx, R, FlowState = F>, + vis: &mut impl ResultsVisitor<'mir, 'tcx, R, Domain = D>, ) where - R: ResultsVisitable<'tcx, FlowState = F>, + R: ResultsVisitable<'tcx, Domain = D>, { results.reset_to_block_entry(state, block); @@ -444,9 +444,9 @@ impl Direction for Forward { block: BasicBlock, block_data: &'mir mir::BasicBlockData<'tcx>, results: &mut R, - vis: &mut impl ResultsVisitor<'mir, 'tcx, R, FlowState = F>, + vis: &mut impl ResultsVisitor<'mir, 'tcx, R, Domain = F>, ) where - R: ResultsVisitable<'tcx, FlowState = F>, + R: ResultsVisitable<'tcx, Domain = F>, { results.reset_to_block_entry(state, block); diff --git a/compiler/rustc_mir_dataflow/src/framework/engine.rs b/compiler/rustc_mir_dataflow/src/framework/engine.rs index 0bab03b027161..54206501d9f6a 100644 --- a/compiler/rustc_mir_dataflow/src/framework/engine.rs +++ b/compiler/rustc_mir_dataflow/src/framework/engine.rs @@ -57,7 +57,7 @@ where &mut self, body: &'mir mir::Body<'tcx>, blocks: impl IntoIterator, - vis: &mut impl ResultsVisitor<'mir, 'tcx, Self, FlowState = A::Domain>, + vis: &mut impl ResultsVisitor<'mir, 'tcx, Self, Domain = A::Domain>, ) { visit_results(body, blocks, self, vis) } @@ -65,7 +65,7 @@ where pub fn visit_reachable_with<'mir>( &mut self, body: &'mir mir::Body<'tcx>, - vis: &mut impl ResultsVisitor<'mir, 'tcx, Self, FlowState = A::Domain>, + vis: &mut impl ResultsVisitor<'mir, 'tcx, Self, Domain = A::Domain>, ) { let blocks = mir::traversal::reachable(body); visit_results(body, blocks.map(|(bb, _)| bb), self, vis) diff --git a/compiler/rustc_mir_dataflow/src/framework/graphviz.rs b/compiler/rustc_mir_dataflow/src/framework/graphviz.rs index 2e860e2d84121..e72dca2c83478 100644 --- a/compiler/rustc_mir_dataflow/src/framework/graphviz.rs +++ b/compiler/rustc_mir_dataflow/src/framework/graphviz.rs @@ -544,15 +544,15 @@ where A: Analysis<'tcx>, A::Domain: DebugWithContext, { - type FlowState = A::Domain; + type Domain = A::Domain; - fn visit_block_start(&mut self, state: &Self::FlowState) { + fn visit_block_start(&mut self, state: &Self::Domain) { if A::Direction::IS_FORWARD { self.prev_state.clone_from(state); } } - fn visit_block_end(&mut self, state: &Self::FlowState) { + fn visit_block_end(&mut self, state: &Self::Domain) { if A::Direction::IS_BACKWARD { self.prev_state.clone_from(state); } @@ -561,7 +561,7 @@ where fn visit_statement_before_primary_effect( &mut self, results: &mut Results<'tcx, A>, - state: &Self::FlowState, + state: &Self::Domain, _statement: &mir::Statement<'tcx>, _location: Location, ) { @@ -574,7 +574,7 @@ where fn visit_statement_after_primary_effect( &mut self, results: &mut Results<'tcx, A>, - state: &Self::FlowState, + state: &Self::Domain, _statement: &mir::Statement<'tcx>, _location: Location, ) { @@ -585,7 +585,7 @@ where fn visit_terminator_before_primary_effect( &mut self, results: &mut Results<'tcx, A>, - state: &Self::FlowState, + state: &Self::Domain, _terminator: &mir::Terminator<'tcx>, _location: Location, ) { @@ -598,7 +598,7 @@ where fn visit_terminator_after_primary_effect( &mut self, results: &mut Results<'tcx, A>, - state: &Self::FlowState, + state: &Self::Domain, _terminator: &mir::Terminator<'tcx>, _location: Location, ) { diff --git a/compiler/rustc_mir_dataflow/src/framework/visitor.rs b/compiler/rustc_mir_dataflow/src/framework/visitor.rs index 8b8a16bda99b1..3d6b008a6846d 100644 --- a/compiler/rustc_mir_dataflow/src/framework/visitor.rs +++ b/compiler/rustc_mir_dataflow/src/framework/visitor.rs @@ -4,15 +4,15 @@ use super::{Analysis, Direction, Results}; /// Calls the corresponding method in `ResultsVisitor` for every location in a `mir::Body` with the /// dataflow state at that location. -pub fn visit_results<'mir, 'tcx, F, R>( +pub fn visit_results<'mir, 'tcx, D, R>( body: &'mir mir::Body<'tcx>, blocks: impl IntoIterator, results: &mut R, - vis: &mut impl ResultsVisitor<'mir, 'tcx, R, FlowState = F>, + vis: &mut impl ResultsVisitor<'mir, 'tcx, R, Domain = D>, ) where - R: ResultsVisitable<'tcx, FlowState = F>, + R: ResultsVisitable<'tcx, Domain = D>, { - let mut state = results.new_flow_state(body); + let mut state = results.bottom_value(body); #[cfg(debug_assertions)] let reachable_blocks = mir::traversal::reachable_as_bitset(body); @@ -29,16 +29,16 @@ pub fn visit_results<'mir, 'tcx, F, R>( /// A visitor over the results of an `Analysis`. The type parameter `R` is the results type being /// visited. pub trait ResultsVisitor<'mir, 'tcx, R> { - type FlowState; + type Domain; - fn visit_block_start(&mut self, _state: &Self::FlowState) {} + fn visit_block_start(&mut self, _state: &Self::Domain) {} /// Called with the `before_statement_effect` of the given statement applied to `state` but not /// its `statement_effect`. fn visit_statement_before_primary_effect( &mut self, _results: &mut R, - _state: &Self::FlowState, + _state: &Self::Domain, _statement: &'mir mir::Statement<'tcx>, _location: Location, ) { @@ -49,7 +49,7 @@ pub trait ResultsVisitor<'mir, 'tcx, R> { fn visit_statement_after_primary_effect( &mut self, _results: &mut R, - _state: &Self::FlowState, + _state: &Self::Domain, _statement: &'mir mir::Statement<'tcx>, _location: Location, ) { @@ -60,7 +60,7 @@ pub trait ResultsVisitor<'mir, 'tcx, R> { fn visit_terminator_before_primary_effect( &mut self, _results: &mut R, - _state: &Self::FlowState, + _state: &Self::Domain, _terminator: &'mir mir::Terminator<'tcx>, _location: Location, ) { @@ -73,13 +73,13 @@ pub trait ResultsVisitor<'mir, 'tcx, R> { fn visit_terminator_after_primary_effect( &mut self, _results: &mut R, - _state: &Self::FlowState, + _state: &Self::Domain, _terminator: &'mir mir::Terminator<'tcx>, _location: Location, ) { } - fn visit_block_end(&mut self, _state: &Self::FlowState) {} + fn visit_block_end(&mut self, _state: &Self::Domain) {} } /// Things that can be visited by a `ResultsVisitor`. @@ -88,40 +88,40 @@ pub trait ResultsVisitor<'mir, 'tcx, R> { /// simultaneously. pub trait ResultsVisitable<'tcx> { type Direction: Direction; - type FlowState; + type Domain; - /// Creates an empty `FlowState` to hold the transient state for these dataflow results. + /// Creates an empty `Domain` to hold the transient state for these dataflow results. /// - /// The value of the newly created `FlowState` will be overwritten by `reset_to_block_entry` + /// The value of the newly created `Domain` will be overwritten by `reset_to_block_entry` /// before it can be observed by a `ResultsVisitor`. - fn new_flow_state(&self, body: &mir::Body<'tcx>) -> Self::FlowState; + fn bottom_value(&self, body: &mir::Body<'tcx>) -> Self::Domain; - fn reset_to_block_entry(&self, state: &mut Self::FlowState, block: BasicBlock); + fn reset_to_block_entry(&self, state: &mut Self::Domain, block: BasicBlock); fn reconstruct_before_statement_effect( &mut self, - state: &mut Self::FlowState, + state: &mut Self::Domain, statement: &mir::Statement<'tcx>, location: Location, ); fn reconstruct_statement_effect( &mut self, - state: &mut Self::FlowState, + state: &mut Self::Domain, statement: &mir::Statement<'tcx>, location: Location, ); fn reconstruct_before_terminator_effect( &mut self, - state: &mut Self::FlowState, + state: &mut Self::Domain, terminator: &mir::Terminator<'tcx>, location: Location, ); fn reconstruct_terminator_effect( &mut self, - state: &mut Self::FlowState, + state: &mut Self::Domain, terminator: &mir::Terminator<'tcx>, location: Location, ); @@ -131,21 +131,20 @@ impl<'tcx, A> ResultsVisitable<'tcx> for Results<'tcx, A> where A: Analysis<'tcx>, { - type FlowState = A::Domain; - + type Domain = A::Domain; type Direction = A::Direction; - fn new_flow_state(&self, body: &mir::Body<'tcx>) -> Self::FlowState { + fn bottom_value(&self, body: &mir::Body<'tcx>) -> Self::Domain { self.analysis.bottom_value(body) } - fn reset_to_block_entry(&self, state: &mut Self::FlowState, block: BasicBlock) { + fn reset_to_block_entry(&self, state: &mut Self::Domain, block: BasicBlock) { state.clone_from(self.entry_set_for_block(block)); } fn reconstruct_before_statement_effect( &mut self, - state: &mut Self::FlowState, + state: &mut Self::Domain, stmt: &mir::Statement<'tcx>, loc: Location, ) { @@ -154,7 +153,7 @@ where fn reconstruct_statement_effect( &mut self, - state: &mut Self::FlowState, + state: &mut Self::Domain, stmt: &mir::Statement<'tcx>, loc: Location, ) { @@ -163,7 +162,7 @@ where fn reconstruct_before_terminator_effect( &mut self, - state: &mut Self::FlowState, + state: &mut Self::Domain, term: &mir::Terminator<'tcx>, loc: Location, ) { @@ -172,7 +171,7 @@ where fn reconstruct_terminator_effect( &mut self, - state: &mut Self::FlowState, + state: &mut Self::Domain, term: &mir::Terminator<'tcx>, loc: Location, ) { diff --git a/compiler/rustc_mir_dataflow/src/points.rs b/compiler/rustc_mir_dataflow/src/points.rs index 4be7492366ad3..adfa94464a054 100644 --- a/compiler/rustc_mir_dataflow/src/points.rs +++ b/compiler/rustc_mir_dataflow/src/points.rs @@ -102,7 +102,7 @@ pub fn save_as_intervals<'tcx, N, R>( ) -> SparseIntervalMatrix where N: Idx, - R: ResultsVisitable<'tcx, FlowState = BitSet>, + R: ResultsVisitable<'tcx, Domain = BitSet>, { let values = SparseIntervalMatrix::new(elements.num_points()); let mut visitor = Visitor { elements, values }; @@ -124,12 +124,12 @@ impl<'mir, 'tcx, R, N> ResultsVisitor<'mir, 'tcx, R> for Visitor<'_, N> where N: Idx, { - type FlowState = BitSet; + type Domain = BitSet; fn visit_statement_after_primary_effect( &mut self, _results: &mut R, - state: &Self::FlowState, + state: &Self::Domain, _statement: &'mir mir::Statement<'tcx>, location: Location, ) { @@ -143,7 +143,7 @@ where fn visit_terminator_after_primary_effect( &mut self, _results: &mut R, - state: &Self::FlowState, + state: &Self::Domain, _terminator: &'mir mir::Terminator<'tcx>, location: Location, ) { diff --git a/compiler/rustc_mir_dataflow/src/rustc_peek.rs b/compiler/rustc_mir_dataflow/src/rustc_peek.rs index 8c3e6f49b1618..ac9b853f21b82 100644 --- a/compiler/rustc_mir_dataflow/src/rustc_peek.rs +++ b/compiler/rustc_mir_dataflow/src/rustc_peek.rs @@ -229,7 +229,7 @@ trait RustcPeekAt<'tcx>: Analysis<'tcx> { &self, tcx: TyCtxt<'tcx>, place: mir::Place<'tcx>, - flow_state: &Self::Domain, + state: &Self::Domain, call: PeekCall, ); } @@ -243,12 +243,12 @@ where &self, tcx: TyCtxt<'tcx>, place: mir::Place<'tcx>, - flow_state: &Self::Domain, + state: &Self::Domain, call: PeekCall, ) { match self.move_data().rev_lookup.find(place.as_ref()) { LookupResult::Exact(peek_mpi) => { - let bit_state = flow_state.contains(peek_mpi); + let bit_state = state.contains(peek_mpi); debug!("rustc_peek({:?} = &{:?}) bit_state: {}", call.arg, place, bit_state); if !bit_state { tcx.dcx().emit_err(PeekBitNotSet { span: call.span }); @@ -267,7 +267,7 @@ impl<'tcx> RustcPeekAt<'tcx> for MaybeLiveLocals { &self, tcx: TyCtxt<'tcx>, place: mir::Place<'tcx>, - flow_state: &BitSet, + state: &BitSet, call: PeekCall, ) { info!(?place, "peek_at"); @@ -276,7 +276,7 @@ impl<'tcx> RustcPeekAt<'tcx> for MaybeLiveLocals { return; }; - if !flow_state.contains(local) { + if !state.contains(local) { tcx.dcx().emit_err(PeekBitNotSet { span: call.span }); } } diff --git a/compiler/rustc_mir_transform/src/coroutine.rs b/compiler/rustc_mir_transform/src/coroutine.rs index 90243cd291082..1fb74f5d82ca2 100644 --- a/compiler/rustc_mir_transform/src/coroutine.rs +++ b/compiler/rustc_mir_transform/src/coroutine.rs @@ -885,12 +885,12 @@ struct StorageConflictVisitor<'a, 'tcx> { impl<'a, 'tcx, R> rustc_mir_dataflow::ResultsVisitor<'a, 'tcx, R> for StorageConflictVisitor<'a, 'tcx> { - type FlowState = BitSet; + type Domain = BitSet; fn visit_statement_before_primary_effect( &mut self, _results: &mut R, - state: &Self::FlowState, + state: &Self::Domain, _statement: &'a Statement<'tcx>, loc: Location, ) { @@ -900,7 +900,7 @@ impl<'a, 'tcx, R> rustc_mir_dataflow::ResultsVisitor<'a, 'tcx, R> fn visit_terminator_before_primary_effect( &mut self, _results: &mut R, - state: &Self::FlowState, + state: &Self::Domain, _terminator: &'a Terminator<'tcx>, loc: Location, ) { @@ -909,13 +909,13 @@ impl<'a, 'tcx, R> rustc_mir_dataflow::ResultsVisitor<'a, 'tcx, R> } impl StorageConflictVisitor<'_, '_> { - fn apply_state(&mut self, flow_state: &BitSet, loc: Location) { + fn apply_state(&mut self, state: &BitSet, loc: Location) { // Ignore unreachable blocks. if let TerminatorKind::Unreachable = self.body.basic_blocks[loc.block].terminator().kind { return; } - self.eligible_storage_live.clone_from(flow_state); + self.eligible_storage_live.clone_from(state); self.eligible_storage_live.intersect(&**self.saved_locals); for local in self.eligible_storage_live.iter() { diff --git a/compiler/rustc_mir_transform/src/dataflow_const_prop.rs b/compiler/rustc_mir_transform/src/dataflow_const_prop.rs index 79f12be4bc353..99c4d22901ea9 100644 --- a/compiler/rustc_mir_transform/src/dataflow_const_prop.rs +++ b/compiler/rustc_mir_transform/src/dataflow_const_prop.rs @@ -723,13 +723,13 @@ impl<'mir, 'tcx> ResultsVisitor<'mir, 'tcx, Results<'tcx, ValueAnalysisWrapper>>> for Collector<'tcx, '_> { - type FlowState = State>; + type Domain = State>; #[instrument(level = "trace", skip(self, results, statement))] fn visit_statement_before_primary_effect( &mut self, results: &mut Results<'tcx, ValueAnalysisWrapper>>, - state: &Self::FlowState, + state: &Self::Domain, statement: &'mir Statement<'tcx>, location: Location, ) { @@ -751,7 +751,7 @@ impl<'mir, 'tcx> fn visit_statement_after_primary_effect( &mut self, results: &mut Results<'tcx, ValueAnalysisWrapper>>, - state: &Self::FlowState, + state: &Self::Domain, statement: &'mir Statement<'tcx>, location: Location, ) { @@ -776,7 +776,7 @@ impl<'mir, 'tcx> fn visit_terminator_before_primary_effect( &mut self, results: &mut Results<'tcx, ValueAnalysisWrapper>>, - state: &Self::FlowState, + state: &Self::Domain, terminator: &'mir Terminator<'tcx>, location: Location, ) { From bde1f4dd57abd8e86dd7d7b325640558c4437d1f Mon Sep 17 00:00:00 2001 From: Vetle Rasmussen Date: Fri, 13 Sep 2024 11:15:19 +0200 Subject: [PATCH 4/8] Add set_dcx to ParseSess --- compiler/rustc_session/src/parse.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/compiler/rustc_session/src/parse.rs b/compiler/rustc_session/src/parse.rs index d6c58e9d1be1c..1db31f5b0a775 100644 --- a/compiler/rustc_session/src/parse.rs +++ b/compiler/rustc_session/src/parse.rs @@ -340,4 +340,8 @@ impl ParseSess { pub fn dcx(&self) -> DiagCtxtHandle<'_> { self.dcx.handle() } + + pub fn set_dcx(&mut self, dcx: DiagCtxt) { + self.dcx = dcx; + } } From f362a59c3ed6241e515e2516a048945a879bbfe3 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 13 Sep 2024 11:13:59 +0200 Subject: [PATCH 5/8] some fixes for clashing_extern_declarations lint --- compiler/rustc_lint/src/foreign_modules.rs | 74 +++++++------- tests/ui/lint/clashing-extern-fn-recursion.rs | 2 + tests/ui/lint/clashing-extern-fn.rs | 99 ++++++++++++++----- tests/ui/lint/clashing-extern-fn.stderr | 86 +++++++++++----- 4 files changed, 172 insertions(+), 89 deletions(-) diff --git a/compiler/rustc_lint/src/foreign_modules.rs b/compiler/rustc_lint/src/foreign_modules.rs index a60fc0ffbbb3b..33b8b7c544bcc 100644 --- a/compiler/rustc_lint/src/foreign_modules.rs +++ b/compiler/rustc_lint/src/foreign_modules.rs @@ -3,8 +3,7 @@ use rustc_data_structures::unord::{UnordMap, UnordSet}; use rustc_hir as hir; use rustc_hir::def::DefKind; use rustc_middle::query::Providers; -use rustc_middle::ty::layout::LayoutError; -use rustc_middle::ty::{self, Instance, Ty, TyCtxt}; +use rustc_middle::ty::{self, AdtDef, Instance, Ty, TyCtxt}; use rustc_session::declare_lint; use rustc_span::{sym, Span, Symbol}; use rustc_target::abi::FIRST_VARIANT; @@ -212,7 +211,17 @@ fn structurally_same_type<'tcx>( ckind: types::CItemKind, ) -> bool { let mut seen_types = UnordSet::default(); - structurally_same_type_impl(&mut seen_types, tcx, param_env, a, b, ckind) + let result = structurally_same_type_impl(&mut seen_types, tcx, param_env, a, b, ckind); + if cfg!(debug_assertions) && result { + // Sanity-check: must have same ABI, size and alignment. + // `extern` blocks cannot be generic, so we'll always get a layout here. + let a_layout = tcx.layout_of(param_env.and(a)).unwrap(); + let b_layout = tcx.layout_of(param_env.and(b)).unwrap(); + assert_eq!(a_layout.abi, b_layout.abi); + assert_eq!(a_layout.size, b_layout.size); + assert_eq!(a_layout.align, b_layout.align); + } + result } fn structurally_same_type_impl<'tcx>( @@ -266,30 +275,21 @@ fn structurally_same_type_impl<'tcx>( // Do a full, depth-first comparison between the two. use rustc_type_ir::TyKind::*; - let compare_layouts = |a, b| -> Result> { - debug!("compare_layouts({:?}, {:?})", a, b); - let a_layout = &tcx.layout_of(param_env.and(a))?.layout.abi(); - let b_layout = &tcx.layout_of(param_env.and(b))?.layout.abi(); - debug!( - "comparing layouts: {:?} == {:?} = {}", - a_layout, - b_layout, - a_layout == b_layout - ); - Ok(a_layout == b_layout) - }; - let is_primitive_or_pointer = |ty: Ty<'tcx>| ty.is_primitive() || matches!(ty.kind(), RawPtr(..) | Ref(..)); ensure_sufficient_stack(|| { match (a.kind(), b.kind()) { - (Adt(a_def, _), Adt(b_def, _)) => { - // We can immediately rule out these types as structurally same if - // their layouts differ. - match compare_layouts(a, b) { - Ok(false) => return false, - _ => (), // otherwise, continue onto the full, fields comparison + (&Adt(a_def, _), &Adt(b_def, _)) => { + // Only `repr(C)` types can be compared structurally. + if !(a_def.repr().c() && b_def.repr().c()) { + return false; + } + // If the types differ in their packed-ness, align, or simd-ness they conflict. + let repr_characteristica = + |def: AdtDef<'tcx>| (def.repr().pack, def.repr().align, def.repr().simd()); + if repr_characteristica(a_def) != repr_characteristica(b_def) { + return false; } // Grab a flattened representation of all fields. @@ -311,9 +311,9 @@ fn structurally_same_type_impl<'tcx>( }, ) } - (Array(a_ty, a_const), Array(b_ty, b_const)) => { - // For arrays, we also check the constness of the type. - a_const.kind() == b_const.kind() + (Array(a_ty, a_len), Array(b_ty, b_len)) => { + // For arrays, we also check the length. + a_len == b_len && structurally_same_type_impl( seen_types, tcx, param_env, *a_ty, *b_ty, ckind, ) @@ -357,10 +357,9 @@ fn structurally_same_type_impl<'tcx>( ckind, ) } - (Tuple(a_args), Tuple(b_args)) => { - a_args.iter().eq_by(b_args.iter(), |a_ty, b_ty| { - structurally_same_type_impl(seen_types, tcx, param_env, a_ty, b_ty, ckind) - }) + (Tuple(..), Tuple(..)) => { + // Tuples are not `repr(C)` so these cannot be compared structurally. + false } // For these, it's not quite as easy to define structural-sameness quite so easily. // For the purposes of this lint, take the conservative approach and mark them as @@ -380,24 +379,21 @@ fn structurally_same_type_impl<'tcx>( // An Adt and a primitive or pointer type. This can be FFI-safe if non-null // enum layout optimisation is being applied. (Adt(..), _) if is_primitive_or_pointer(b) => { - if let Some(ty) = types::repr_nullable_ptr(tcx, param_env, a, ckind) { - ty == b + if let Some(a_inner) = types::repr_nullable_ptr(tcx, param_env, a, ckind) { + a_inner == b } else { - compare_layouts(a, b).unwrap_or(false) + false } } (_, Adt(..)) if is_primitive_or_pointer(a) => { - if let Some(ty) = types::repr_nullable_ptr(tcx, param_env, b, ckind) { - ty == a + if let Some(b_inner) = types::repr_nullable_ptr(tcx, param_env, b, ckind) { + b_inner == a } else { - compare_layouts(a, b).unwrap_or(false) + false } } - // Otherwise, just compare the layouts. This may fail to lint for some - // incompatible types, but at the very least, will stop reads into - // uninitialised memory. - _ => compare_layouts(a, b).unwrap_or(false), + _ => false, } }) } diff --git a/tests/ui/lint/clashing-extern-fn-recursion.rs b/tests/ui/lint/clashing-extern-fn-recursion.rs index 40bef400594f6..ab311d398f92f 100644 --- a/tests/ui/lint/clashing-extern-fn-recursion.rs +++ b/tests/ui/lint/clashing-extern-fn-recursion.rs @@ -92,6 +92,7 @@ mod ref_recursion_once_removed { reffy: &'a Reffy2<'a>, } + #[repr(C)] struct Reffy2<'a> { reffy: &'a Reffy1<'a>, } @@ -107,6 +108,7 @@ mod ref_recursion_once_removed { reffy: &'a Reffy2<'a>, } + #[repr(C)] struct Reffy2<'a> { reffy: &'a Reffy1<'a>, } diff --git a/tests/ui/lint/clashing-extern-fn.rs b/tests/ui/lint/clashing-extern-fn.rs index 728dfabb393e0..a12fe81eecdaf 100644 --- a/tests/ui/lint/clashing-extern-fn.rs +++ b/tests/ui/lint/clashing-extern-fn.rs @@ -1,7 +1,6 @@ //@ check-pass //@ aux-build:external_extern_fn.rs #![crate_type = "lib"] -#![warn(clashing_extern_declarations)] mod redeclared_different_signature { mod a { @@ -132,7 +131,7 @@ mod banana { mod three { // This _should_ trigger the lint, because repr(packed) should generate a struct that has a // different layout. - #[repr(packed)] + #[repr(C, packed)] struct Banana { weight: u32, length: u16, @@ -143,6 +142,79 @@ mod banana { //~^ WARN `weigh_banana` redeclared with a different signature } } + + mod four { + // This _should_ trigger the lint, because the type is not repr(C). + struct Banana { + weight: u32, + length: u16, + } + #[allow(improper_ctypes)] + extern "C" { + fn weigh_banana(count: *const Banana) -> u64; + //~^ WARN `weigh_banana` redeclared with a different signature + } + } +} + +// 3-field structs can't be distinguished by ScalarPair, side-stepping some shortucts +// the logic used to (incorrectly) take. +mod banana3 { + mod one { + #[repr(C)] + struct Banana { + weight: u32, + length: u16, + color: u8, + } + extern "C" { + fn weigh_banana3(count: *const Banana) -> u64; + } + } + + mod two { + #[repr(C)] + struct Banana { + weight: u32, + length: u16, + color: u8, + } // note: distinct type + // This should not trigger the lint because two::Banana is structurally equivalent to + // one::Banana. + extern "C" { + fn weigh_banana3(count: *const Banana) -> u64; + } + } + + mod three { + // This _should_ trigger the lint, because repr(packed) should generate a struct that has a + // different layout. + #[repr(C, packed)] + struct Banana { + weight: u32, + length: u16, + color: u8, + } + #[allow(improper_ctypes)] + extern "C" { + fn weigh_banana3(count: *const Banana) -> u64; + //~^ WARN `weigh_banana3` redeclared with a different signature + } + } + + mod four { + // This _should_ trigger the lint, because the type is not repr(C). + struct Banana { + weight: u32, + length: u16, + color: u8, + } + #[allow(improper_ctypes)] + extern "C" { + fn weigh_banana3(count: *const Banana) -> u64; + //~^ WARN `weigh_banana3` redeclared with a different signature + } + } } mod sameish_members { @@ -223,27 +295,6 @@ mod transparent { } } -#[allow(improper_ctypes)] -mod zst { - mod transparent { - #[repr(transparent)] - struct TransparentZst(()); - extern "C" { - fn zst() -> (); - fn transparent_zst() -> TransparentZst; - } - } - - mod not_transparent { - struct NotTransparentZst(()); - extern "C" { - // These shouldn't warn since all return types are zero sized - fn zst() -> NotTransparentZst; - fn transparent_zst() -> NotTransparentZst; - } - } -} - mod missing_return_type { mod a { extern "C" { @@ -384,6 +435,7 @@ mod unknown_layout { extern "C" { pub fn generic(l: Link); } + #[repr(C)] pub struct Link { pub item: T, pub next: *const Link, @@ -394,6 +446,7 @@ mod unknown_layout { extern "C" { pub fn generic(l: Link); } + #[repr(C)] pub struct Link { pub item: T, pub next: *const Link, diff --git a/tests/ui/lint/clashing-extern-fn.stderr b/tests/ui/lint/clashing-extern-fn.stderr index f75ff6d05a15c..b30dd476a1d74 100644 --- a/tests/ui/lint/clashing-extern-fn.stderr +++ b/tests/ui/lint/clashing-extern-fn.stderr @@ -1,5 +1,5 @@ warning: `extern` block uses type `Option`, which is not FFI-safe - --> $DIR/clashing-extern-fn.rs:429:55 + --> $DIR/clashing-extern-fn.rs:482:55 | LL | fn hidden_niche_transparent_no_niche() -> Option; | ^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -9,7 +9,7 @@ LL | fn hidden_niche_transparent_no_niche() -> Option>>`, which is not FFI-safe - --> $DIR/clashing-extern-fn.rs:433:46 + --> $DIR/clashing-extern-fn.rs:486:46 | LL | fn hidden_niche_unsafe_cell() -> Option>>; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -18,7 +18,7 @@ LL | fn hidden_niche_unsafe_cell() -> Option $DIR/clashing-extern-fn.rs:14:13 + --> $DIR/clashing-extern-fn.rs:13:13 | LL | fn clash(x: u8); | ---------------- `clash` previously declared here @@ -28,14 +28,10 @@ LL | fn clash(x: u64); | = note: expected `unsafe extern "C" fn(u8)` found `unsafe extern "C" fn(u64)` -note: the lint level is defined here - --> $DIR/clashing-extern-fn.rs:4:9 - | -LL | #![warn(clashing_extern_declarations)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: `#[warn(clashing_extern_declarations)]` on by default warning: `extern_link_name` redeclared with a different signature - --> $DIR/clashing-extern-fn.rs:52:9 + --> $DIR/clashing-extern-fn.rs:51:9 | LL | #[link_name = "extern_link_name"] | --------------------------------- `extern_link_name` previously declared here @@ -47,7 +43,7 @@ LL | fn extern_link_name(x: u32); found `unsafe extern "C" fn(u32)` warning: `some_other_extern_link_name` redeclares `some_other_new_name` with a different signature - --> $DIR/clashing-extern-fn.rs:55:9 + --> $DIR/clashing-extern-fn.rs:54:9 | LL | fn some_other_new_name(x: i16); | ------------------------------- `some_other_new_name` previously declared here @@ -59,7 +55,7 @@ LL | #[link_name = "some_other_new_name"] found `unsafe extern "C" fn(u32)` warning: `other_both_names_different` redeclares `link_name_same` with a different signature - --> $DIR/clashing-extern-fn.rs:59:9 + --> $DIR/clashing-extern-fn.rs:58:9 | LL | #[link_name = "link_name_same"] | ------------------------------- `link_name_same` previously declared here @@ -71,7 +67,7 @@ LL | #[link_name = "link_name_same"] found `unsafe extern "C" fn(u32)` warning: `different_mod` redeclared with a different signature - --> $DIR/clashing-extern-fn.rs:72:9 + --> $DIR/clashing-extern-fn.rs:71:9 | LL | fn different_mod(x: u8); | ------------------------ `different_mod` previously declared here @@ -83,7 +79,7 @@ LL | fn different_mod(x: u64); found `unsafe extern "C" fn(u64)` warning: `variadic_decl` redeclared with a different signature - --> $DIR/clashing-extern-fn.rs:82:9 + --> $DIR/clashing-extern-fn.rs:81:9 | LL | fn variadic_decl(x: u8, ...); | ----------------------------- `variadic_decl` previously declared here @@ -95,7 +91,19 @@ LL | fn variadic_decl(x: u8); found `unsafe extern "C" fn(u8)` warning: `weigh_banana` redeclared with a different signature - --> $DIR/clashing-extern-fn.rs:142:13 + --> $DIR/clashing-extern-fn.rs:141:13 + | +LL | fn weigh_banana(count: *const Banana) -> u64; + | --------------------------------------------- `weigh_banana` previously declared here +... +LL | fn weigh_banana(count: *const Banana) -> u64; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration + | + = note: expected `unsafe extern "C" fn(*const banana::one::Banana) -> u64` + found `unsafe extern "C" fn(*const banana::three::Banana) -> u64` + +warning: `weigh_banana` redeclared with a different signature + --> $DIR/clashing-extern-fn.rs:154:13 | LL | fn weigh_banana(count: *const Banana) -> u64; | --------------------------------------------- `weigh_banana` previously declared here @@ -103,11 +111,35 @@ LL | fn weigh_banana(count: *const Banana) -> u64; LL | fn weigh_banana(count: *const Banana) -> u64; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration | - = note: expected `unsafe extern "C" fn(*const one::Banana) -> u64` - found `unsafe extern "C" fn(*const three::Banana) -> u64` + = note: expected `unsafe extern "C" fn(*const banana::one::Banana) -> u64` + found `unsafe extern "C" fn(*const banana::four::Banana) -> u64` + +warning: `weigh_banana3` redeclared with a different signature + --> $DIR/clashing-extern-fn.rs:200:13 + | +LL | fn weigh_banana3(count: *const Banana) -> u64; + | ---------------------------------------------- `weigh_banana3` previously declared here +... +LL | fn weigh_banana3(count: *const Banana) -> u64; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration + | + = note: expected `unsafe extern "C" fn(*const banana3::one::Banana) -> u64` + found `unsafe extern "C" fn(*const banana3::three::Banana) -> u64` + +warning: `weigh_banana3` redeclared with a different signature + --> $DIR/clashing-extern-fn.rs:214:13 + | +LL | fn weigh_banana3(count: *const Banana) -> u64; + | ---------------------------------------------- `weigh_banana3` previously declared here +... +LL | fn weigh_banana3(count: *const Banana) -> u64; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration + | + = note: expected `unsafe extern "C" fn(*const banana3::one::Banana) -> u64` + found `unsafe extern "C" fn(*const banana3::four::Banana) -> u64` warning: `draw_point` redeclared with a different signature - --> $DIR/clashing-extern-fn.rs:171:13 + --> $DIR/clashing-extern-fn.rs:243:13 | LL | fn draw_point(p: Point); | ------------------------ `draw_point` previously declared here @@ -119,7 +151,7 @@ LL | fn draw_point(p: Point); found `unsafe extern "C" fn(sameish_members::b::Point)` warning: `origin` redeclared with a different signature - --> $DIR/clashing-extern-fn.rs:197:13 + --> $DIR/clashing-extern-fn.rs:269:13 | LL | fn origin() -> Point3; | ---------------------- `origin` previously declared here @@ -131,7 +163,7 @@ LL | fn origin() -> Point3; found `unsafe extern "C" fn() -> same_sized_members_clash::b::Point3` warning: `transparent_incorrect` redeclared with a different signature - --> $DIR/clashing-extern-fn.rs:220:13 + --> $DIR/clashing-extern-fn.rs:292:13 | LL | fn transparent_incorrect() -> T; | -------------------------------- `transparent_incorrect` previously declared here @@ -143,7 +175,7 @@ LL | fn transparent_incorrect() -> isize; found `unsafe extern "C" fn() -> isize` warning: `missing_return_type` redeclared with a different signature - --> $DIR/clashing-extern-fn.rs:259:13 + --> $DIR/clashing-extern-fn.rs:310:13 | LL | fn missing_return_type() -> usize; | ---------------------------------- `missing_return_type` previously declared here @@ -155,7 +187,7 @@ LL | fn missing_return_type(); found `unsafe extern "C" fn()` warning: `non_zero_usize` redeclared with a different signature - --> $DIR/clashing-extern-fn.rs:277:13 + --> $DIR/clashing-extern-fn.rs:328:13 | LL | fn non_zero_usize() -> core::num::NonZero; | ------------------------------------------------- `non_zero_usize` previously declared here @@ -167,7 +199,7 @@ LL | fn non_zero_usize() -> usize; found `unsafe extern "C" fn() -> usize` warning: `non_null_ptr` redeclared with a different signature - --> $DIR/clashing-extern-fn.rs:279:13 + --> $DIR/clashing-extern-fn.rs:330:13 | LL | fn non_null_ptr() -> core::ptr::NonNull; | ----------------------------------------------- `non_null_ptr` previously declared here @@ -179,7 +211,7 @@ LL | fn non_null_ptr() -> *const usize; found `unsafe extern "C" fn() -> *const usize` warning: `option_non_zero_usize_incorrect` redeclared with a different signature - --> $DIR/clashing-extern-fn.rs:373:13 + --> $DIR/clashing-extern-fn.rs:424:13 | LL | fn option_non_zero_usize_incorrect() -> usize; | ---------------------------------------------- `option_non_zero_usize_incorrect` previously declared here @@ -191,7 +223,7 @@ LL | fn option_non_zero_usize_incorrect() -> isize; found `unsafe extern "C" fn() -> isize` warning: `option_non_null_ptr_incorrect` redeclared with a different signature - --> $DIR/clashing-extern-fn.rs:375:13 + --> $DIR/clashing-extern-fn.rs:426:13 | LL | fn option_non_null_ptr_incorrect() -> *const usize; | --------------------------------------------------- `option_non_null_ptr_incorrect` previously declared here @@ -203,7 +235,7 @@ LL | fn option_non_null_ptr_incorrect() -> *const isize; found `unsafe extern "C" fn() -> *const isize` warning: `hidden_niche_transparent_no_niche` redeclared with a different signature - --> $DIR/clashing-extern-fn.rs:429:13 + --> $DIR/clashing-extern-fn.rs:482:13 | LL | fn hidden_niche_transparent_no_niche() -> usize; | ------------------------------------------------ `hidden_niche_transparent_no_niche` previously declared here @@ -215,7 +247,7 @@ LL | fn hidden_niche_transparent_no_niche() -> Option Option` warning: `hidden_niche_unsafe_cell` redeclared with a different signature - --> $DIR/clashing-extern-fn.rs:433:13 + --> $DIR/clashing-extern-fn.rs:486:13 | LL | fn hidden_niche_unsafe_cell() -> usize; | --------------------------------------- `hidden_niche_unsafe_cell` previously declared here @@ -226,5 +258,5 @@ LL | fn hidden_niche_unsafe_cell() -> Option usize` found `unsafe extern "C" fn() -> Option>>` -warning: 19 warnings emitted +warning: 22 warnings emitted From d195f80639715d54918a7cff171113ed25474d27 Mon Sep 17 00:00:00 2001 From: Trevor Spiteri Date: Fri, 13 Sep 2024 13:28:22 +0200 Subject: [PATCH 6/8] Revert "stabilize const_float_bits_conv" for src/tools/clippy/clippy_lints This reverts the part of commit 19908ff7a37a9a431399bb6f2868efc22820f2b6 in subdirectory src/tools/clippy/clippy_lints. --- src/tools/clippy/clippy_lints/src/transmute/mod.rs | 6 +++--- .../clippy_lints/src/transmute/transmute_float_to_int.rs | 3 ++- .../clippy_lints/src/transmute/transmute_int_to_float.rs | 3 ++- .../clippy_lints/src/transmute/transmute_num_to_bytes.rs | 6 ++++++ 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/tools/clippy/clippy_lints/src/transmute/mod.rs b/src/tools/clippy/clippy_lints/src/transmute/mod.rs index a2ae36cc484ad..373bf61d8ff94 100644 --- a/src/tools/clippy/clippy_lints/src/transmute/mod.rs +++ b/src/tools/clippy/clippy_lints/src/transmute/mod.rs @@ -619,10 +619,10 @@ impl<'tcx> LateLintPass<'tcx> for Transmute { | transmute_ref_to_ref::check(cx, e, from_ty, to_ty, arg, const_context) | transmute_ptr_to_ptr::check(cx, e, from_ty, to_ty, arg, &self.msrv) | transmute_int_to_bool::check(cx, e, from_ty, to_ty, arg) - | transmute_int_to_float::check(cx, e, from_ty, to_ty, arg) + | transmute_int_to_float::check(cx, e, from_ty, to_ty, arg, const_context) | transmute_int_to_non_zero::check(cx, e, from_ty, to_ty, arg) - | transmute_float_to_int::check(cx, e, from_ty, to_ty, arg) - | transmute_num_to_bytes::check(cx, e, from_ty, to_ty, arg) + | transmute_float_to_int::check(cx, e, from_ty, to_ty, arg, const_context) + | transmute_num_to_bytes::check(cx, e, from_ty, to_ty, arg, const_context) | (unsound_collection_transmute::check(cx, e, from_ty, to_ty) || transmute_undefined_repr::check(cx, e, from_ty, to_ty)) | (eager_transmute::check(cx, e, arg, from_ty, to_ty)); diff --git a/src/tools/clippy/clippy_lints/src/transmute/transmute_float_to_int.rs b/src/tools/clippy/clippy_lints/src/transmute/transmute_float_to_int.rs index cb46109c27e2f..ab3bb5e1062d3 100644 --- a/src/tools/clippy/clippy_lints/src/transmute/transmute_float_to_int.rs +++ b/src/tools/clippy/clippy_lints/src/transmute/transmute_float_to_int.rs @@ -15,9 +15,10 @@ pub(super) fn check<'tcx>( from_ty: Ty<'tcx>, to_ty: Ty<'tcx>, mut arg: &'tcx Expr<'_>, + const_context: bool, ) -> bool { match (&from_ty.kind(), &to_ty.kind()) { - (ty::Float(float_ty), ty::Int(_) | ty::Uint(_)) => { + (ty::Float(float_ty), ty::Int(_) | ty::Uint(_)) if !const_context => { span_lint_and_then( cx, TRANSMUTE_FLOAT_TO_INT, diff --git a/src/tools/clippy/clippy_lints/src/transmute/transmute_int_to_float.rs b/src/tools/clippy/clippy_lints/src/transmute/transmute_int_to_float.rs index e00fb90c3074e..d51888e30971b 100644 --- a/src/tools/clippy/clippy_lints/src/transmute/transmute_int_to_float.rs +++ b/src/tools/clippy/clippy_lints/src/transmute/transmute_int_to_float.rs @@ -14,9 +14,10 @@ pub(super) fn check<'tcx>( from_ty: Ty<'tcx>, to_ty: Ty<'tcx>, arg: &'tcx Expr<'_>, + const_context: bool, ) -> bool { match (&from_ty.kind(), &to_ty.kind()) { - (ty::Int(_) | ty::Uint(_), ty::Float(_)) => { + (ty::Int(_) | ty::Uint(_), ty::Float(_)) if !const_context => { span_lint_and_then( cx, TRANSMUTE_INT_TO_FLOAT, diff --git a/src/tools/clippy/clippy_lints/src/transmute/transmute_num_to_bytes.rs b/src/tools/clippy/clippy_lints/src/transmute/transmute_num_to_bytes.rs index 362f2bb6960a2..88b0ac5a36887 100644 --- a/src/tools/clippy/clippy_lints/src/transmute/transmute_num_to_bytes.rs +++ b/src/tools/clippy/clippy_lints/src/transmute/transmute_num_to_bytes.rs @@ -14,12 +14,18 @@ pub(super) fn check<'tcx>( from_ty: Ty<'tcx>, to_ty: Ty<'tcx>, arg: &'tcx Expr<'_>, + const_context: bool, ) -> bool { match (&from_ty.kind(), &to_ty.kind()) { (ty::Int(_) | ty::Uint(_) | ty::Float(_), ty::Array(arr_ty, _)) => { if !matches!(arr_ty.kind(), ty::Uint(UintTy::U8)) { return false; } + if matches!(from_ty.kind(), ty::Float(_)) && const_context { + // TODO: Remove when const_float_bits_conv is stabilized + // rust#72447 + return false; + } span_lint_and_then( cx, From 581f1924e35a6f0dbc58e8c22befdc83630370b0 Mon Sep 17 00:00:00 2001 From: Trevor Spiteri Date: Fri, 13 Sep 2024 13:43:41 +0200 Subject: [PATCH 7/8] handle transmutes in const context if msrvs::CONST_FLOAT_BITS_CONV --- src/tools/clippy/clippy_config/src/msrvs.rs | 3 ++- src/tools/clippy/clippy_lints/src/transmute/mod.rs | 6 +++--- .../clippy_lints/src/transmute/transmute_float_to_int.rs | 6 +++++- .../clippy_lints/src/transmute/transmute_int_to_float.rs | 4 +++- .../clippy_lints/src/transmute/transmute_num_to_bytes.rs | 6 +++--- 5 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/tools/clippy/clippy_config/src/msrvs.rs b/src/tools/clippy/clippy_config/src/msrvs.rs index 0e8215aa8e3cd..e17458b3310fe 100644 --- a/src/tools/clippy/clippy_config/src/msrvs.rs +++ b/src/tools/clippy/clippy_config/src/msrvs.rs @@ -17,7 +17,8 @@ macro_rules! msrv_aliases { // names may refer to stabilized feature flags or library items msrv_aliases! { - 1,81,0 { LINT_REASONS_STABILIZATION } + 1,83,0 { CONST_FLOAT_BITS_CONV } + 1,81,0 { LINT_REASONS_STABILIZATION } 1,80,0 { BOX_INTO_ITER} 1,77,0 { C_STR_LITERALS } 1,76,0 { PTR_FROM_REF, OPTION_RESULT_INSPECT } diff --git a/src/tools/clippy/clippy_lints/src/transmute/mod.rs b/src/tools/clippy/clippy_lints/src/transmute/mod.rs index 373bf61d8ff94..da7906b118353 100644 --- a/src/tools/clippy/clippy_lints/src/transmute/mod.rs +++ b/src/tools/clippy/clippy_lints/src/transmute/mod.rs @@ -619,10 +619,10 @@ impl<'tcx> LateLintPass<'tcx> for Transmute { | transmute_ref_to_ref::check(cx, e, from_ty, to_ty, arg, const_context) | transmute_ptr_to_ptr::check(cx, e, from_ty, to_ty, arg, &self.msrv) | transmute_int_to_bool::check(cx, e, from_ty, to_ty, arg) - | transmute_int_to_float::check(cx, e, from_ty, to_ty, arg, const_context) + | transmute_int_to_float::check(cx, e, from_ty, to_ty, arg, const_context, &self.msrv) | transmute_int_to_non_zero::check(cx, e, from_ty, to_ty, arg) - | transmute_float_to_int::check(cx, e, from_ty, to_ty, arg, const_context) - | transmute_num_to_bytes::check(cx, e, from_ty, to_ty, arg, const_context) + | transmute_float_to_int::check(cx, e, from_ty, to_ty, arg, const_context, &self.msrv) + | transmute_num_to_bytes::check(cx, e, from_ty, to_ty, arg, const_context, &self.msrv) | (unsound_collection_transmute::check(cx, e, from_ty, to_ty) || transmute_undefined_repr::check(cx, e, from_ty, to_ty)) | (eager_transmute::check(cx, e, arg, from_ty, to_ty)); diff --git a/src/tools/clippy/clippy_lints/src/transmute/transmute_float_to_int.rs b/src/tools/clippy/clippy_lints/src/transmute/transmute_float_to_int.rs index ab3bb5e1062d3..3507eb9a12480 100644 --- a/src/tools/clippy/clippy_lints/src/transmute/transmute_float_to_int.rs +++ b/src/tools/clippy/clippy_lints/src/transmute/transmute_float_to_int.rs @@ -1,4 +1,5 @@ use super::TRANSMUTE_FLOAT_TO_INT; +use clippy_config::msrvs::{self, Msrv}; use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::sugg; use rustc_ast as ast; @@ -16,9 +17,12 @@ pub(super) fn check<'tcx>( to_ty: Ty<'tcx>, mut arg: &'tcx Expr<'_>, const_context: bool, + msrv: &Msrv, ) -> bool { match (&from_ty.kind(), &to_ty.kind()) { - (ty::Float(float_ty), ty::Int(_) | ty::Uint(_)) if !const_context => { + (ty::Float(float_ty), ty::Int(_) | ty::Uint(_)) + if !const_context || msrv.meets(msrvs::CONST_FLOAT_BITS_CONV) => + { span_lint_and_then( cx, TRANSMUTE_FLOAT_TO_INT, diff --git a/src/tools/clippy/clippy_lints/src/transmute/transmute_int_to_float.rs b/src/tools/clippy/clippy_lints/src/transmute/transmute_int_to_float.rs index d51888e30971b..c5c7ed6d398b9 100644 --- a/src/tools/clippy/clippy_lints/src/transmute/transmute_int_to_float.rs +++ b/src/tools/clippy/clippy_lints/src/transmute/transmute_int_to_float.rs @@ -1,4 +1,5 @@ use super::TRANSMUTE_INT_TO_FLOAT; +use clippy_config::msrvs::{self, Msrv}; use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::sugg; use rustc_errors::Applicability; @@ -15,9 +16,10 @@ pub(super) fn check<'tcx>( to_ty: Ty<'tcx>, arg: &'tcx Expr<'_>, const_context: bool, + msrv: &Msrv, ) -> bool { match (&from_ty.kind(), &to_ty.kind()) { - (ty::Int(_) | ty::Uint(_), ty::Float(_)) if !const_context => { + (ty::Int(_) | ty::Uint(_), ty::Float(_)) if !const_context || msrv.meets(msrvs::CONST_FLOAT_BITS_CONV) => { span_lint_and_then( cx, TRANSMUTE_INT_TO_FLOAT, diff --git a/src/tools/clippy/clippy_lints/src/transmute/transmute_num_to_bytes.rs b/src/tools/clippy/clippy_lints/src/transmute/transmute_num_to_bytes.rs index 88b0ac5a36887..a94cd27c7fd1d 100644 --- a/src/tools/clippy/clippy_lints/src/transmute/transmute_num_to_bytes.rs +++ b/src/tools/clippy/clippy_lints/src/transmute/transmute_num_to_bytes.rs @@ -1,4 +1,5 @@ use super::TRANSMUTE_NUM_TO_BYTES; +use clippy_config::msrvs::{self, Msrv}; use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::sugg; use rustc_errors::Applicability; @@ -15,15 +16,14 @@ pub(super) fn check<'tcx>( to_ty: Ty<'tcx>, arg: &'tcx Expr<'_>, const_context: bool, + msrv: &Msrv, ) -> bool { match (&from_ty.kind(), &to_ty.kind()) { (ty::Int(_) | ty::Uint(_) | ty::Float(_), ty::Array(arr_ty, _)) => { if !matches!(arr_ty.kind(), ty::Uint(UintTy::U8)) { return false; } - if matches!(from_ty.kind(), ty::Float(_)) && const_context { - // TODO: Remove when const_float_bits_conv is stabilized - // rust#72447 + if matches!(from_ty.kind(), ty::Float(_)) && const_context && !msrv.meets(msrvs::CONST_FLOAT_BITS_CONV) { return false; } From 57de75050a9e2aeddfe6a9f4c47776809167b0f7 Mon Sep 17 00:00:00 2001 From: Jesse Rusak Date: Mon, 19 Aug 2024 13:10:26 -0400 Subject: [PATCH 8/8] When calling a method on Fn* traits explicitly, argument diagnostics should point at the called method (eg Fn::call_once), not the underlying callee. Fixes 128848 --- compiler/rustc_hir_typeck/src/callee.rs | 2 +- .../rustc_hir_typeck/src/fn_ctxt/checks.rs | 413 +++++++++--------- tests/crashes/128848.rs | 5 - .../mismatch-args-crash-issue-128848.rs | 10 + .../mismatch-args-crash-issue-128848.stderr | 16 + 5 files changed, 245 insertions(+), 201 deletions(-) delete mode 100644 tests/crashes/128848.rs create mode 100644 tests/ui/mismatched_types/mismatch-args-crash-issue-128848.rs create mode 100644 tests/ui/mismatched_types/mismatch-args-crash-issue-128848.stderr diff --git a/compiler/rustc_hir_typeck/src/callee.rs b/compiler/rustc_hir_typeck/src/callee.rs index 9863d0364498e..3dc8759a9edee 100644 --- a/compiler/rustc_hir_typeck/src/callee.rs +++ b/compiler/rustc_hir_typeck/src/callee.rs @@ -58,7 +58,7 @@ pub(crate) fn check_legal_trait_for_method_call( enum CallStep<'tcx> { Builtin(Ty<'tcx>), DeferredClosure(LocalDefId, ty::FnSig<'tcx>), - /// E.g., enum variant constructors. + /// Call overloading when callee implements one of the Fn* traits. Overloaded(MethodCallee<'tcx>), } diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs index 21b040bd0bcb3..8348c6e9a169e 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs @@ -506,6 +506,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { fn_def_id, call_span, call_expr, + tuple_arguments, ); } } @@ -520,6 +521,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { fn_def_id: Option, call_span: Span, call_expr: &'tcx hir::Expr<'tcx>, + tuple_arguments: TupleArgumentsFlag, ) -> ErrorGuaranteed { // Next, let's construct the error let (error_span, call_ident, full_call_span, call_name, is_method) = match &call_expr.kind { @@ -865,6 +867,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { &matched_inputs, &formal_and_expected_inputs, is_method, + tuple_arguments, ); suggest_confusable(&mut err); return err.emit(); @@ -1001,6 +1004,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { &matched_inputs, &formal_and_expected_inputs, is_method, + tuple_arguments, ); suggest_confusable(&mut err); return err.emit(); @@ -1448,6 +1452,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { &matched_inputs, &formal_and_expected_inputs, is_method, + tuple_arguments, ); // And add a suggestion block for all of the parameters @@ -2219,21 +2224,27 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { matched_inputs: &IndexVec>, formal_and_expected_inputs: &IndexVec, Ty<'tcx>)>, is_method: bool, + tuple_arguments: TupleArgumentsFlag, ) { let Some(mut def_id) = callable_def_id else { return; }; + // If we're calling a method of a Fn/FnMut/FnOnce trait object implicitly + // (eg invoking a closure) we want to point at the underlying callable, + // not the method implicitly invoked (eg call_once). if let Some(assoc_item) = self.tcx.opt_associated_item(def_id) - // Possibly points at either impl or trait item, so try to get it - // to point to trait item, then get the parent. - // This parent might be an impl in the case of an inherent function, - // but the next check will fail. + // Since this is an associated item, it might point at either an impl or a trait item. + // We want it to always point to the trait item. + // If we're pointing at an inherent function, we don't need to do anything, + // so we fetch the parent and verify if it's a trait item. && let maybe_trait_item_def_id = assoc_item.trait_item_def_id.unwrap_or(def_id) && let maybe_trait_def_id = self.tcx.parent(maybe_trait_item_def_id) // Just an easy way to check "trait_def_id == Fn/FnMut/FnOnce" && let Some(call_kind) = self.tcx.fn_trait_kind_from_def_id(maybe_trait_def_id) && let Some(callee_ty) = callee_ty + // TupleArguments is set only when this is an implicit call (my_closure(...)) rather than explicit (my_closure.call(...)) + && tuple_arguments == TupleArguments { let callee_ty = callee_ty.peel_refs(); match *callee_ty.kind() { @@ -2303,166 +2314,173 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { { let mut spans: MultiSpan = def_span.into(); - let params_with_generics = self.get_hir_params_with_generics(def_id, is_method); - let mut generics_with_unmatched_params = Vec::new(); + if let Some(params_with_generics) = self.get_hir_params_with_generics(def_id, is_method) + { + debug_assert_eq!(params_with_generics.len(), matched_inputs.len()); - let check_for_matched_generics = || { - if matched_inputs.iter().any(|x| x.is_some()) - && params_with_generics.iter().any(|x| x.0.is_some()) - { - for (idx, (generic, _)) in params_with_generics.iter().enumerate() { - // Param has to have a generic and be matched to be relevant - if matched_inputs[idx.into()].is_none() { - continue; - } + let mut generics_with_unmatched_params = Vec::new(); - let Some(generic) = generic else { - continue; - }; + let check_for_matched_generics = || { + if matched_inputs.iter().any(|x| x.is_some()) + && params_with_generics.iter().any(|x| x.0.is_some()) + { + for (idx, (generic, _)) in params_with_generics.iter().enumerate() { + // Param has to have a generic and be matched to be relevant + if matched_inputs[idx.into()].is_none() { + continue; + } - for unmatching_idx in idx + 1..params_with_generics.len() { - if matched_inputs[unmatching_idx.into()].is_none() - && let Some(unmatched_idx_param_generic) = - params_with_generics[unmatching_idx].0 - && unmatched_idx_param_generic.name.ident() == generic.name.ident() - { - // We found a parameter that didn't match that needed to - return true; + let Some(generic) = generic else { + continue; + }; + + for unmatching_idx in idx + 1..params_with_generics.len() { + if matched_inputs[unmatching_idx.into()].is_none() + && let Some(unmatched_idx_param_generic) = + params_with_generics[unmatching_idx].0 + && unmatched_idx_param_generic.name.ident() + == generic.name.ident() + { + // We found a parameter that didn't match that needed to + return true; + } } } } - } - false - }; - - let check_for_matched_generics = check_for_matched_generics(); - - for (idx, (generic_param, param)) in - params_with_generics.iter().enumerate().filter(|(idx, _)| { - check_for_matched_generics - || expected_idx.is_none_or(|expected_idx| expected_idx == *idx) - }) - { - let Some(generic_param) = generic_param else { - spans.push_span_label(param.span, ""); - continue; + false }; - let other_params_matched: Vec<(usize, &hir::Param<'_>)> = params_with_generics - .iter() - .enumerate() - .filter(|(other_idx, (other_generic_param, _))| { - if *other_idx == idx { - return false; - } - let Some(other_generic_param) = other_generic_param else { - return false; - }; - if matched_inputs[idx.into()].is_none() - && matched_inputs[(*other_idx).into()].is_none() - { - return false; - } - if matched_inputs[idx.into()].is_some() - && matched_inputs[(*other_idx).into()].is_some() - { - return false; - } - other_generic_param.name.ident() == generic_param.name.ident() + let check_for_matched_generics = check_for_matched_generics(); + + for (idx, (generic_param, param)) in + params_with_generics.iter().enumerate().filter(|(idx, _)| { + check_for_matched_generics + || expected_idx.is_none_or(|expected_idx| expected_idx == *idx) }) - .map(|(other_idx, (_, other_param))| (other_idx, *other_param)) - .collect(); + { + let Some(generic_param) = generic_param else { + spans.push_span_label(param.span, ""); + continue; + }; - if !other_params_matched.is_empty() { - let other_param_matched_names: Vec = other_params_matched + let other_params_matched: Vec<(usize, &hir::Param<'_>)> = params_with_generics .iter() - .map(|(_, other_param)| { - if let hir::PatKind::Binding(_, _, ident, _) = other_param.pat.kind { - format!("`{ident}`") - } else { - "{unknown}".to_string() + .enumerate() + .filter(|(other_idx, (other_generic_param, _))| { + if *other_idx == idx { + return false; + } + let Some(other_generic_param) = other_generic_param else { + return false; + }; + if matched_inputs[idx.into()].is_none() + && matched_inputs[(*other_idx).into()].is_none() + { + return false; } + if matched_inputs[idx.into()].is_some() + && matched_inputs[(*other_idx).into()].is_some() + { + return false; + } + other_generic_param.name.ident() == generic_param.name.ident() }) + .map(|(other_idx, (_, other_param))| (other_idx, *other_param)) .collect(); - let matched_ty = self - .resolve_vars_if_possible(formal_and_expected_inputs[idx.into()].1) - .sort_string(self.tcx); + if !other_params_matched.is_empty() { + let other_param_matched_names: Vec = other_params_matched + .iter() + .map(|(_, other_param)| { + if let hir::PatKind::Binding(_, _, ident, _) = other_param.pat.kind + { + format!("`{ident}`") + } else { + "{unknown}".to_string() + } + }) + .collect(); - if matched_inputs[idx.into()].is_some() { - spans.push_span_label( - param.span, - format!( - "{} {} to match the {} type of this parameter", - display_list_with_comma_and(&other_param_matched_names), + let matched_ty = self + .resolve_vars_if_possible(formal_and_expected_inputs[idx.into()].1) + .sort_string(self.tcx); + + if matched_inputs[idx.into()].is_some() { + spans.push_span_label( + param.span, format!( - "need{}", - pluralize!(if other_param_matched_names.len() == 1 { - 0 - } else { - 1 - }) + "{} {} to match the {} type of this parameter", + display_list_with_comma_and(&other_param_matched_names), + format!( + "need{}", + pluralize!(if other_param_matched_names.len() == 1 { + 0 + } else { + 1 + }) + ), + matched_ty, ), - matched_ty, - ), - ); + ); + } else { + spans.push_span_label( + param.span, + format!( + "this parameter needs to match the {} type of {}", + matched_ty, + display_list_with_comma_and(&other_param_matched_names), + ), + ); + } + generics_with_unmatched_params.push(generic_param); } else { + spans.push_span_label(param.span, ""); + } + } + + for generic_param in self + .tcx + .hir() + .get_if_local(def_id) + .and_then(|node| node.generics()) + .into_iter() + .flat_map(|x| x.params) + .filter(|x| { + generics_with_unmatched_params + .iter() + .any(|y| x.name.ident() == y.name.ident()) + }) + { + let param_idents_matching: Vec = params_with_generics + .iter() + .filter(|(generic, _)| { + if let Some(generic) = generic { + generic.name.ident() == generic_param.name.ident() + } else { + false + } + }) + .map(|(_, param)| { + if let hir::PatKind::Binding(_, _, ident, _) = param.pat.kind { + format!("`{ident}`") + } else { + "{unknown}".to_string() + } + }) + .collect(); + + if !param_idents_matching.is_empty() { spans.push_span_label( - param.span, + generic_param.span, format!( - "this parameter needs to match the {} type of {}", - matched_ty, - display_list_with_comma_and(&other_param_matched_names), + "{} all reference this parameter {}", + display_list_with_comma_and(¶m_idents_matching), + generic_param.name.ident().name, ), ); } - generics_with_unmatched_params.push(generic_param); - } else { - spans.push_span_label(param.span, ""); } } - - for generic_param in self - .tcx - .hir() - .get_if_local(def_id) - .and_then(|node| node.generics()) - .into_iter() - .flat_map(|x| x.params) - .filter(|x| { - generics_with_unmatched_params.iter().any(|y| x.name.ident() == y.name.ident()) - }) - { - let param_idents_matching: Vec = params_with_generics - .iter() - .filter(|(generic, _)| { - if let Some(generic) = generic { - generic.name.ident() == generic_param.name.ident() - } else { - false - } - }) - .map(|(_, param)| { - if let hir::PatKind::Binding(_, _, ident, _) = param.pat.kind { - format!("`{ident}`") - } else { - "{unknown}".to_string() - } - }) - .collect(); - - if !param_idents_matching.is_empty() { - spans.push_span_label( - generic_param.span, - format!( - "{} all reference this parameter {}", - display_list_with_comma_and(¶m_idents_matching), - generic_param.name.ident().name, - ), - ); - } - } - err.span_note(spans, format!("{} defined here", self.tcx.def_descr(def_id))); } else if let Some(hir::Node::Expr(e)) = self.tcx.hir().get_if_local(def_id) && let hir::ExprKind::Closure(hir::Closure { body, .. }) = &e.kind @@ -2535,74 +2553,77 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { return; }; - let params_with_generics = self.get_hir_params_with_generics(def_id, is_method); - - for (idx, (generic_param, _)) in params_with_generics.iter().enumerate() { - if matched_inputs[idx.into()].is_none() { - continue; - } + if let Some(params_with_generics) = self.get_hir_params_with_generics(def_id, is_method) { + debug_assert_eq!(params_with_generics.len(), matched_inputs.len()); + for (idx, (generic_param, _)) in params_with_generics.iter().enumerate() { + if matched_inputs[idx.into()].is_none() { + continue; + } - let Some((_, matched_arg_span)) = provided_arg_tys.get(idx.into()) else { - continue; - }; + let Some((_, matched_arg_span)) = provided_arg_tys.get(idx.into()) else { + continue; + }; - let Some(generic_param) = generic_param else { - continue; - }; + let Some(generic_param) = generic_param else { + continue; + }; - let mut idxs_matched: Vec = vec![]; - for (other_idx, (_, _)) in params_with_generics.iter().enumerate().filter( - |(other_idx, (other_generic_param, _))| { - if *other_idx == idx { - return false; - } - let Some(other_generic_param) = other_generic_param else { - return false; - }; - if matched_inputs[(*other_idx).into()].is_some() { - return false; - } - other_generic_param.name.ident() == generic_param.name.ident() - }, - ) { - idxs_matched.push(other_idx); - } + let mut idxs_matched: Vec = vec![]; + for (other_idx, (_, _)) in params_with_generics.iter().enumerate().filter( + |(other_idx, (other_generic_param, _))| { + if *other_idx == idx { + return false; + } + let Some(other_generic_param) = other_generic_param else { + return false; + }; + if matched_inputs[(*other_idx).into()].is_some() { + return false; + } + other_generic_param.name.ident() == generic_param.name.ident() + }, + ) { + idxs_matched.push(other_idx); + } - if idxs_matched.is_empty() { - continue; - } + if idxs_matched.is_empty() { + continue; + } - let expected_display_type = self - .resolve_vars_if_possible(formal_and_expected_inputs[idx.into()].1) - .sort_string(self.tcx); - let label = if idxs_matched.len() == params_with_generics.len() - 1 { - format!( - "expected all arguments to be this {} type because they need to match the type of this parameter", - expected_display_type - ) - } else { - format!( - "expected some other arguments to be {} {} type to match the type of this parameter", - a_or_an(&expected_display_type), - expected_display_type, - ) - }; + let expected_display_type = self + .resolve_vars_if_possible(formal_and_expected_inputs[idx.into()].1) + .sort_string(self.tcx); + let label = if idxs_matched.len() == params_with_generics.len() - 1 { + format!( + "expected all arguments to be this {} type because they need to match the type of this parameter", + expected_display_type + ) + } else { + format!( + "expected some other arguments to be {} {} type to match the type of this parameter", + a_or_an(&expected_display_type), + expected_display_type, + ) + }; - err.span_label(*matched_arg_span, label); + err.span_label(*matched_arg_span, label); + } } } + /// Returns the parameters of a function, with their generic parameters if those are the full + /// type of that parameter. Returns `None` if the function body is unavailable (eg is an instrinsic). fn get_hir_params_with_generics( &self, def_id: DefId, is_method: bool, - ) -> Vec<(Option<&hir::GenericParam<'_>>, &hir::Param<'_>)> { - let fn_node = self.tcx.hir().get_if_local(def_id); + ) -> Option>, &hir::Param<'_>)>> { + let fn_node = self.tcx.hir().get_if_local(def_id)?; let generic_params: Vec>> = fn_node - .and_then(|node| node.fn_decl()) + .fn_decl()? + .inputs .into_iter() - .flat_map(|decl| decl.inputs) .skip(if is_method { 1 } else { 0 }) .map(|param| { if let hir::TyKind::Path(QPath::Resolved( @@ -2611,7 +2632,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { )) = param.kind { fn_node - .and_then(|node| node.generics()) + .generics() .into_iter() .flat_map(|generics| generics.params) .find(|param| ¶m.def_id.to_def_id() == res_def_id) @@ -2621,14 +2642,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { }) .collect(); - let params: Vec<&hir::Param<'_>> = fn_node - .and_then(|node| node.body_id()) + let params: Vec<&hir::Param<'_>> = self + .tcx + .hir() + .body(fn_node.body_id()?) + .params .into_iter() - .flat_map(|id| self.tcx.hir().body(id).params) .skip(if is_method { 1 } else { 0 }) .collect(); - generic_params.into_iter().zip(params).collect() + Some(generic_params.into_iter().zip_eq(params).collect()) } } diff --git a/tests/crashes/128848.rs b/tests/crashes/128848.rs deleted file mode 100644 index 636811fc6b504..0000000000000 --- a/tests/crashes/128848.rs +++ /dev/null @@ -1,5 +0,0 @@ -//@ known-bug: rust-lang/rust#128848 - -fn f(a: T, b: T, c: T) { - f.call_once() -} diff --git a/tests/ui/mismatched_types/mismatch-args-crash-issue-128848.rs b/tests/ui/mismatched_types/mismatch-args-crash-issue-128848.rs new file mode 100644 index 0000000000000..bfd6d4aeae645 --- /dev/null +++ b/tests/ui/mismatched_types/mismatch-args-crash-issue-128848.rs @@ -0,0 +1,10 @@ +#![feature(fn_traits)] + +// Regression test for https://github.com/rust-lang/rust/issues/128848 + +fn f(a: T, b: T, c: T) { + f.call_once() + //~^ ERROR this method takes 1 argument but 0 arguments were supplied +} + +fn main() {} diff --git a/tests/ui/mismatched_types/mismatch-args-crash-issue-128848.stderr b/tests/ui/mismatched_types/mismatch-args-crash-issue-128848.stderr new file mode 100644 index 0000000000000..899cf435ddf5d --- /dev/null +++ b/tests/ui/mismatched_types/mismatch-args-crash-issue-128848.stderr @@ -0,0 +1,16 @@ +error[E0061]: this method takes 1 argument but 0 arguments were supplied + --> $DIR/mismatch-args-crash-issue-128848.rs:6:7 + | +LL | f.call_once() + | ^^^^^^^^^-- argument #1 of type `(_, _, _)` is missing + | +note: method defined here + --> $SRC_DIR/core/src/ops/function.rs:LL:COL +help: provide the argument + | +LL | f.call_once(/* args */) + | ~~~~~~~~~~~~ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0061`.