From 06a41687b160cbb4cbf8fce0b3ad3a2e352e8338 Mon Sep 17 00:00:00 2001 From: WANG Rui Date: Thu, 14 Dec 2023 22:07:53 +0800 Subject: [PATCH 01/29] Add unstable `-Z direct-access-external-data` cmdline flag for `rustc` The new flag has been described in the Major Change Proposal at https://github.com/rust-lang/compiler-team/issues/707 --- compiler/rustc_codegen_llvm/src/mono_item.rs | 25 +++++++++++-------- compiler/rustc_interface/src/tests.rs | 1 + compiler/rustc_session/src/options.rs | 2 ++ compiler/rustc_session/src/session.rs | 7 ++++++ compiler/rustc_target/src/spec/mod.rs | 11 ++++++++ .../targets/loongarch64_unknown_linux_gnu.rs | 1 + .../direct-access-external-data.md | 16 ++++++++++++ tests/codegen/direct-access-external-data.rs | 21 ++++++++++++++++ 8 files changed, 74 insertions(+), 10 deletions(-) create mode 100644 src/doc/unstable-book/src/compiler-flags/direct-access-external-data.md create mode 100644 tests/codegen/direct-access-external-data.rs diff --git a/compiler/rustc_codegen_llvm/src/mono_item.rs b/compiler/rustc_codegen_llvm/src/mono_item.rs index f796ce0990f12..f763071936837 100644 --- a/compiler/rustc_codegen_llvm/src/mono_item.rs +++ b/compiler/rustc_codegen_llvm/src/mono_item.rs @@ -123,6 +123,17 @@ impl CodegenCx<'_, '_> { return false; } + // Match clang by only supporting COFF and ELF for now. + if self.tcx.sess.target.is_like_osx { + return false; + } + + // With pie relocation model calls of functions defined in the translation + // unit can use copy relocations. + if self.tcx.sess.relocation_model() == RelocModel::Pie && !is_declaration { + return true; + } + // Thread-local variables generally don't support copy relocations. let is_thread_local_var = llvm::LLVMIsAGlobalVariable(llval) .is_some_and(|v| llvm::LLVMIsThreadLocal(v) == llvm::True); @@ -130,18 +141,12 @@ impl CodegenCx<'_, '_> { return false; } - // Match clang by only supporting COFF and ELF for now. - if self.tcx.sess.target.is_like_osx { - return false; + // Respect the direct-access-external-data to override default behavior if present. + if let Some(direct) = self.tcx.sess.direct_access_external_data() { + return direct; } // Static relocation model should force copy relocations everywhere. - if self.tcx.sess.relocation_model() == RelocModel::Static { - return true; - } - - // With pie relocation model calls of functions defined in the translation - // unit can use copy relocations. - self.tcx.sess.relocation_model() == RelocModel::Pie && !is_declaration + self.tcx.sess.relocation_model() == RelocModel::Static } } diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index 588139a303c52..948db6a6ea101 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -748,6 +748,7 @@ fn test_unstable_options_tracking_hash() { tracked!(debug_macros, true); tracked!(default_hidden_visibility, Some(true)); tracked!(dep_info_omit_d_target, true); + tracked!(direct_access_external_data, Some(true)); tracked!(dual_proc_macros, true); tracked!(dwarf_version, Some(5)); tracked!(emit_thin_lto, false); diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index c97b18ebd66ad..ff8c5f4b3568e 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -1553,6 +1553,8 @@ options! { dep_info_omit_d_target: bool = (false, parse_bool, [TRACKED], "in dep-info output, omit targets for tracking dependencies of the dep-info files \ themselves (default: no)"), + direct_access_external_data: Option = (None, parse_opt_bool, [TRACKED], + "Direct or use GOT indirect to reference external data symbols"), dual_proc_macros: bool = (false, parse_bool, [TRACKED], "load proc macros for both target and host, but only link to the target (default: no)"), dump_dep_graph: bool = (false, parse_bool, [UNTRACKED], diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index 720599f609544..392485e2238c5 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -793,6 +793,13 @@ impl Session { self.opts.unstable_opts.tls_model.unwrap_or(self.target.tls_model) } + pub fn direct_access_external_data(&self) -> Option { + self.opts + .unstable_opts + .direct_access_external_data + .or(self.target.direct_access_external_data) + } + pub fn split_debuginfo(&self) -> SplitDebuginfo { self.opts.cg.split_debuginfo.unwrap_or(self.target.split_debuginfo) } diff --git a/compiler/rustc_target/src/spec/mod.rs b/compiler/rustc_target/src/spec/mod.rs index 8e26327196a1a..7979be7b5d6e8 100644 --- a/compiler/rustc_target/src/spec/mod.rs +++ b/compiler/rustc_target/src/spec/mod.rs @@ -1885,6 +1885,8 @@ pub struct TargetOptions { /// passed, and cannot be disabled even via `-C`. Corresponds to `llc /// -mattr=$features`. pub features: StaticCow, + /// Direct or use GOT indirect to reference external data symbols + pub direct_access_external_data: Option, /// Whether dynamic linking is available on this target. Defaults to false. pub dynamic_linking: bool, /// Whether dynamic linking can export TLS globals. Defaults to true. @@ -2279,6 +2281,7 @@ impl Default for TargetOptions { asm_args: cvs![], cpu: "generic".into(), features: "".into(), + direct_access_external_data: None, dynamic_linking: false, dll_tls_export: true, only_cdylib: false, @@ -2575,6 +2578,12 @@ impl Target { base.$key_name = s as u32; } } ); + ($key_name:ident, Option) => ( { + let name = (stringify!($key_name)).replace("_", "-"); + if let Some(s) = obj.remove(&name).and_then(|b| b.as_bool()) { + base.$key_name = Some(s); + } + } ); ($key_name:ident, Option) => ( { let name = (stringify!($key_name)).replace("_", "-"); if let Some(s) = obj.remove(&name).and_then(|b| b.as_u64()) { @@ -3003,6 +3012,7 @@ impl Target { key!(cpu); key!(features); key!(dynamic_linking, bool); + key!(direct_access_external_data, Option); key!(dll_tls_export, bool); key!(only_cdylib, bool); key!(executables, bool); @@ -3257,6 +3267,7 @@ impl ToJson for Target { target_option_val!(cpu); target_option_val!(features); target_option_val!(dynamic_linking); + target_option_val!(direct_access_external_data); target_option_val!(dll_tls_export); target_option_val!(only_cdylib); target_option_val!(executables); 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 0f05e7c475a83..234270c999b2a 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 @@ -11,6 +11,7 @@ pub fn target() -> Target { features: "+f,+d".into(), llvm_abiname: "lp64d".into(), max_atomic_width: Some(64), + direct_access_external_data: Some(false), ..base::linux_gnu::opts() }, } diff --git a/src/doc/unstable-book/src/compiler-flags/direct-access-external-data.md b/src/doc/unstable-book/src/compiler-flags/direct-access-external-data.md new file mode 100644 index 0000000000000..c72df43796038 --- /dev/null +++ b/src/doc/unstable-book/src/compiler-flags/direct-access-external-data.md @@ -0,0 +1,16 @@ +# `direct_access_external_data` + +The tracking issue for this feature is: https://github.com/rust-lang/compiler-team/issues/707 + +------------------------ + +Option `-Z direct-access-external-data` controls how to access symbols of +external data. + +Supported values for this option are: + +- `yes` - Don't use GOT indirection to reference external data symbols. +- `no` - Use GOT indirection to reference external data symbols. + +If the option is not explicitly specified, different targets have different +default values. diff --git a/tests/codegen/direct-access-external-data.rs b/tests/codegen/direct-access-external-data.rs new file mode 100644 index 0000000000000..ec4bfc33518db --- /dev/null +++ b/tests/codegen/direct-access-external-data.rs @@ -0,0 +1,21 @@ +// only-loongarch64-unknown-linux-gnu + +// revisions: DEFAULT DIRECT INDIRECT +// [DEFAULT] compile-flags: -C relocation-model=static +// [DIRECT] compile-flags: -C relocation-model=static -Z direct-access-external-data=yes +// [INDIRECT] compile-flags: -C relocation-model=static -Z direct-access-external-data=no + +#![crate_type = "rlib"] + +// DEFAULT: @VAR = external {{.*}} global i32 +// DIRECT: @VAR = external dso_local {{.*}} global i32 +// INDIRECT: @VAR = external {{.*}} global i32 + +extern "C" { + static VAR: i32; +} + +#[no_mangle] +pub fn get() -> i32 { + unsafe { VAR } +} From f38489e957a1f169be4071947a4426885793f03b Mon Sep 17 00:00:00 2001 From: Jarl Evanson Date: Sun, 28 Jan 2024 13:47:52 -0600 Subject: [PATCH 02/29] Enable `remove_storage_markers` MIR-opt test --- tests/mir-opt/remove_storage_markers.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/mir-opt/remove_storage_markers.rs b/tests/mir-opt/remove_storage_markers.rs index 6666ff3b7263b..74e399b6c0eed 100644 --- a/tests/mir-opt/remove_storage_markers.rs +++ b/tests/mir-opt/remove_storage_markers.rs @@ -1,4 +1,3 @@ -// skip-filecheck // EMIT_MIR_FOR_EACH_PANIC_STRATEGY // unit-test: RemoveStorageMarkers @@ -8,6 +7,10 @@ // EMIT_MIR remove_storage_markers.main.RemoveStorageMarkers.diff fn main() { + // CHECK-LABLE: fn main( + + // CHECK-NOT: StorageDead + // CHECK-NOT: StorageLive let mut sum = 0; for i in 0..10 { sum += i; From d1edc9d0dbd4ce9f1d71eba2ae4116efea8a6f2f Mon Sep 17 00:00:00 2001 From: Jarl Evanson Date: Sun, 28 Jan 2024 13:50:20 -0600 Subject: [PATCH 03/29] Enable `simplify` MIR-opt test --- tests/mir-opt/simplify_if.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/mir-opt/simplify_if.rs b/tests/mir-opt/simplify_if.rs index 19b5806f72094..f600c05958198 100644 --- a/tests/mir-opt/simplify_if.rs +++ b/tests/mir-opt/simplify_if.rs @@ -1,10 +1,13 @@ -// skip-filecheck // EMIT_MIR_FOR_EACH_PANIC_STRATEGY #[inline(never)] fn noop() {} // EMIT_MIR simplify_if.main.SimplifyConstCondition-after-const-prop.diff fn main() { + // CHECK-LABEL: fn main( + + // CHECK: bb0: { + // CHECK-NEXT: return; if false { noop(); } From 103159809aa83e00111f3b35be7281b8165681bc Mon Sep 17 00:00:00 2001 From: Jarl Evanson Date: Sun, 28 Jan 2024 16:04:07 -0600 Subject: [PATCH 04/29] Enable `lifetimes` SROA MIR-opt test --- tests/mir-opt/sroa/lifetimes.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/mir-opt/sroa/lifetimes.rs b/tests/mir-opt/sroa/lifetimes.rs index cc5c0c9bbcdb5..ea04fac15710e 100644 --- a/tests/mir-opt/sroa/lifetimes.rs +++ b/tests/mir-opt/sroa/lifetimes.rs @@ -1,4 +1,3 @@ -// skip-filecheck // unit-test: ScalarReplacementOfAggregates // compile-flags: -Cpanic=abort // no-prefer-dynamic @@ -16,6 +15,10 @@ struct Foo { // EMIT_MIR lifetimes.foo.ScalarReplacementOfAggregates.diff fn foo() { + // CHECK-LABEL: fn foo( + + // CHECK-NOT: [foo:_.*]: Foo + // CHECK-NOT: Box let foo: Foo = Foo { x: Ok(Box::new(5_u32)), y: 7_u32, From 7a2b66319ee219e757b6661549f460d8548dcbed Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Wed, 24 Jan 2024 11:04:55 +0000 Subject: [PATCH 05/29] interning doesn't check alignment anymroe, because it doesn't do any more projections. --- compiler/rustc_const_eval/src/const_eval/eval_queries.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs index 6a92ed9717de5..8499de20498ff 100644 --- a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs +++ b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs @@ -1,5 +1,3 @@ -use std::mem; - use either::{Left, Right}; use rustc_hir::def::DefKind; @@ -75,9 +73,7 @@ fn eval_body_using_ecx<'mir, 'tcx>( None => InternKind::Constant, } }; - let check_alignment = mem::replace(&mut ecx.machine.check_alignment, CheckAlignment::No); // interning doesn't need to respect alignment intern_const_alloc_recursive(ecx, intern_kind, &ret)?; - ecx.machine.check_alignment = check_alignment; debug!("eval_body_using_ecx done: {:?}", ret); Ok(ret) From b6d0225cafcf7d2421ad943647ed6ef4b8eb7bb4 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Wed, 24 Jan 2024 11:05:14 +0000 Subject: [PATCH 06/29] prefer instrumentation over entry/exit tracing statements --- compiler/rustc_const_eval/src/const_eval/eval_queries.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs index 8499de20498ff..a2d0f1c5583f3 100644 --- a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs +++ b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs @@ -22,12 +22,13 @@ use crate::interpret::{ }; // Returns a pointer to where the result lives +#[instrument(level = "trace", skip(ecx, body), ret)] fn eval_body_using_ecx<'mir, 'tcx>( ecx: &mut CompileTimeEvalContext<'mir, 'tcx>, cid: GlobalId<'tcx>, body: &'mir mir::Body<'tcx>, ) -> InterpResult<'tcx, MPlaceTy<'tcx>> { - debug!("eval_body_using_ecx: {:?}, {:?}", cid, ecx.param_env); + trace!(?ecx.param_env); let tcx = *ecx.tcx; assert!( cid.promoted.is_some() @@ -75,7 +76,6 @@ fn eval_body_using_ecx<'mir, 'tcx>( }; intern_const_alloc_recursive(ecx, intern_kind, &ret)?; - debug!("eval_body_using_ecx done: {:?}", ret); Ok(ret) } From a73c44889a6402f13d25fd5b973765ff62fb9885 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Wed, 24 Jan 2024 11:32:38 +0000 Subject: [PATCH 07/29] Prefer external iteration now that we don't actually recurse anymore --- .../rustc_const_eval/src/interpret/intern.rs | 38 ++++++++----------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/intern.rs b/compiler/rustc_const_eval/src/interpret/intern.rs index 751fbfacaad00..48920ba384ada 100644 --- a/compiler/rustc_const_eval/src/interpret/intern.rs +++ b/compiler/rustc_const_eval/src/interpret/intern.rs @@ -41,13 +41,12 @@ pub trait CompileTimeMachine<'mir, 'tcx: 'mir, T> = Machine< /// allocation is interned immutably; if it is `Mutability::Mut`, then the allocation *must be* /// already mutable (as a sanity check). /// -/// `recursive_alloc` is called for all recursively encountered allocations. +/// Returns an iterator over all relocations referred to by this allocation. fn intern_shallow<'rt, 'mir, 'tcx, T, M: CompileTimeMachine<'mir, 'tcx, T>>( ecx: &'rt mut InterpCx<'mir, 'tcx, M>, alloc_id: AllocId, mutability: Mutability, - mut recursive_alloc: impl FnMut(&InterpCx<'mir, 'tcx, M>, CtfeProvenance), -) -> Result<(), ()> { +) -> Result + 'tcx, ()> { trace!("intern_shallow {:?}", alloc_id); // remove allocation let Some((_kind, mut alloc)) = ecx.memory.alloc_map.remove(&alloc_id) else { @@ -65,14 +64,10 @@ fn intern_shallow<'rt, 'mir, 'tcx, T, M: CompileTimeMachine<'mir, 'tcx, T>>( assert_eq!(alloc.mutability, Mutability::Mut); } } - // record child allocations - for &(_, prov) in alloc.provenance().ptrs().iter() { - recursive_alloc(ecx, prov); - } // link the alloc id to the actual allocation let alloc = ecx.tcx.mk_const_alloc(alloc); ecx.tcx.set_alloc_id_memory(alloc_id, alloc); - Ok(()) + Ok(alloc.0.0.provenance().ptrs().iter().map(|&(_, prov)| prov)) } /// How a constant value should be interned. @@ -128,7 +123,7 @@ pub fn intern_const_alloc_recursive< } }; - // Initialize recursive interning. + // Intern the base allocation, and initialize todo list for recursive interning. let base_alloc_id = ret.ptr().provenance.unwrap().alloc_id(); let mut todo = vec![(base_alloc_id, base_mutability)]; // We need to distinguish "has just been interned" from "was already in `tcx`", @@ -154,7 +149,10 @@ pub fn intern_const_alloc_recursive< continue; } just_interned.insert(alloc_id); - intern_shallow(ecx, alloc_id, mutability, |ecx, prov| { + let provs = intern_shallow(ecx, alloc_id, mutability).map_err(|()| { + ecx.tcx.dcx().emit_err(DanglingPtrInFinal { span: ecx.tcx.span, kind: intern_kind }) + })?; + for prov in provs { let alloc_id = prov.alloc_id(); if intern_kind != InternKind::Promoted && inner_mutability == Mutability::Not @@ -169,7 +167,7 @@ pub fn intern_const_alloc_recursive< // during interning is to justify why we intern the *new* allocations immutably, // so we can completely ignore existing allocations. We also don't need to add // this to the todo list, since after all it is already interned. - return; + continue; } // Found a mutable pointer inside a const where inner allocations should be // immutable. We exclude promoteds from this, since things like `&mut []` and @@ -189,10 +187,7 @@ pub fn intern_const_alloc_recursive< // okay with losing some potential for immutability here. This can anyway only affect // `static mut`. todo.push((alloc_id, inner_mutability)); - }) - .map_err(|()| { - ecx.tcx.dcx().emit_err(DanglingPtrInFinal { span: ecx.tcx.span, kind: intern_kind }) - })?; + } } if found_bad_mutable_pointer { return Err(ecx @@ -220,13 +215,13 @@ pub fn intern_const_alloc_for_constprop< return Ok(()); } // Move allocation to `tcx`. - intern_shallow(ecx, alloc_id, Mutability::Not, |_ecx, _| { + for _ in intern_shallow(ecx, alloc_id, Mutability::Not).map_err(|()| err_ub!(DeadLocal))? { // We are not doing recursive interning, so we don't currently support provenance. // (If this assertion ever triggers, we should just implement a // proper recursive interning loop -- or just call `intern_const_alloc_recursive`. panic!("`intern_const_alloc_for_constprop` called on allocation with nested provenance") - }) - .map_err(|()| err_ub!(DeadLocal).into()) + } + Ok(()) } impl<'mir, 'tcx: 'mir, M: super::intern::CompileTimeMachine<'mir, 'tcx, !>> @@ -247,15 +242,14 @@ impl<'mir, 'tcx: 'mir, M: super::intern::CompileTimeMachine<'mir, 'tcx, !>> let dest = self.allocate(layout, MemoryKind::Stack)?; f(self, &dest.clone().into())?; let alloc_id = dest.ptr().provenance.unwrap().alloc_id(); // this was just allocated, it must have provenance - intern_shallow(self, alloc_id, Mutability::Not, |ecx, prov| { + for prov in intern_shallow(self, alloc_id, Mutability::Not).unwrap() { // We are not doing recursive interning, so we don't currently support provenance. // (If this assertion ever triggers, we should just implement a // proper recursive interning loop -- or just call `intern_const_alloc_recursive`. - if !ecx.tcx.try_get_global_alloc(prov.alloc_id()).is_some() { + if !self.tcx.try_get_global_alloc(prov.alloc_id()).is_some() { panic!("`intern_with_temp_alloc` with nested allocations"); } - }) - .unwrap(); + } Ok(alloc_id) } } From a57a00ebf69722de2944d37de10946cf3aa6fe15 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Wed, 24 Jan 2024 11:46:57 +0000 Subject: [PATCH 08/29] separately intern the outermost alloc from the rest --- .../rustc_const_eval/src/interpret/intern.rs | 80 +++++++++---------- 1 file changed, 39 insertions(+), 41 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/intern.rs b/compiler/rustc_const_eval/src/interpret/intern.rs index 48920ba384ada..7621d038855f5 100644 --- a/compiler/rustc_const_eval/src/interpret/intern.rs +++ b/compiler/rustc_const_eval/src/interpret/intern.rs @@ -125,10 +125,11 @@ pub fn intern_const_alloc_recursive< // Intern the base allocation, and initialize todo list for recursive interning. let base_alloc_id = ret.ptr().provenance.unwrap().alloc_id(); - let mut todo = vec![(base_alloc_id, base_mutability)]; + let mut todo: Vec<_> = + intern_shallow(ecx, base_alloc_id, base_mutability).unwrap().map(|prov| prov).collect(); // We need to distinguish "has just been interned" from "was already in `tcx`", // so we track this in a separate set. - let mut just_interned = FxHashSet::default(); + let mut just_interned: FxHashSet<_> = std::iter::once(base_alloc_id).collect(); // Whether we encountered a bad mutable pointer. // We want to first report "dangling" and then "mutable", so we need to delay reporting these // errors. @@ -142,52 +143,49 @@ pub fn intern_const_alloc_recursive< // raw pointers, so we cannot rely on validation to catch them -- and since interning runs // before validation, and interning doesn't know the type of anything, this means we can't show // better errors. Maybe we should consider doing validation before interning in the future. - while let Some((alloc_id, mutability)) = todo.pop() { + while let Some(prov) = todo.pop() { + let alloc_id = prov.alloc_id(); + if intern_kind != InternKind::Promoted + && inner_mutability == Mutability::Not + && !prov.immutable() + { + if ecx.tcx.try_get_global_alloc(alloc_id).is_some() + && !just_interned.contains(&alloc_id) + { + // This is a pointer to some memory from another constant. We encounter mutable + // pointers to such memory since we do not always track immutability through + // these "global" pointers. Allowing them is harmless; the point of these checks + // during interning is to justify why we intern the *new* allocations immutably, + // so we can completely ignore existing allocations. We also don't need to add + // this to the todo list, since after all it is already interned. + continue; + } + // Found a mutable pointer inside a const where inner allocations should be + // immutable. We exclude promoteds from this, since things like `&mut []` and + // `&None::>` lead to promotion that can produce mutable pointers. We rely + // on the promotion analysis not screwing up to ensure that it is sound to intern + // promoteds as immutable. + found_bad_mutable_pointer = true; + } if ecx.tcx.try_get_global_alloc(alloc_id).is_some() { // Already interned. debug_assert!(!ecx.memory.alloc_map.contains_key(&alloc_id)); continue; } just_interned.insert(alloc_id); - let provs = intern_shallow(ecx, alloc_id, mutability).map_err(|()| { + // We always intern with `inner_mutability`, and furthermore we ensured above that if + // that is "immutable", then there are *no* mutable pointers anywhere in the newly + // interned memory -- justifying that we can indeed intern immutably. However this also + // means we can *not* easily intern immutably here if `prov.immutable()` is true and + // `inner_mutability` is `Mut`: there might be other pointers to that allocation, and + // we'd have to somehow check that they are *all* immutable before deciding that this + // allocation can be made immutable. In the future we could consider analyzing all + // pointers before deciding which allocations can be made immutable; but for now we are + // okay with losing some potential for immutability here. This can anyway only affect + // `static mut`. + todo.extend(intern_shallow(ecx, alloc_id, inner_mutability).map_err(|()| { ecx.tcx.dcx().emit_err(DanglingPtrInFinal { span: ecx.tcx.span, kind: intern_kind }) - })?; - for prov in provs { - let alloc_id = prov.alloc_id(); - if intern_kind != InternKind::Promoted - && inner_mutability == Mutability::Not - && !prov.immutable() - { - if ecx.tcx.try_get_global_alloc(alloc_id).is_some() - && !just_interned.contains(&alloc_id) - { - // This is a pointer to some memory from another constant. We encounter mutable - // pointers to such memory since we do not always track immutability through - // these "global" pointers. Allowing them is harmless; the point of these checks - // during interning is to justify why we intern the *new* allocations immutably, - // so we can completely ignore existing allocations. We also don't need to add - // this to the todo list, since after all it is already interned. - continue; - } - // Found a mutable pointer inside a const where inner allocations should be - // immutable. We exclude promoteds from this, since things like `&mut []` and - // `&None::>` lead to promotion that can produce mutable pointers. We rely - // on the promotion analysis not screwing up to ensure that it is sound to intern - // promoteds as immutable. - found_bad_mutable_pointer = true; - } - // We always intern with `inner_mutability`, and furthermore we ensured above that if - // that is "immutable", then there are *no* mutable pointers anywhere in the newly - // interned memory -- justifying that we can indeed intern immutably. However this also - // means we can *not* easily intern immutably here if `prov.immutable()` is true and - // `inner_mutability` is `Mut`: there might be other pointers to that allocation, and - // we'd have to somehow check that they are *all* immutable before deciding that this - // allocation can be made immutable. In the future we could consider analyzing all - // pointers before deciding which allocations can be made immutable; but for now we are - // okay with losing some potential for immutability here. This can anyway only affect - // `static mut`. - todo.push((alloc_id, inner_mutability)); - } + })?); } if found_bad_mutable_pointer { return Err(ecx From 5d46b982c539ef3a227bd5557ec8a1648dfc5a5c Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 25 Jan 2024 10:38:17 +0000 Subject: [PATCH 09/29] Document base vs nested alloc interning --- compiler/rustc_const_eval/src/interpret/intern.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/compiler/rustc_const_eval/src/interpret/intern.rs b/compiler/rustc_const_eval/src/interpret/intern.rs index 7621d038855f5..c3a53f90e60a0 100644 --- a/compiler/rustc_const_eval/src/interpret/intern.rs +++ b/compiler/rustc_const_eval/src/interpret/intern.rs @@ -125,6 +125,9 @@ pub fn intern_const_alloc_recursive< // Intern the base allocation, and initialize todo list for recursive interning. let base_alloc_id = ret.ptr().provenance.unwrap().alloc_id(); + // First we intern the base allocation, as it requires a different mutability. + // This gives us the initial set of nested allocations, which will then all be processed + // recursively in the loop below. let mut todo: Vec<_> = intern_shallow(ecx, base_alloc_id, base_mutability).unwrap().map(|prov| prov).collect(); // We need to distinguish "has just been interned" from "was already in `tcx`", From 2e212b79e09bce189a787a5b0c05ee5318e3c574 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 25 Jan 2024 13:35:40 +1100 Subject: [PATCH 10/29] coverage: Split out counter increment sites from BCB node/edge counters This makes it possible for two nodes/edges in the coverage graph to share the same counter, without causing the instrumentor to inject unwanted duplicate counter-increment statements. --- .../src/coverage/counters.rs | 70 ++++++++------- .../rustc_mir_transform/src/coverage/mod.rs | 89 +++++++++---------- 2 files changed, 79 insertions(+), 80 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/counters.rs b/compiler/rustc_mir_transform/src/coverage/counters.rs index 8c11dea5d4ee6..9a1d8bae6b410 100644 --- a/compiler/rustc_mir_transform/src/coverage/counters.rs +++ b/compiler/rustc_mir_transform/src/coverage/counters.rs @@ -1,4 +1,5 @@ -use rustc_data_structures::fx::FxIndexMap; +use rustc_data_structures::captures::Captures; +use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::graph::WithNumNodes; use rustc_index::bit_set::BitSet; use rustc_index::IndexVec; @@ -38,19 +39,27 @@ impl Debug for BcbCounter { } } +#[derive(Debug)] +pub(super) enum CounterIncrementSite { + Node { bcb: BasicCoverageBlock }, + Edge { from_bcb: BasicCoverageBlock, to_bcb: BasicCoverageBlock }, +} + /// Generates and stores coverage counter and coverage expression information /// associated with nodes/edges in the BCB graph. pub(super) struct CoverageCounters { - next_counter_id: CounterId, + /// List of places where a counter-increment statement should be injected + /// into MIR, each with its corresponding counter ID. + counter_increment_sites: IndexVec, /// Coverage counters/expressions that are associated with individual BCBs. bcb_counters: IndexVec>, /// Coverage counters/expressions that are associated with the control-flow /// edge between two BCBs. /// - /// The iteration order of this map can affect the precise contents of MIR, - /// so we use `FxIndexMap` to avoid query stability hazards. - bcb_edge_counters: FxIndexMap<(BasicCoverageBlock, BasicCoverageBlock), BcbCounter>, + /// We currently don't iterate over this map, but if we do in the future, + /// switch it back to `FxIndexMap` to avoid query stability hazards. + bcb_edge_counters: FxHashMap<(BasicCoverageBlock, BasicCoverageBlock), BcbCounter>, /// Tracks which BCBs have a counter associated with some incoming edge. /// Only used by assertions, to verify that BCBs with incoming edge /// counters do not have their own physical counters (expressions are allowed). @@ -71,9 +80,9 @@ impl CoverageCounters { let num_bcbs = basic_coverage_blocks.num_nodes(); let mut this = Self { - next_counter_id: CounterId::START, + counter_increment_sites: IndexVec::new(), bcb_counters: IndexVec::from_elem_n(None, num_bcbs), - bcb_edge_counters: FxIndexMap::default(), + bcb_edge_counters: FxHashMap::default(), bcb_has_incoming_edge_counters: BitSet::new_empty(num_bcbs), expressions: IndexVec::new(), }; @@ -84,8 +93,8 @@ impl CoverageCounters { this } - fn make_counter(&mut self) -> BcbCounter { - let id = self.next_counter(); + fn make_counter(&mut self, site: CounterIncrementSite) -> BcbCounter { + let id = self.counter_increment_sites.push(site); BcbCounter::Counter { id } } @@ -103,15 +112,8 @@ impl CoverageCounters { self.make_expression(lhs, Op::Add, rhs) } - /// Counter IDs start from one and go up. - fn next_counter(&mut self) -> CounterId { - let next = self.next_counter_id; - self.next_counter_id = self.next_counter_id + 1; - next - } - pub(super) fn num_counters(&self) -> usize { - self.next_counter_id.as_usize() + self.counter_increment_sites.len() } #[cfg(test)] @@ -171,22 +173,26 @@ impl CoverageCounters { self.bcb_counters[bcb] } - pub(super) fn bcb_node_counters( + /// Returns an iterator over all the nodes/edges in the coverage graph that + /// should have a counter-increment statement injected into MIR, along with + /// each site's corresponding counter ID. + pub(super) fn counter_increment_sites( &self, - ) -> impl Iterator { - self.bcb_counters - .iter_enumerated() - .filter_map(|(bcb, counter_kind)| Some((bcb, counter_kind.as_ref()?))) + ) -> impl Iterator { + self.counter_increment_sites.iter_enumerated() } - /// For each edge in the BCB graph that has an associated counter, yields - /// that edge's *from* and *to* nodes, and its counter. - pub(super) fn bcb_edge_counters( + /// Returns an iterator over the subset of BCB nodes that have been associated + /// with a counter *expression*, along with the ID of that expression. + pub(super) fn bcb_nodes_with_coverage_expressions( &self, - ) -> impl Iterator { - self.bcb_edge_counters - .iter() - .map(|(&(from_bcb, to_bcb), counter_kind)| (from_bcb, to_bcb, counter_kind)) + ) -> impl Iterator + Captures<'_> { + self.bcb_counters.iter_enumerated().filter_map(|(bcb, &counter_kind)| match counter_kind { + // Yield the BCB along with its associated expression ID. + Some(BcbCounter::Expression { id }) => Some((bcb, id)), + // This BCB is associated with a counter or nothing, so skip it. + Some(BcbCounter::Counter { .. }) | None => None, + }) } pub(super) fn into_expressions(self) -> IndexVec { @@ -339,7 +345,8 @@ impl<'a> MakeBcbCounters<'a> { // program results in a tight infinite loop, but it should still compile. let one_path_to_target = !self.basic_coverage_blocks.bcb_has_multiple_in_edges(bcb); if one_path_to_target || self.bcb_predecessors(bcb).contains(&bcb) { - let counter_kind = self.coverage_counters.make_counter(); + let counter_kind = + self.coverage_counters.make_counter(CounterIncrementSite::Node { bcb }); if one_path_to_target { debug!("{bcb:?} gets a new counter: {counter_kind:?}"); } else { @@ -401,7 +408,8 @@ impl<'a> MakeBcbCounters<'a> { } // Make a new counter to count this edge. - let counter_kind = self.coverage_counters.make_counter(); + let counter_kind = + self.coverage_counters.make_counter(CounterIncrementSite::Edge { from_bcb, to_bcb }); debug!("Edge {from_bcb:?}->{to_bcb:?} gets a new counter: {counter_kind:?}"); self.coverage_counters.set_bcb_edge_counter(from_bcb, to_bcb, counter_kind) } diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index 5fb72fcf0cf3e..ef40af3716c1f 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -7,7 +7,7 @@ mod spans; #[cfg(test)] mod tests; -use self::counters::{BcbCounter, CoverageCounters}; +use self::counters::{CounterIncrementSite, CoverageCounters}; use self::graph::{BasicCoverageBlock, CoverageGraph}; use self::spans::{BcbMapping, BcbMappingKind, CoverageSpans}; @@ -155,61 +155,52 @@ fn inject_coverage_statements<'tcx>( bcb_has_coverage_spans: impl Fn(BasicCoverageBlock) -> bool, coverage_counters: &CoverageCounters, ) { - // Process the counters associated with BCB nodes. - for (bcb, counter_kind) in coverage_counters.bcb_node_counters() { - let do_inject = match counter_kind { - // Counter-increment statements always need to be injected. - BcbCounter::Counter { .. } => true, - // The only purpose of expression-used statements is to detect - // when a mapping is unreachable, so we only inject them for - // expressions with one or more mappings. - BcbCounter::Expression { .. } => bcb_has_coverage_spans(bcb), - }; - if do_inject { - inject_statement( - mir_body, - make_mir_coverage_kind(counter_kind), - basic_coverage_blocks[bcb].leader_bb(), - ); - } - } - - // Process the counters associated with BCB edges. - for (from_bcb, to_bcb, counter_kind) in coverage_counters.bcb_edge_counters() { - let do_inject = match counter_kind { - // Counter-increment statements always need to be injected. - BcbCounter::Counter { .. } => true, - // BCB-edge expressions never have mappings, so they never need - // a corresponding statement. - BcbCounter::Expression { .. } => false, + // Inject counter-increment statements into MIR. + for (id, counter_increment_site) in coverage_counters.counter_increment_sites() { + // Determine the block to inject a counter-increment statement into. + // For BCB nodes this is just their first block, but for edges we need + // to create a new block between the two BCBs, and inject into that. + let target_bb = match *counter_increment_site { + CounterIncrementSite::Node { bcb } => basic_coverage_blocks[bcb].leader_bb(), + CounterIncrementSite::Edge { from_bcb, to_bcb } => { + // Create a new block between the last block of `from_bcb` and + // the first block of `to_bcb`. + let from_bb = basic_coverage_blocks[from_bcb].last_bb(); + let to_bb = basic_coverage_blocks[to_bcb].leader_bb(); + + let new_bb = inject_edge_counter_basic_block(mir_body, from_bb, to_bb); + debug!( + "Edge {from_bcb:?} (last {from_bb:?}) -> {to_bcb:?} (leader {to_bb:?}) \ + requires a new MIR BasicBlock {new_bb:?} for counter increment {id:?}", + ); + new_bb + } }; - if !do_inject { - continue; - } - - // We need to inject a coverage statement into a new BB between the - // last BB of `from_bcb` and the first BB of `to_bcb`. - let from_bb = basic_coverage_blocks[from_bcb].last_bb(); - let to_bb = basic_coverage_blocks[to_bcb].leader_bb(); - let new_bb = inject_edge_counter_basic_block(mir_body, from_bb, to_bb); - debug!( - "Edge {from_bcb:?} (last {from_bb:?}) -> {to_bcb:?} (leader {to_bb:?}) \ - requires a new MIR BasicBlock {new_bb:?} for edge counter {counter_kind:?}", - ); - - // Inject a counter into the newly-created BB. - inject_statement(mir_body, make_mir_coverage_kind(counter_kind), new_bb); + inject_statement(mir_body, CoverageKind::CounterIncrement { id }, target_bb); } -} -fn make_mir_coverage_kind(counter_kind: &BcbCounter) -> CoverageKind { - match *counter_kind { - BcbCounter::Counter { id } => CoverageKind::CounterIncrement { id }, - BcbCounter::Expression { id } => CoverageKind::ExpressionUsed { id }, + // For each counter expression that is directly associated with at least one + // span, we inject an "expression-used" statement, so that coverage codegen + // can check whether the injected statement survived MIR optimization. + // (BCB edges can't have spans, so we only need to process BCB nodes here.) + // + // See the code in `rustc_codegen_llvm::coverageinfo::map_data` that deals + // with "expressions seen" and "zero terms". + for (bcb, expression_id) in coverage_counters + .bcb_nodes_with_coverage_expressions() + .filter(|&(bcb, _)| bcb_has_coverage_spans(bcb)) + { + inject_statement( + mir_body, + CoverageKind::ExpressionUsed { id: expression_id }, + basic_coverage_blocks[bcb].leader_bb(), + ); } } +/// Given two basic blocks that have a control-flow edge between them, creates +/// and returns a new block that sits between those blocks. fn inject_edge_counter_basic_block( mir_body: &mut mir::Body<'_>, from_bb: BasicBlock, From bae4f177b8767f6acab5474c6fec2734df759a32 Mon Sep 17 00:00:00 2001 From: Jarl Evanson Date: Sun, 4 Feb 2024 12:04:39 -0600 Subject: [PATCH 11/29] Enable `structs` SROA MIR-opt test --- tests/mir-opt/remove_storage_markers.rs | 2 +- tests/mir-opt/sroa/structs.rs | 125 ++++++++++++++++++++++-- 2 files changed, 120 insertions(+), 7 deletions(-) diff --git a/tests/mir-opt/remove_storage_markers.rs b/tests/mir-opt/remove_storage_markers.rs index 74e399b6c0eed..27661ab325411 100644 --- a/tests/mir-opt/remove_storage_markers.rs +++ b/tests/mir-opt/remove_storage_markers.rs @@ -7,7 +7,7 @@ // EMIT_MIR remove_storage_markers.main.RemoveStorageMarkers.diff fn main() { - // CHECK-LABLE: fn main( + // CHECK-LABEL: fn main( // CHECK-NOT: StorageDead // CHECK-NOT: StorageLive diff --git a/tests/mir-opt/sroa/structs.rs b/tests/mir-opt/sroa/structs.rs index 73563e12c94fc..5ea3795b86e2d 100644 --- a/tests/mir-opt/sroa/structs.rs +++ b/tests/mir-opt/sroa/structs.rs @@ -1,4 +1,3 @@ -// skip-filecheck // unit-test: ScalarReplacementOfAggregates // compile-flags: -Cpanic=abort // no-prefer-dynamic @@ -13,28 +12,68 @@ impl Drop for Tag { fn drop(&mut self) {} } +/// Check that SROA excludes structs with a `Drop` implementation. pub fn dropping() { + // CHECK-LABEL: fn dropping( + + // CHECK: [[aggregate:_[0-9]+]]: S; + + // CHECK: bb0: { + // CHECK: [[aggregate]] = S S(Tag(0), Tag(1), Tag(2)).1; } +/// Check that SROA excludes enums. pub fn enums(a: usize) -> usize { + // CHECK-LABEL: fn enums( + + // CHECK: [[enum:_[0-9]+]]: std::option::Option; + + // CHECK: bb0: { + // CHECK: [[enum]] = Option::::Some + // CHECK: _5 = (([[enum]] as Some).0: usize) + // CHECK: _0 = _5 if let Some(a) = Some(a) { a } else { 0 } } +/// Check that SROA destructures `U`. pub fn structs(a: f32) -> f32 { + // CHECK-LABEL: fn structs( struct U { _foo: usize, a: f32, } - + // CHECK: [[ret:_0]]: f32; + // CHECK: [[struct:_[0-9]+]]: structs::U; + // CHECK: [[a_tmp:_[0-9]+]]: f32; + // CHECK: [[foo:_[0-9]+]]: usize; + // CHECK: [[a_ret:_[0-9]+]]: f32; + + // CHECK: bb0: { + // CHECK-NOT: [[struct]] + // CHECK: [[a_tmp]] = _1; + // CHECK-NOT: [[struct]] + // CHECK: [[foo]] = const 0_usize; + // CHECK-NOT: [[struct]] + // CHECK: [[a_ret]] = move [[a_tmp]]; + // CHECK-NOT: [[struct]] + // CHECK: _0 = [[a_ret]]; + // CHECK-NOT: [[struct]] U { _foo: 0, a }.a } +/// Check that SROA excludes unions. pub fn unions(a: f32) -> u32 { + // CHECK-LABEL: fn unions( union Repr { f: f32, u: u32, } + // CHECK: [[union:_[0-9]+]]: unions::Repr; + + // CHECK: bb0: { + // CHECK: [[union]] = Repr { + // CHECK: _0 = ([[union]].1: u32) unsafe { Repr { f: a }.u } } @@ -46,11 +85,21 @@ struct Foo { d: Option, } -fn g() -> u32 { - 3 -} - +/// Check that non-escaping uses of a struct are destructured. pub fn flat() { + // CHECK-LABEL: fn flat( + + // CHECK: [[struct:_[0-9]+]]: Foo; + + // CHECK: bb0: { + // CHECK: [[init_unit:_[0-9]+]] = (); + // CHECK: [[init_opt_isize:_[0-9]+]] = Option::::Some + + // CHECK: [[destr_five:_[0-9]+]] = const 5_u8; + // CHECK: [[destr_unit:_[0-9]+]] = move [[init_unit]]; + // CHECK: [[destr_a:_[0-9]+]] = const "a"; + // CHECK: [[destr_opt_isize:_[0-9]+]] = move [[init_opt_isize]]; + let Foo { a, b, c, d } = Foo { a: 5, b: (), c: "a", d: Some(-4) }; let _ = a; let _ = b; @@ -65,6 +114,10 @@ struct Escaping { c: u32, } +fn g() -> u32 { + 3 +} + fn f(a: *const u32) { println!("{}", unsafe { *a.add(2) }); } @@ -76,10 +129,38 @@ fn f(a: *const u32) { // of them to `f`. However, this would lead to a miscompilation because `b` and `c` // might no longer appear right after `a` in memory. pub fn escaping() { + // CHECK-LABEL: fn escaping( + + // CHECK: [[ptr:_[0-9]+]]: *const u32; + // CHECK: [[ref:_[0-9]+]]: &u32; + // CHECK: [[struct:_[0-9]+]]: Escaping; + // CHECK: [[a:_[0-9]+]]: u32; + + // CHECK: bb0: { + // CHECK: [[struct]] = Escaping { + // CHECK: [[ref]] = &([[struct]].0 + // CHECK: [[ptr]] = &raw const (*[[ref]]); f(&Escaping { a: 1, b: 2, c: g() }.a); } +/// Check that copies from an internal struct are destructured and reassigned to +/// the original struct. fn copies(x: Foo) { + // CHECK-LABEL: fn copies( + + // CHECK: [[external:_[0-9]+]]: Foo) -> + // CHECK: [[internal:_[0-9]+]]: Foo; + // CHECK: [[byte:_[0-9]+]]: u8; + // CHECK: [[unit:_[0-9]+]]: (); + // CHECK: [[str:_[0-9]+]]: &str; + // CHECK: [[opt_isize:_[0-9]+]]: std::option::Option; + + // CHECK: bb0: { + // CHECK: [[byte]] = ([[external]].0 + // CHECK: [[unit]] = ([[external]].1 + // CHECK: [[str]] = ([[external]].2 + // CHECK: [[opt_isize]] = ([[external]].3 + let y = x; let t = y.a; let u = y.c; @@ -87,13 +168,44 @@ fn copies(x: Foo) { let a = z.b; } +/// Check that copies from an internal struct are destructured and reassigned to +/// the original struct. fn ref_copies(x: &Foo) { + // CHECK-LABEL: fn ref_copies( + + // CHECK: [[external:_[0-9]+]]: &Foo) -> + // CHECK: [[internal:_[0-9]+]]: Foo; + // CHECK: [[byte:_[0-9]+]]: u8; + // CHECK: [[unit:_[0-9]+]]: (); + // CHECK: [[str:_[0-9]+]]: &str; + // CHECK: [[opt_isize:_[0-9]+]]: std::option::Option; + + // CHECK: bb0: { + // CHECK: [[byte]] = ((*[[external]]).0 + // CHECK: [[unit]] = ((*[[external]]).1 + // CHECK: [[str]] = ((*[[external]]).2 + // CHECK: [[opt_isize]] = ((*[[external]]).3 + let y = *x; let t = y.a; let u = y.c; } +/// Check that deaggregated assignments from constants are placed after the constant's +/// assignment. Also check that copying field accesses from the copy of the constant are +/// reassigned to copy from the constant. fn constant() { + // CHECK-LABEL: constant( + + // CHECK: [[constant:_[0-9]+]]: (usize, u8); + // CHECK: [[t:_[0-9]+]]: usize; + // CHECK: [[u:_[0-9]+]]: u8; + + // CHECK: bb0: { + // CHECK-NOT: [[constant]] + // CHECK: [[constant]] = const + // CHECK: [[t]] = move ([[constant]].0: usize) + // CHECK: [[u]] = move ([[constant]].1: u8) const U: (usize, u8) = (5, 9); let y = U; let t = y.0; @@ -101,6 +213,7 @@ fn constant() { } fn main() { + // CHECK-LABEL: fn main( dropping(); enums(5); structs(5.); From d6221957e03d017d2763bd6635f88548984176eb Mon Sep 17 00:00:00 2001 From: Chris Copeland Date: Sun, 4 Feb 2024 15:28:35 -0800 Subject: [PATCH 12/29] Add an `armv8r-none-eabihf` target to support the Cortex-R52. --- compiler/rustc_target/src/spec/mod.rs | 1 + .../src/spec/targets/armv8r_none_eabihf.rs | 35 ++++++++++++++++ src/doc/rustc/src/SUMMARY.md | 1 + src/doc/rustc/src/platform-support.md | 1 + .../src/platform-support/arm-none-eabi.md | 1 + .../platform-support/armv8r-none-eabihf.md | 40 +++++++++++++++++++ src/tools/build-manifest/src/main.rs | 1 + tests/assembly/targets/targets-elf.rs | 3 ++ 8 files changed, 83 insertions(+) create mode 100644 compiler/rustc_target/src/spec/targets/armv8r_none_eabihf.rs create mode 100644 src/doc/rustc/src/platform-support/armv8r-none-eabihf.md diff --git a/compiler/rustc_target/src/spec/mod.rs b/compiler/rustc_target/src/spec/mod.rs index 6c698c5b01dd5..a57255db795ac 100644 --- a/compiler/rustc_target/src/spec/mod.rs +++ b/compiler/rustc_target/src/spec/mod.rs @@ -1543,6 +1543,7 @@ supported_targets! { ("armebv7r-none-eabihf", armebv7r_none_eabihf), ("armv7r-none-eabi", armv7r_none_eabi), ("armv7r-none-eabihf", armv7r_none_eabihf), + ("armv8r-none-eabihf", armv8r_none_eabihf), ("x86_64-pc-solaris", x86_64_pc_solaris), ("sparcv9-sun-solaris", sparcv9_sun_solaris), diff --git a/compiler/rustc_target/src/spec/targets/armv8r_none_eabihf.rs b/compiler/rustc_target/src/spec/targets/armv8r_none_eabihf.rs new file mode 100644 index 0000000000000..28dba4f7f5d83 --- /dev/null +++ b/compiler/rustc_target/src/spec/targets/armv8r_none_eabihf.rs @@ -0,0 +1,35 @@ +// Targets the Little-endian Cortex-R52 processor (ARMv8-R) + +use crate::spec::{Cc, LinkerFlavor, Lld, PanicStrategy, RelocModel, Target, TargetOptions}; + +pub fn target() -> Target { + Target { + llvm_target: "armv8r-none-eabihf".into(), + pointer_width: 32, + data_layout: "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64".into(), + arch: "arm".into(), + + options: TargetOptions { + abi: "eabihf".into(), + linker_flavor: LinkerFlavor::Gnu(Cc::No, Lld::Yes), + linker: Some("rust-lld".into()), + relocation_model: RelocModel::Static, + panic_strategy: PanicStrategy::Abort, + // The Cortex-R52 has two variants with respect to floating-point support: + // 1. fp-armv8, SP-only, with 16 DP (32 SP) registers + // 2. neon-fp-armv8, SP+DP, with 32 DP registers + // Use the lesser of these two options as the default, as it will produce code + // compatible with either variant. + // + // Reference: + // Arm Cortex-R52 Processor Technical Reference Manual + // - Chapter 15 Advanced SIMD and floating-point support + features: "+fp-armv8,-fp64,-d32".into(), + max_atomic_width: Some(64), + emit_debug_gdb_scripts: false, + // GCC defaults to 8 for arm-none here. + c_enum_min_bits: Some(8), + ..Default::default() + }, + } +} diff --git a/src/doc/rustc/src/SUMMARY.md b/src/doc/rustc/src/SUMMARY.md index 1998b008dc811..1f9307203bc3c 100644 --- a/src/doc/rustc/src/SUMMARY.md +++ b/src/doc/rustc/src/SUMMARY.md @@ -26,6 +26,7 @@ - [armv4t-none-eabi](platform-support/armv4t-none-eabi.md) - [armv5te-none-eabi](platform-support/armv5te-none-eabi.md) - [armv7r-none-eabi](platform-support/armv7r-none-eabi.md) + - [armv8r-none-eabihf](platform-support/armv8r-none-eabihf.md) - [armv6k-nintendo-3ds](platform-support/armv6k-nintendo-3ds.md) - [armv7-sony-vita-newlibeabihf](platform-support/armv7-sony-vita-newlibeabihf.md) - [armv7-unknown-linux-uclibceabi](platform-support/armv7-unknown-linux-uclibceabi.md) diff --git a/src/doc/rustc/src/platform-support.md b/src/doc/rustc/src/platform-support.md index f648a60b6c48d..13a7b483d6f3d 100644 --- a/src/doc/rustc/src/platform-support.md +++ b/src/doc/rustc/src/platform-support.md @@ -278,6 +278,7 @@ target | std | host | notes [`armv7a-kmc-solid_asp3-eabi`](platform-support/kmc-solid.md) | ✓ | | ARM SOLID with TOPPERS/ASP3 [`armv7a-kmc-solid_asp3-eabihf`](platform-support/kmc-solid.md) | ✓ | | ARM SOLID with TOPPERS/ASP3, hardfloat [`armv7a-none-eabihf`](platform-support/arm-none-eabi.md) | * | | Bare ARMv7-A, hardfloat +[`armv8r-none-eabihf`](platform-support/armv8r-none-eabihf.md) | * | | Bare ARMv8-R, hardfloat [`armv7k-apple-watchos`](platform-support/apple-watchos.md) | ✓ | | ARMv7-A Apple WatchOS `armv7s-apple-ios` | ✓ | | ARMv7-A Apple-A6 Apple iOS `avr-unknown-gnu-atmega328` | * | | AVR. Requires `-Z build-std=core` diff --git a/src/doc/rustc/src/platform-support/arm-none-eabi.md b/src/doc/rustc/src/platform-support/arm-none-eabi.md index 4f76d0d7bbce7..6335a6405a102 100644 --- a/src/doc/rustc/src/platform-support/arm-none-eabi.md +++ b/src/doc/rustc/src/platform-support/arm-none-eabi.md @@ -13,6 +13,7 @@ - [{arm,thumb}v4t-none-eabi](armv4t-none-eabi.md) - [{arm,thumb}v5te-none-eabi](armv5te-none-eabi.md) - armv7a-none-eabihf +- [armv8r-none-eabihf](armv8r-none-eabihf.md) Bare-metal target for 32-bit ARM CPUs. diff --git a/src/doc/rustc/src/platform-support/armv8r-none-eabihf.md b/src/doc/rustc/src/platform-support/armv8r-none-eabihf.md new file mode 100644 index 0000000000000..588e5d7c99449 --- /dev/null +++ b/src/doc/rustc/src/platform-support/armv8r-none-eabihf.md @@ -0,0 +1,40 @@ +# `armv8r-none-eabihf` + +**Tier: 3** + +Bare-metal target for CPUs in the ARMv8-R architecture family, supporting +dual ARM/Thumb mode, with ARM mode as the default. + +Processors in this family include the Arm [Cortex-R52][cortex-r52] +and [Cortex-R52+][cortex-r52-plus]. + +See [`arm-none-eabi`](arm-none-eabi.md) for information applicable to all +`arm-none-eabi` targets. + +[cortex-r52]: https://www.arm.com/products/silicon-ip-cpu/cortex-r/cortex-r52 +[cortex-r52-plus]: https://www.arm.com/products/silicon-ip-cpu/cortex-r/cortex-r52-plus + +## Target maintainers + +- [Chris Copeland](https://github.com/chrisnc), `chris@chrisnc.net` + +## Requirements + +The Cortex-R52 family always includes a floating-point unit, so there is no +non-`hf` version of this target. The floating-point features assumed by this +target are those of the single-precision-only config of the Cortex-R52, which +has 16 double-precision registers, accessible as 32 single-precision registers. +The other variant of Cortex-R52 includes double-precision, 32 double-precision +registers, and Advanced SIMD (Neon). + +The manual refers to this as the "Full Advanced SIMD config". To compile code +for this variant, use: `-C target-feature=+fp64,+d32,+neon`. See the [Advanced +SIMD and floating-point support][fpu] section of the Cortex-R52 Processor +Technical Reference Manual for more details. + +[fpu]: https://developer.arm.com/documentation/100026/0104/Advanced-SIMD-and-floating-point-support/About-the-Advanced-SIMD-and-floating-point-support + +## Cross-compilation toolchains and C code + +This target supports C code compiled with the `arm-none-eabi` target triple and +`-march=armv8-r` or a suitable `-mcpu` flag. diff --git a/src/tools/build-manifest/src/main.rs b/src/tools/build-manifest/src/main.rs index 1ef8cf7de3cd5..808fb0c07ea51 100644 --- a/src/tools/build-manifest/src/main.rs +++ b/src/tools/build-manifest/src/main.rs @@ -83,6 +83,7 @@ static TARGETS: &[&str] = &[ "armebv7r-none-eabihf", "armv7r-none-eabi", "armv7r-none-eabihf", + "armv8r-none-eabihf", "armv7s-apple-ios", "bpfeb-unknown-none", "bpfel-unknown-none", diff --git a/tests/assembly/targets/targets-elf.rs b/tests/assembly/targets/targets-elf.rs index 41f5df0fba001..6105ea430ddd3 100644 --- a/tests/assembly/targets/targets-elf.rs +++ b/tests/assembly/targets/targets-elf.rs @@ -174,6 +174,9 @@ // revisions: armv7r_none_eabihf // [armv7r_none_eabihf] compile-flags: --target armv7r-none-eabihf // [armv7r_none_eabihf] needs-llvm-components: arm +// revisions: armv8r_none_eabihf +// [armv8r_none_eabihf] compile-flags: --target armv8r-none-eabihf +// [armv8r_none_eabihf] needs-llvm-components: arm // FIXME: disabled since it fails on CI saying the csky component is missing /* revisions: csky_unknown_linux_gnuabiv2 From 9aff42e358c8199d5667b92f157f753814c39235 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Mon, 5 Feb 2024 14:59:07 -0600 Subject: [PATCH 13/29] Update src/doc/rustc/src/platform-support.md --- src/doc/rustc/src/platform-support.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/doc/rustc/src/platform-support.md b/src/doc/rustc/src/platform-support.md index 13a7b483d6f3d..80d9dc85f162a 100644 --- a/src/doc/rustc/src/platform-support.md +++ b/src/doc/rustc/src/platform-support.md @@ -278,9 +278,9 @@ target | std | host | notes [`armv7a-kmc-solid_asp3-eabi`](platform-support/kmc-solid.md) | ✓ | | ARM SOLID with TOPPERS/ASP3 [`armv7a-kmc-solid_asp3-eabihf`](platform-support/kmc-solid.md) | ✓ | | ARM SOLID with TOPPERS/ASP3, hardfloat [`armv7a-none-eabihf`](platform-support/arm-none-eabi.md) | * | | Bare ARMv7-A, hardfloat -[`armv8r-none-eabihf`](platform-support/armv8r-none-eabihf.md) | * | | Bare ARMv8-R, hardfloat [`armv7k-apple-watchos`](platform-support/apple-watchos.md) | ✓ | | ARMv7-A Apple WatchOS `armv7s-apple-ios` | ✓ | | ARMv7-A Apple-A6 Apple iOS +[`armv8r-none-eabihf`](platform-support/armv8r-none-eabihf.md) | * | | Bare ARMv8-R, hardfloat `avr-unknown-gnu-atmega328` | * | | AVR. Requires `-Z build-std=core` `bpfeb-unknown-none` | * | | BPF (big endian) `bpfel-unknown-none` | * | | BPF (little endian) From c94769a9748233559313c532d524f58ebb643b1d Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 5 Feb 2024 22:21:40 +0100 Subject: [PATCH 14/29] Clarify order of operations during interning Co-authored-by: Ralf Jung --- compiler/rustc_const_eval/src/interpret/intern.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/compiler/rustc_const_eval/src/interpret/intern.rs b/compiler/rustc_const_eval/src/interpret/intern.rs index c3a53f90e60a0..38e7843761be8 100644 --- a/compiler/rustc_const_eval/src/interpret/intern.rs +++ b/compiler/rustc_const_eval/src/interpret/intern.rs @@ -148,6 +148,13 @@ pub fn intern_const_alloc_recursive< // better errors. Maybe we should consider doing validation before interning in the future. while let Some(prov) = todo.pop() { let alloc_id = prov.alloc_id(); + // Crucially, we check this *before* checking whether the `alloc_id` + // has already been interned. The point of this check is to ensure that when + // there are multiple pointers to the same allocation, they are *all* immutable. + // Therefore it would be bad if we only checked the first pointer to any given + // allocation. + // (It is likely not possible to actually have multiple pointers to the same allocation, + // so alternatively we could also check that and ICE if there are multiple such pointers.) if intern_kind != InternKind::Promoted && inner_mutability == Mutability::Not && !prov.immutable() From 411967c0786495d750a60d1cb67087471bc3684f Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Wed, 31 Jan 2024 02:37:44 +0100 Subject: [PATCH 15/29] Zip together `place_ty` and `place_validity` --- .../rustc_pattern_analysis/src/usefulness.rs | 79 +++++++++++-------- 1 file changed, 46 insertions(+), 33 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/usefulness.rs b/compiler/rustc_pattern_analysis/src/usefulness.rs index bbb68b353e436..246b15ad7d013 100644 --- a/compiler/rustc_pattern_analysis/src/usefulness.rs +++ b/compiler/rustc_pattern_analysis/src/usefulness.rs @@ -767,12 +767,6 @@ impl<'a, Cx: TypeCx> PlaceCtxt<'a, Cx> { fn ctor_arity(&self, ctor: &Constructor) -> usize { self.cx.ctor_arity(ctor, self.ty) } - fn ctor_sub_tys( - &'a self, - ctor: &'a Constructor, - ) -> impl Iterator + ExactSizeIterator + Captures<'a> { - self.cx.ctor_sub_tys(ctor, self.ty) - } fn ctors_for_ty(&self) -> Result, Cx::Error> { self.cx.ctors_for_ty(self.ty) } @@ -828,6 +822,32 @@ impl fmt::Display for ValidityConstraint { } } +/// Data about a place under investigation. +struct PlaceInfo { + /// The type of the place. + ty: Cx::Ty, + /// Whether the place is known to contain valid data. + validity: ValidityConstraint, +} + +impl PlaceInfo { + fn specialize<'a>( + &'a self, + cx: &'a Cx, + ctor: &'a Constructor, + ) -> impl Iterator + ExactSizeIterator + Captures<'a> { + let ctor_sub_tys = cx.ctor_sub_tys(ctor, &self.ty); + let ctor_sub_validity = self.validity.specialize(ctor); + ctor_sub_tys.map(move |ty| PlaceInfo { ty, validity: ctor_sub_validity }) + } +} + +impl Clone for PlaceInfo { + fn clone(&self) -> Self { + Self { ty: self.ty.clone(), validity: self.validity } + } +} + /// Represents a pattern-tuple under investigation. // The three lifetimes are: // - 'p coming from the input @@ -1001,10 +1021,9 @@ struct Matrix<'p, Cx: TypeCx> { /// each column must have the same type. Each column corresponds to a place within the /// scrutinee. rows: Vec>, - /// Track the type of each column/place. - place_ty: SmallVec<[Cx::Ty; 2]>, - /// Track for each column/place whether it contains a known valid value. - place_validity: SmallVec<[ValidityConstraint; 2]>, + /// Track info about each place. Each place corresponds to a column in `rows`, and their types + /// must match. + place_info: SmallVec<[PlaceInfo; 2]>, /// Track whether the virtual wildcard row used to compute exhaustiveness is relevant. See top /// of the file for details on relevancy. wildcard_row_is_relevant: bool, @@ -1032,10 +1051,10 @@ impl<'p, Cx: TypeCx> Matrix<'p, Cx> { scrut_ty: Cx::Ty, scrut_validity: ValidityConstraint, ) -> Self { + let place_info = PlaceInfo { ty: scrut_ty, validity: scrut_validity }; let mut matrix = Matrix { rows: Vec::with_capacity(arms.len()), - place_ty: smallvec![scrut_ty], - place_validity: smallvec![scrut_validity], + place_info: smallvec![place_info], wildcard_row_is_relevant: true, }; for (row_id, arm) in arms.iter().enumerate() { @@ -1051,11 +1070,11 @@ impl<'p, Cx: TypeCx> Matrix<'p, Cx> { matrix } - fn head_ty(&self) -> Option<&Cx::Ty> { - self.place_ty.first() + fn head_place(&self) -> Option<&PlaceInfo> { + self.place_info.first() } fn column_count(&self) -> usize { - self.place_ty.len() + self.place_info.len() } fn rows( @@ -1083,18 +1102,13 @@ impl<'p, Cx: TypeCx> Matrix<'p, Cx> { ctor: &Constructor, ctor_is_relevant: bool, ) -> Result, Cx::Error> { - let ctor_sub_tys = pcx.ctor_sub_tys(ctor); - let arity = ctor_sub_tys.len(); - let specialized_place_ty = ctor_sub_tys.chain(self.place_ty[1..].iter().cloned()).collect(); - let ctor_sub_validity = self.place_validity[0].specialize(ctor); - let specialized_place_validity = std::iter::repeat(ctor_sub_validity) - .take(arity) - .chain(self.place_validity[1..].iter().copied()) - .collect(); + let subfield_place_info = self.place_info[0].specialize(pcx.cx, ctor); + let arity = subfield_place_info.len(); + let specialized_place_info = + subfield_place_info.chain(self.place_info[1..].iter().cloned()).collect(); let mut matrix = Matrix { rows: Vec::new(), - place_ty: specialized_place_ty, - place_validity: specialized_place_validity, + place_info: specialized_place_info, wildcard_row_is_relevant: self.wildcard_row_is_relevant && ctor_is_relevant, }; for (i, row) in self.rows().enumerate() { @@ -1127,11 +1141,11 @@ impl<'p, Cx: TypeCx> fmt::Debug for Matrix<'p, Cx> { .map(|row| row.iter().map(|pat| format!("{pat:?}")).collect()) .collect(); pretty_printed_matrix - .push(self.place_validity.iter().map(|validity| format!("{validity}")).collect()); + .push(self.place_info.iter().map(|place| format!("{}", place.validity)).collect()); let column_count = self.column_count(); assert!(self.rows.iter().all(|row| row.len() == column_count)); - assert!(self.place_validity.len() == column_count); + assert!(self.place_info.len() == column_count); let column_widths: Vec = (0..column_count) .map(|col| pretty_printed_matrix.iter().map(|row| row[col].len()).max().unwrap_or(0)) .collect(); @@ -1447,7 +1461,7 @@ fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>( return Ok(WitnessMatrix::empty()); } - let Some(ty) = matrix.head_ty().cloned() else { + let Some(place) = matrix.head_place() else { // The base case: there are no columns in the matrix. We are morally pattern-matching on (). // A row is useful iff it has no (unguarded) rows above it. let mut useful = true; // Whether the next row is useful. @@ -1467,18 +1481,17 @@ fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>( }; }; - debug!("ty: {ty:?}"); - let pcx = &PlaceCtxt { cx: mcx.tycx, ty: &ty }; + let ty = &place.ty.clone(); // Clone it out so we can mutate `matrix` later. + let pcx = &PlaceCtxt { cx: mcx.tycx, ty }; + debug!("ty: {:?}", pcx.ty); let ctors_for_ty = pcx.ctors_for_ty()?; - // Whether the place/column we are inspecting is known to contain valid data. - let place_validity = matrix.place_validity[0]; // We treat match scrutinees of type `!` or `EmptyEnum` differently. let is_toplevel_exception = is_top_level && matches!(ctors_for_ty, ConstructorSet::NoConstructors); // Whether empty patterns are counted as useful or not. We only warn an empty arm unreachable if // it is guaranteed unreachable by the opsem (i.e. if the place is `known_valid`). - let empty_arms_are_unreachable = place_validity.is_known_valid() + let empty_arms_are_unreachable = place.validity.is_known_valid() && (is_toplevel_exception || mcx.tycx.is_exhaustive_patterns_feature_on() || mcx.tycx.is_min_exhaustive_patterns_feature_on()); From 6cac1c459ec1f2aa7dd31e9b1b90040c906c64f5 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Wed, 31 Jan 2024 02:46:10 +0100 Subject: [PATCH 16/29] Track `is_top_level` via `PlaceInfo` --- .../rustc_pattern_analysis/src/usefulness.rs | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/usefulness.rs b/compiler/rustc_pattern_analysis/src/usefulness.rs index 246b15ad7d013..1ac984ce67eec 100644 --- a/compiler/rustc_pattern_analysis/src/usefulness.rs +++ b/compiler/rustc_pattern_analysis/src/usefulness.rs @@ -828,6 +828,8 @@ struct PlaceInfo { ty: Cx::Ty, /// Whether the place is known to contain valid data. validity: ValidityConstraint, + /// Whether the place is the scrutinee itself or a subplace of it. + is_scrutinee: bool, } impl PlaceInfo { @@ -838,13 +840,17 @@ impl PlaceInfo { ) -> impl Iterator + ExactSizeIterator + Captures<'a> { let ctor_sub_tys = cx.ctor_sub_tys(ctor, &self.ty); let ctor_sub_validity = self.validity.specialize(ctor); - ctor_sub_tys.map(move |ty| PlaceInfo { ty, validity: ctor_sub_validity }) + ctor_sub_tys.map(move |ty| PlaceInfo { + ty, + validity: ctor_sub_validity, + is_scrutinee: false, + }) } } impl Clone for PlaceInfo { fn clone(&self) -> Self { - Self { ty: self.ty.clone(), validity: self.validity } + Self { ty: self.ty.clone(), validity: self.validity, is_scrutinee: self.is_scrutinee } } } @@ -1051,7 +1057,7 @@ impl<'p, Cx: TypeCx> Matrix<'p, Cx> { scrut_ty: Cx::Ty, scrut_validity: ValidityConstraint, ) -> Self { - let place_info = PlaceInfo { ty: scrut_ty, validity: scrut_validity }; + let place_info = PlaceInfo { ty: scrut_ty, validity: scrut_validity, is_scrutinee: true }; let mut matrix = Matrix { rows: Vec::with_capacity(arms.len()), place_info: smallvec![place_info], @@ -1446,11 +1452,10 @@ fn collect_overlapping_range_endpoints<'p, Cx: TypeCx>( /// - unspecialization, where we lift the results from the previous step into results for this step /// (using `apply_constructor` and by updating `row.useful` for each parent row). /// This is all explained at the top of the file. -#[instrument(level = "debug", skip(mcx, is_top_level), ret)] +#[instrument(level = "debug", skip(mcx), ret)] fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>( mcx: UsefulnessCtxt<'a, Cx>, matrix: &mut Matrix<'p, Cx>, - is_top_level: bool, ) -> Result, Cx::Error> { debug_assert!(matrix.rows().all(|r| r.len() == matrix.column_count())); @@ -1488,7 +1493,7 @@ fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>( // We treat match scrutinees of type `!` or `EmptyEnum` differently. let is_toplevel_exception = - is_top_level && matches!(ctors_for_ty, ConstructorSet::NoConstructors); + place.is_scrutinee && matches!(ctors_for_ty, ConstructorSet::NoConstructors); // Whether empty patterns are counted as useful or not. We only warn an empty arm unreachable if // it is guaranteed unreachable by the opsem (i.e. if the place is `known_valid`). let empty_arms_are_unreachable = place.validity.is_known_valid() @@ -1517,7 +1522,7 @@ fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>( // Decide what constructors to report. let is_integers = matches!(ctors_for_ty, ConstructorSet::Integers { .. }); - let always_report_all = is_top_level && !is_integers; + let always_report_all = place.is_scrutinee && !is_integers; // Whether we should report "Enum::A and Enum::C are missing" or "_ is missing". let report_individual_missing_ctors = always_report_all || !all_missing; // Which constructors are considered missing. We ensure that `!missing_ctors.is_empty() => @@ -1538,7 +1543,7 @@ fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>( let ctor_is_relevant = matches!(ctor, Constructor::Missing) || missing_ctors.is_empty(); let mut spec_matrix = matrix.specialize_constructor(pcx, &ctor, ctor_is_relevant)?; let mut witnesses = ensure_sufficient_stack(|| { - compute_exhaustiveness_and_usefulness(mcx, &mut spec_matrix, false) + compute_exhaustiveness_and_usefulness(mcx, &mut spec_matrix) })?; // Transform witnesses for `spec_matrix` into witnesses for `matrix`. @@ -1613,8 +1618,7 @@ pub fn compute_match_usefulness<'p, Cx: TypeCx>( ) -> Result, Cx::Error> { let cx = UsefulnessCtxt { tycx }; let mut matrix = Matrix::new(arms, scrut_ty, scrut_validity); - let non_exhaustiveness_witnesses = - compute_exhaustiveness_and_usefulness(cx, &mut matrix, true)?; + let non_exhaustiveness_witnesses = compute_exhaustiveness_and_usefulness(cx, &mut matrix)?; let non_exhaustiveness_witnesses: Vec<_> = non_exhaustiveness_witnesses.single_column(); let arm_usefulness: Vec<_> = arms From a939bad513e6201d7be76c13080fc0cc901bd658 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 29 Jan 2024 20:34:29 +0000 Subject: [PATCH 17/29] Suggest turnging `if let` into irrefutable `let` if appropriate When encountering an `if let` tail expression without an `else` arm for an enum with a single variant, suggest writing an irrefutable `let` binding instead. ``` error[E0317]: `if` may be missing an `else` clause --> $DIR/irrefutable-if-let-without-else.rs:8:5 | LL | fn foo(x: Enum) -> i32 { | --- expected `i32` because of this return type LL | / if let Enum::Variant(value) = x { LL | | value LL | | } | |_____^ expected `i32`, found `()` | = note: `if` expressions without `else` evaluate to `()` = help: consider adding an `else` block that evaluates to the expected type help: consider using an irrefutable `let` binding instead | LL ~ let Enum::Variant(value) = x; LL ~ value | ``` Fix #61788. --- compiler/rustc_hir_typeck/src/_match.rs | 116 +++++++++++++++--- compiler/rustc_hir_typeck/src/expr.rs | 2 +- .../irrefutable-if-let-without-else.fixed | 25 ++++ .../irrefutable-if-let-without-else.rs | 28 +++++ .../irrefutable-if-let-without-else.stderr | 61 +++++++++ 5 files changed, 214 insertions(+), 18 deletions(-) create mode 100644 tests/ui/binding/irrefutable-if-let-without-else.fixed create mode 100644 tests/ui/binding/irrefutable-if-let-without-else.rs create mode 100644 tests/ui/binding/irrefutable-if-let-without-else.stderr diff --git a/compiler/rustc_hir_typeck/src/_match.rs b/compiler/rustc_hir_typeck/src/_match.rs index cf1f232229d51..7ea0469deddee 100644 --- a/compiler/rustc_hir_typeck/src/_match.rs +++ b/compiler/rustc_hir_typeck/src/_match.rs @@ -1,7 +1,11 @@ use crate::coercion::{AsCoercionSite, CoerceMany}; use crate::{Diverges, Expectation, FnCtxt, Needs}; -use rustc_errors::Diagnostic; -use rustc_hir::{self as hir, ExprKind}; +use rustc_errors::{Applicability, Diagnostic}; +use rustc_hir::{ + self as hir, + def::{CtorOf, DefKind, Res}, + ExprKind, PatKind, +}; use rustc_hir_pretty::ty_to_string; use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind}; use rustc_infer::traits::Obligation; @@ -273,7 +277,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { /// Returns `true` if there was an error forcing the coercion to the `()` type. pub(super) fn if_fallback_coercion( &self, - span: Span, + if_span: Span, + cond_expr: &'tcx hir::Expr<'tcx>, then_expr: &'tcx hir::Expr<'tcx>, coercion: &mut CoerceMany<'tcx, '_, T>, ) -> bool @@ -283,29 +288,106 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // If this `if` expr is the parent's function return expr, // the cause of the type coercion is the return type, point at it. (#25228) let hir_id = self.tcx.hir().parent_id(self.tcx.hir().parent_id(then_expr.hir_id)); - let ret_reason = self.maybe_get_coercion_reason(hir_id, span); - let cause = self.cause(span, ObligationCauseCode::IfExpressionWithNoElse); + let ret_reason = self.maybe_get_coercion_reason(hir_id, if_span); + let cause = self.cause(if_span, ObligationCauseCode::IfExpressionWithNoElse); let mut error = false; coercion.coerce_forced_unit( self, &cause, - |err| { - if let Some((span, msg)) = &ret_reason { - err.span_label(*span, msg.clone()); - } else if let ExprKind::Block(block, _) = &then_expr.kind - && let Some(expr) = &block.expr - { - err.span_label(expr.span, "found here"); - } - err.note("`if` expressions without `else` evaluate to `()`"); - err.help("consider adding an `else` block that evaluates to the expected type"); - error = true; - }, + |err| self.explain_if_expr(err, ret_reason, if_span, cond_expr, then_expr, &mut error), false, ); error } + /// Explain why `if` expressions without `else` evaluate to `()` and detect likely irrefutable + /// `if let PAT = EXPR {}` expressions that could be turned into `let PAT = EXPR;`. + fn explain_if_expr( + &self, + err: &mut Diagnostic, + ret_reason: Option<(Span, String)>, + if_span: Span, + cond_expr: &'tcx hir::Expr<'tcx>, + then_expr: &'tcx hir::Expr<'tcx>, + error: &mut bool, + ) { + if let Some((if_span, msg)) = ret_reason { + err.span_label(if_span, msg.clone()); + } else if let ExprKind::Block(block, _) = then_expr.kind + && let Some(expr) = block.expr + { + err.span_label(expr.span, "found here"); + } + err.note("`if` expressions without `else` evaluate to `()`"); + err.help("consider adding an `else` block that evaluates to the expected type"); + *error = true; + if let ExprKind::Let(hir::Let { span, pat, init, .. }) = cond_expr.kind + && let ExprKind::Block(block, _) = then_expr.kind + // Refutability checks occur on the MIR, so we approximate it here by checking + // if we have an enum with a single variant or a struct in the pattern. + && let PatKind::TupleStruct(qpath, ..) | PatKind::Struct(qpath, ..) = pat.kind + && let hir::QPath::Resolved(_, path) = qpath + { + match path.res { + Res::Def(DefKind::Ctor(CtorOf::Struct, _), _) => { + // Structs are always irrefutable. Their fields might not be, but we + // don't check for that here, it's only an approximation. + } + Res::Def(DefKind::Ctor(CtorOf::Variant, _), def_id) + if self + .tcx + .adt_def(self.tcx.parent(self.tcx.parent(def_id))) + .variants() + .len() + == 1 => + { + // There's only a single variant in the `enum`, so we can suggest the + // irrefutable `let` instead of `if let`. + } + _ => return, + } + + let mut sugg = vec![ + // Remove the `if` + (if_span.until(*span), String::new()), + ]; + match (block.stmts, block.expr) { + ([first, ..], Some(expr)) => { + let padding = self + .tcx + .sess + .source_map() + .indentation_before(first.span) + .unwrap_or_else(|| String::new()); + sugg.extend([ + (init.span.between(first.span), format!(";\n{padding}")), + (expr.span.shrink_to_hi().with_hi(block.span.hi()), String::new()), + ]); + } + ([], Some(expr)) => { + let padding = self + .tcx + .sess + .source_map() + .indentation_before(expr.span) + .unwrap_or_else(|| String::new()); + sugg.extend([ + (init.span.between(expr.span), format!(";\n{padding}")), + (expr.span.shrink_to_hi().with_hi(block.span.hi()), String::new()), + ]); + } + // If there's no value in the body, then the `if` expression would already + // be of type `()`, so checking for those cases is unnecessary. + (_, None) => return, + } + err.multipart_suggestion( + "consider using an irrefutable `let` binding instead", + sugg, + Applicability::MaybeIncorrect, + ); + } + } + pub fn maybe_get_coercion_reason( &self, hir_id: hir::HirId, diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs index 2bbef11fa2450..4f96b1fde1a6d 100644 --- a/compiler/rustc_hir_typeck/src/expr.rs +++ b/compiler/rustc_hir_typeck/src/expr.rs @@ -1118,7 +1118,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // We won't diverge unless both branches do (or the condition does). self.diverges.set(cond_diverges | then_diverges & else_diverges); } else { - self.if_fallback_coercion(sp, then_expr, &mut coerce); + self.if_fallback_coercion(sp, cond_expr, then_expr, &mut coerce); // If the condition is false we can't diverge. self.diverges.set(cond_diverges); diff --git a/tests/ui/binding/irrefutable-if-let-without-else.fixed b/tests/ui/binding/irrefutable-if-let-without-else.fixed new file mode 100644 index 0000000000000..3d7f4695ca864 --- /dev/null +++ b/tests/ui/binding/irrefutable-if-let-without-else.fixed @@ -0,0 +1,25 @@ +// run-rustfix +enum Enum { + Variant(i32), +} +struct Struct(i32); + +fn foo(x: Enum) -> i32 { + let Enum::Variant(value) = x; + value +} +fn bar(x: Enum) -> i32 { + let Enum::Variant(value) = x; + let x = value + 1; + x +} +fn baz(x: Struct) -> i32 { + let Struct(value) = x; + let x = value + 1; + x +} +fn main() { + let _ = foo(Enum::Variant(42)); + let _ = bar(Enum::Variant(42)); + let _ = baz(Struct(42)); +} diff --git a/tests/ui/binding/irrefutable-if-let-without-else.rs b/tests/ui/binding/irrefutable-if-let-without-else.rs new file mode 100644 index 0000000000000..5aaf4ace3f821 --- /dev/null +++ b/tests/ui/binding/irrefutable-if-let-without-else.rs @@ -0,0 +1,28 @@ +// run-rustfix +enum Enum { + Variant(i32), +} +struct Struct(i32); + +fn foo(x: Enum) -> i32 { + if let Enum::Variant(value) = x { //~ ERROR `if` may be missing an `else` clause + value + } +} +fn bar(x: Enum) -> i32 { + if let Enum::Variant(value) = x { //~ ERROR `if` may be missing an `else` clause + let x = value + 1; + x + } +} +fn baz(x: Struct) -> i32 { + if let Struct(value) = x { //~ ERROR `if` may be missing an `else` clause + let x = value + 1; + x + } +} +fn main() { + let _ = foo(Enum::Variant(42)); + let _ = bar(Enum::Variant(42)); + let _ = baz(Struct(42)); +} diff --git a/tests/ui/binding/irrefutable-if-let-without-else.stderr b/tests/ui/binding/irrefutable-if-let-without-else.stderr new file mode 100644 index 0000000000000..e234cfdd945f3 --- /dev/null +++ b/tests/ui/binding/irrefutable-if-let-without-else.stderr @@ -0,0 +1,61 @@ +error[E0317]: `if` may be missing an `else` clause + --> $DIR/irrefutable-if-let-without-else.rs:8:5 + | +LL | fn foo(x: Enum) -> i32 { + | --- expected `i32` because of this return type +LL | / if let Enum::Variant(value) = x { +LL | | value +LL | | } + | |_____^ expected `i32`, found `()` + | + = note: `if` expressions without `else` evaluate to `()` + = help: consider adding an `else` block that evaluates to the expected type +help: consider using an irrefutable `let` binding instead + | +LL ~ let Enum::Variant(value) = x; +LL ~ value + | + +error[E0317]: `if` may be missing an `else` clause + --> $DIR/irrefutable-if-let-without-else.rs:13:5 + | +LL | fn bar(x: Enum) -> i32 { + | --- expected `i32` because of this return type +LL | / if let Enum::Variant(value) = x { +LL | | let x = value + 1; +LL | | x +LL | | } + | |_____^ expected `i32`, found `()` + | + = note: `if` expressions without `else` evaluate to `()` + = help: consider adding an `else` block that evaluates to the expected type +help: consider using an irrefutable `let` binding instead + | +LL ~ let Enum::Variant(value) = x; +LL ~ let x = value + 1; +LL ~ x + | + +error[E0317]: `if` may be missing an `else` clause + --> $DIR/irrefutable-if-let-without-else.rs:19:5 + | +LL | fn baz(x: Struct) -> i32 { + | --- expected `i32` because of this return type +LL | / if let Struct(value) = x { +LL | | let x = value + 1; +LL | | x +LL | | } + | |_____^ expected `i32`, found `()` + | + = note: `if` expressions without `else` evaluate to `()` + = help: consider adding an `else` block that evaluates to the expected type +help: consider using an irrefutable `let` binding instead + | +LL ~ let Struct(value) = x; +LL ~ let x = value + 1; +LL ~ x + | + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0317`. From aef18c9bfb8c7844b95f6d950133f61b0fa2158b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 29 Jan 2024 18:54:29 +0000 Subject: [PATCH 18/29] Mark "unused binding" suggestion as maybe incorrect Ignoring unused bindings should be a determination made by a human, `rustfix` shouldn't auto-apply the suggested change. Fix #54196. --- compiler/rustc_passes/src/errors.rs | 4 ++-- tests/ui/lint/future-incompat-json-test.stderr | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_passes/src/errors.rs b/compiler/rustc_passes/src/errors.rs index 77bfe57e3706f..5db2369ad76c2 100644 --- a/compiler/rustc_passes/src/errors.rs +++ b/compiler/rustc_passes/src/errors.rs @@ -1739,7 +1739,7 @@ pub struct UnusedVariableTryPrefix { #[derive(Subdiagnostic)] pub enum UnusedVariableSugg { - #[multipart_suggestion(passes_suggestion, applicability = "machine-applicable")] + #[multipart_suggestion(passes_suggestion, applicability = "maybe-incorrect")] TryPrefixSugg { #[suggestion_part(code = "_{name}")] spans: Vec, @@ -1778,7 +1778,7 @@ pub struct UnusedVarTryIgnore { } #[derive(Subdiagnostic)] -#[multipart_suggestion(passes_suggestion, applicability = "machine-applicable")] +#[multipart_suggestion(passes_suggestion, applicability = "maybe-incorrect")] pub struct UnusedVarTryIgnoreSugg { #[suggestion_part(code = "{name}: _")] pub shorthands: Vec, diff --git a/tests/ui/lint/future-incompat-json-test.stderr b/tests/ui/lint/future-incompat-json-test.stderr index c4ab5a00d16cc..18fc3f17f0020 100644 --- a/tests/ui/lint/future-incompat-json-test.stderr +++ b/tests/ui/lint/future-incompat-json-test.stderr @@ -1,4 +1,4 @@ -{"$message_type":"future_incompat","future_incompat_report":[{"diagnostic":{"$message_type":"diagnostic","message":"unused variable: `x`","code":{"code":"unused_variables","explanation":null},"level":"warning","spans":[{"file_name":"$DIR/future-incompat-json-test.rs","byte_start":338,"byte_end":339,"line_start":9,"line_end":9,"column_start":9,"column_end":10,"is_primary":true,"text":[{"text":" let x = 1;","highlight_start":9,"highlight_end":10}],"label":null,"suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[{"message":"`-A unused-variables` implied by `-A unused`","code":null,"level":"note","spans":[],"children":[],"rendered":null},{"message":"to override `-A unused` add `#[allow(unused_variables)]`","code":null,"level":"help","spans":[],"children":[],"rendered":null},{"message":"if this is intentional, prefix it with an underscore","code":null,"level":"help","spans":[{"file_name":"$DIR/future-incompat-json-test.rs","byte_start":338,"byte_end":339,"line_start":9,"line_end":9,"column_start":9,"column_end":10,"is_primary":true,"text":[{"text":" let x = 1;","highlight_start":9,"highlight_end":10}],"label":null,"suggested_replacement":"_x","suggestion_applicability":"MachineApplicable","expansion":null}],"children":[],"rendered":null}],"rendered":"warning: unused variable: `x` +{"$message_type":"future_incompat","future_incompat_report":[{"diagnostic":{"$message_type":"diagnostic","message":"unused variable: `x`","code":{"code":"unused_variables","explanation":null},"level":"warning","spans":[{"file_name":"$DIR/future-incompat-json-test.rs","byte_start":338,"byte_end":339,"line_start":9,"line_end":9,"column_start":9,"column_end":10,"is_primary":true,"text":[{"text":" let x = 1;","highlight_start":9,"highlight_end":10}],"label":null,"suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[{"message":"`-A unused-variables` implied by `-A unused`","code":null,"level":"note","spans":[],"children":[],"rendered":null},{"message":"to override `-A unused` add `#[allow(unused_variables)]`","code":null,"level":"help","spans":[],"children":[],"rendered":null},{"message":"if this is intentional, prefix it with an underscore","code":null,"level":"help","spans":[{"file_name":"$DIR/future-incompat-json-test.rs","byte_start":338,"byte_end":339,"line_start":9,"line_end":9,"column_start":9,"column_end":10,"is_primary":true,"text":[{"text":" let x = 1;","highlight_start":9,"highlight_end":10}],"label":null,"suggested_replacement":"_x","suggestion_applicability":"MaybeIncorrect","expansion":null}],"children":[],"rendered":null}],"rendered":"warning: unused variable: `x` --> $DIR/future-incompat-json-test.rs:9:9 | LL | let x = 1; From 8d1c20a539b72abdbe1ede872f19e0a1a6f34be4 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 6 Feb 2024 17:13:32 +1100 Subject: [PATCH 19/29] Remove return value from `emit_stashed_diagnostics`. It's never used. --- compiler/rustc_errors/src/lib.rs | 9 +++------ compiler/rustc_session/src/session.rs | 2 +- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index a4112d717d029..7c62e3aa42281 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -708,7 +708,7 @@ impl DiagCtxt { } /// Emit all stashed diagnostics. - pub fn emit_stashed_diagnostics(&self) -> Option { + pub fn emit_stashed_diagnostics(&self) { self.inner.borrow_mut().emit_stashed_diagnostics() } @@ -1216,9 +1216,8 @@ impl DiagCtxt { // `DiagCtxtInner::foo`. impl DiagCtxtInner { /// Emit all stashed diagnostics. - fn emit_stashed_diagnostics(&mut self) -> Option { + fn emit_stashed_diagnostics(&mut self) { let has_errors = self.has_errors(); - let mut reported = None; for (_, diag) in std::mem::take(&mut self.stashed_diagnostics).into_iter() { // Decrement the count tracking the stash; emitting will increment it. if diag.is_error() { @@ -1235,10 +1234,8 @@ impl DiagCtxtInner { continue; } } - let reported_this = self.emit_diagnostic(diag); - reported = reported.or(reported_this); + self.emit_diagnostic(diag); } - reported } fn emit_diagnostic(&mut self, mut diagnostic: Diagnostic) -> Option { diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index f6af5a4f87e07..ec14385f40c62 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -315,7 +315,7 @@ impl Session { pub fn compile_status(&self) -> Result<(), ErrorGuaranteed> { // We must include lint errors here. if let Some(reported) = self.dcx().has_errors_or_lint_errors() { - let _ = self.dcx().emit_stashed_diagnostics(); + self.dcx().emit_stashed_diagnostics(); Err(reported) } else { Ok(()) From 7fb4512ee8d5aea8e362dd736fe08586db369416 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Tue, 6 Feb 2024 23:31:52 +0300 Subject: [PATCH 20/29] fix `llvm_out` to use the correct LLVM root When `download-ci-llvm` is enabled, `llvm_out` ends up with the error below due to an incorrect path on cross-compilations. This change fixes that. ``` failed to execute command: "/rust/build/x86_64-unknown-linux-gnu/llvm/build/bin/llvm-config" "--version" ERROR: No such file or directory (os error 2) ``` Signed-off-by: onur-ozkan --- src/bootstrap/src/core/build_steps/llvm.rs | 2 +- src/bootstrap/src/lib.rs | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/llvm.rs b/src/bootstrap/src/core/build_steps/llvm.rs index 4b2d3e9ab4b75..afbbbb5bd269d 100644 --- a/src/bootstrap/src/core/build_steps/llvm.rs +++ b/src/bootstrap/src/core/build_steps/llvm.rs @@ -98,7 +98,7 @@ pub fn prebuilt_llvm_config( let out_dir = builder.llvm_out(target); let mut llvm_config_ret_dir = builder.llvm_out(builder.config.build); - if !builder.config.build.is_msvc() || builder.ninja() { + if (!builder.config.build.is_msvc() || builder.ninja()) && !builder.config.llvm_from_ci { llvm_config_ret_dir.push("build"); } llvm_config_ret_dir.push("bin"); diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 1336abf6c7aba..c6c929c90b3d3 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -796,12 +796,16 @@ impl Build { self.stage_out(compiler, mode).join(&*target.triple).join(self.cargo_dir()) } - /// Root output directory for LLVM compiled for `target` + /// Root output directory of LLVM for `target` /// /// Note that if LLVM is configured externally then the directory returned /// will likely be empty. fn llvm_out(&self, target: TargetSelection) -> PathBuf { - self.out.join(&*target.triple).join("llvm") + if self.config.llvm_from_ci && self.config.build == target { + self.config.ci_llvm_root() + } else { + self.out.join(&*target.triple).join("llvm") + } } fn lld_out(&self, target: TargetSelection) -> PathBuf { From 14dda5fdfb81e61b2acc85f27609f879aeaa166d Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Tue, 6 Feb 2024 21:03:07 -0500 Subject: [PATCH 21/29] Don't use bashism in checktools.sh --- src/ci/docker/host-x86_64/x86_64-gnu-tools/checktools.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ci/docker/host-x86_64/x86_64-gnu-tools/checktools.sh b/src/ci/docker/host-x86_64/x86_64-gnu-tools/checktools.sh index cc0c658aabd2b..f5c426a717f78 100755 --- a/src/ci/docker/host-x86_64/x86_64-gnu-tools/checktools.sh +++ b/src/ci/docker/host-x86_64/x86_64-gnu-tools/checktools.sh @@ -30,7 +30,7 @@ python3 "$X_PY" test --stage 2 src/tools/rustfmt # We set the GC interval to the shortest possible value (0 would be off) to increase the chance # that bugs which only surface when the GC runs at a specific time are more likely to cause CI to fail. # This significantly increases the runtime of our test suite, or we'd do this in PR CI too. -if [[ -z "${PR_CI_JOB:-}" ]]; then +if [ -z "${PR_CI_JOB:-}" ]; then MIRIFLAGS=-Zmiri-provenance-gc=1 python3 "$X_PY" test --stage 2 src/tools/miri else python3 "$X_PY" test --stage 2 src/tools/miri From 63cc3c7b8f4df8076422ebc74260eccaebb6897c Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Wed, 7 Feb 2024 09:47:34 +0300 Subject: [PATCH 22/29] test `llvm_out` behaviour Signed-off-by: onur-ozkan --- src/bootstrap/src/core/builder/tests.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/bootstrap/src/core/builder/tests.rs b/src/bootstrap/src/core/builder/tests.rs index 700ebcf5e3741..2cbebbcf4e2af 100644 --- a/src/bootstrap/src/core/builder/tests.rs +++ b/src/bootstrap/src/core/builder/tests.rs @@ -524,6 +524,23 @@ mod dist { ); } + #[test] + fn llvm_out_behaviour() { + let mut config = configure(&["A"], &["B"]); + config.llvm_from_ci = true; + let build = Build::new(config.clone()); + + let target = TargetSelection::from_user("A"); + assert!(build.llvm_out(target).ends_with("ci-llvm")); + let target = TargetSelection::from_user("B"); + assert!(build.llvm_out(target).ends_with("llvm")); + + config.llvm_from_ci = false; + let build = Build::new(config.clone()); + let target = TargetSelection::from_user("A"); + assert!(build.llvm_out(target).ends_with("llvm")); + } + #[test] fn build_with_empty_host() { let config = configure(&[], &["C"]); From a59d00674a6b04ba1eaa5e0c3cf00ddb0346296b Mon Sep 17 00:00:00 2001 From: SparrowLii Date: Wed, 7 Feb 2024 15:26:57 +0800 Subject: [PATCH 23/29] Add parallel rustc ui tests --- src/tools/tidy/src/ui_tests.rs | 2 +- .../cache-after-waiting-issue-111528.rs | 16 ++++++++++++++ .../cache-after-waiting-issue-111528.stderr | 8 +++++++ .../export-symbols-deadlock-issue-118205-2.rs | 7 ++++++ .../export-symbols-deadlock-issue-118205.rs | 22 +++++++++++++++++++ tests/ui/parallel-rustc/hello_world.rs | 6 +++++ .../read-stolen-value-issue-111520.rs | 18 +++++++++++++++ 7 files changed, 78 insertions(+), 1 deletion(-) create mode 100644 tests/ui/parallel-rustc/cache-after-waiting-issue-111528.rs create mode 100644 tests/ui/parallel-rustc/cache-after-waiting-issue-111528.stderr create mode 100644 tests/ui/parallel-rustc/export-symbols-deadlock-issue-118205-2.rs create mode 100644 tests/ui/parallel-rustc/export-symbols-deadlock-issue-118205.rs create mode 100644 tests/ui/parallel-rustc/hello_world.rs create mode 100644 tests/ui/parallel-rustc/read-stolen-value-issue-111520.rs diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs index e7b1ccf6a0299..4a44c40debd22 100644 --- a/src/tools/tidy/src/ui_tests.rs +++ b/src/tools/tidy/src/ui_tests.rs @@ -15,7 +15,7 @@ use std::path::{Path, PathBuf}; const ENTRY_LIMIT: usize = 900; // FIXME: The following limits should be reduced eventually. const ISSUES_ENTRY_LIMIT: usize = 1819; -const ROOT_ENTRY_LIMIT: usize = 870; +const ROOT_ENTRY_LIMIT: usize = 871; const EXPECTED_TEST_FILE_EXTENSIONS: &[&str] = &[ "rs", // test source files diff --git a/tests/ui/parallel-rustc/cache-after-waiting-issue-111528.rs b/tests/ui/parallel-rustc/cache-after-waiting-issue-111528.rs new file mode 100644 index 0000000000000..148a59240f99a --- /dev/null +++ b/tests/ui/parallel-rustc/cache-after-waiting-issue-111528.rs @@ -0,0 +1,16 @@ +// compile-flags: -Z threads=16 +// build-fail + +#![crate_type="rlib"] +#![allow(warnings)] + +#[export_name="fail"] +pub fn a() { +} + +#[export_name="fail"] +pub fn b() { +//~^ Error symbol `fail` is already defined +} + +fn main() {} diff --git a/tests/ui/parallel-rustc/cache-after-waiting-issue-111528.stderr b/tests/ui/parallel-rustc/cache-after-waiting-issue-111528.stderr new file mode 100644 index 0000000000000..7963165e31b0a --- /dev/null +++ b/tests/ui/parallel-rustc/cache-after-waiting-issue-111528.stderr @@ -0,0 +1,8 @@ +error: symbol `fail` is already defined + --> $DIR/cache-after-waiting-issue-111528.rs:12:1 + | +LL | pub fn b() { + | ^^^^^^^^^^ + +error: aborting due to 1 previous error + diff --git a/tests/ui/parallel-rustc/export-symbols-deadlock-issue-118205-2.rs b/tests/ui/parallel-rustc/export-symbols-deadlock-issue-118205-2.rs new file mode 100644 index 0000000000000..8240b249018e2 --- /dev/null +++ b/tests/ui/parallel-rustc/export-symbols-deadlock-issue-118205-2.rs @@ -0,0 +1,7 @@ +// compile-flags:-C extra-filename=-1 -Z threads=16 +// no-prefer-dynamic +// build-pass +#![crate_name = "crateresolve1"] +#![crate_type = "lib"] + +pub fn f() -> isize { 10 } diff --git a/tests/ui/parallel-rustc/export-symbols-deadlock-issue-118205.rs b/tests/ui/parallel-rustc/export-symbols-deadlock-issue-118205.rs new file mode 100644 index 0000000000000..691c36cfc9ec4 --- /dev/null +++ b/tests/ui/parallel-rustc/export-symbols-deadlock-issue-118205.rs @@ -0,0 +1,22 @@ +// compile-flags: -Z threads=16 +// build-pass + +pub static GLOBAL: isize = 3; + +static GLOBAL0: isize = 4; + +pub static GLOBAL2: &'static isize = &GLOBAL0; + +pub fn verify_same(a: &'static isize) { + let a = a as *const isize as usize; + let b = &GLOBAL as *const isize as usize; + assert_eq!(a, b); +} + +pub fn verify_same2(a: &'static isize) { + let a = a as *const isize as usize; + let b = GLOBAL2 as *const isize as usize; + assert_eq!(a, b); +} + +fn main() {} diff --git a/tests/ui/parallel-rustc/hello_world.rs b/tests/ui/parallel-rustc/hello_world.rs new file mode 100644 index 0000000000000..53e95c890ef52 --- /dev/null +++ b/tests/ui/parallel-rustc/hello_world.rs @@ -0,0 +1,6 @@ +// compile-flags: -Z threads=8 +// run-pass + +fn main() { + println!("Hello world!"); +} diff --git a/tests/ui/parallel-rustc/read-stolen-value-issue-111520.rs b/tests/ui/parallel-rustc/read-stolen-value-issue-111520.rs new file mode 100644 index 0000000000000..1907348cf09dc --- /dev/null +++ b/tests/ui/parallel-rustc/read-stolen-value-issue-111520.rs @@ -0,0 +1,18 @@ +// compile-flags: -Z threads=16 +// run-pass + +#[repr(transparent)] +struct Sched { + i: i32, +} +impl Sched { + extern "C" fn get(self) -> i32 { self.i } +} + +fn main() { + let s = Sched { i: 4 }; + let f = || -> i32 { + s.get() + }; + println!("f: {}", f()); +} From e55df623ead33023fe6c4488064e5d5e4e141b9e Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 6 Feb 2024 17:40:11 +1100 Subject: [PATCH 24/29] Remove an `unchecked_claim_error_was_emitted` call. When `catch_fatal_errors` catches a `FatalErrorMarker`, it returns an `ErrorGuaranteed` that is conjured out of thin air with `unchecked_claim_error_was_emitted`. But that `ErrorGuaranteed` is never used. This commit changes it to instead conjure a `FatalError` out of thin air. (A non-deprecated action!) This makes more sense because `FatalError` and `FatalErrorMarker` are a natural pairing -- a `FatalErrorMarker` is created by calling `FatalError::raise`, so this is effectively getting back the original `FatalError`. This requires a tiny change in `catch_with_exit_code`. The old result of the `catch_fatal_errors` call there was `Result, ErrorGuaranteed>` which could be `flatten`ed into `Result<(), ErrorGuaranteed>`. The new result of the `catch_fatal_errors` calls is `Result, FatalError>`, which can't be `flatten`ed but is still easily matched for the success case. --- compiler/rustc_driver_impl/src/lib.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_driver_impl/src/lib.rs b/compiler/rustc_driver_impl/src/lib.rs index 5903c43ae98af..519bde988200e 100644 --- a/compiler/rustc_driver_impl/src/lib.rs +++ b/compiler/rustc_driver_impl/src/lib.rs @@ -24,7 +24,9 @@ use rustc_data_structures::profiling::{ get_resident_set_size, print_time_passes_entry, TimePassesFormat, }; use rustc_errors::registry::Registry; -use rustc_errors::{markdown, ColorConfig, DiagCtxt, ErrCode, ErrorGuaranteed, PResult}; +use rustc_errors::{ + markdown, ColorConfig, DiagCtxt, ErrCode, ErrorGuaranteed, FatalError, PResult, +}; use rustc_feature::find_gated_cfg; use rustc_interface::util::{self, collect_crate_types, get_codegen_backend}; use rustc_interface::{interface, Queries}; @@ -1231,11 +1233,10 @@ fn parse_crate_attrs<'a>(sess: &'a Session) -> PResult<'a, ast::AttrVec> { /// The compiler currently unwinds with a special sentinel value to abort /// compilation on fatal errors. This function catches that sentinel and turns /// the panic into a `Result` instead. -pub fn catch_fatal_errors R, R>(f: F) -> Result { +pub fn catch_fatal_errors R, R>(f: F) -> Result { catch_unwind(panic::AssertUnwindSafe(f)).map_err(|value| { if value.is::() { - #[allow(deprecated)] - ErrorGuaranteed::unchecked_claim_error_was_emitted() + FatalError } else { panic::resume_unwind(value); } @@ -1245,9 +1246,9 @@ pub fn catch_fatal_errors R, R>(f: F) -> Result interface::Result<()>) -> i32 { - match catch_fatal_errors(f).flatten() { - Ok(()) => EXIT_SUCCESS, - Err(_) => EXIT_FAILURE, + match catch_fatal_errors(f) { + Ok(Ok(())) => EXIT_SUCCESS, + _ => EXIT_FAILURE, } } From e6794ddfb00e14b29b3befc5bc054a15094321b7 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 6 Feb 2024 20:47:47 +1100 Subject: [PATCH 25/29] rustdoc: make `main` more like rustc's. By making non-unicode arguments a fatal error instead of a warning, we don't need to handle what comes after, which avoids the need for an `unchecked_claim_error_was_emitted` call. --- src/librustdoc/lib.rs | 30 ++++++++++-------------------- 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index 8c10f14116a0c..0fe3adadba347 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -177,13 +177,16 @@ pub fn main() { init_logging(&early_dcx); rustc_driver::init_logger(&early_dcx, rustc_log::LoggerConfig::from_env("RUSTDOC_LOG")); - let exit_code = rustc_driver::catch_with_exit_code(|| match get_args(&early_dcx) { - Some(args) => main_args(&mut early_dcx, &args, using_internal_features), - _ => - { - #[allow(deprecated)] - Err(ErrorGuaranteed::unchecked_claim_error_was_emitted()) - } + let exit_code = rustc_driver::catch_with_exit_code(|| { + let args = env::args_os() + .enumerate() + .map(|(i, arg)| { + arg.into_string().unwrap_or_else(|arg| { + early_dcx.early_fatal(format!("argument {i} is not valid Unicode: {arg:?}")) + }) + }) + .collect::>(); + main_args(&mut early_dcx, &args, using_internal_features) }); process::exit(exit_code); } @@ -219,19 +222,6 @@ fn init_logging(early_dcx: &EarlyDiagCtxt) { tracing::subscriber::set_global_default(subscriber).unwrap(); } -fn get_args(early_dcx: &EarlyDiagCtxt) -> Option> { - env::args_os() - .enumerate() - .map(|(i, arg)| { - arg.into_string() - .map_err(|arg| { - early_dcx.early_warn(format!("Argument {i} is not valid Unicode: {arg:?}")); - }) - .ok() - }) - .collect() -} - fn opts() -> Vec { let stable: fn(_, fn(&mut getopts::Options) -> &mut _) -> _ = RustcOptGroup::stable; let unstable: fn(_, fn(&mut getopts::Options) -> &mut _) -> _ = RustcOptGroup::unstable; From 83adf883a20fc7531e886996ce07b8555c943fba Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 7 Feb 2024 09:01:49 +1100 Subject: [PATCH 26/29] rustdoc: remove `unchecked_claim_error_was_emitted` call in `main_args`. `main_args` calls `from_matches`, which does lots of initialization. If anything goes wrong, `from_matches` emits an error message and returns `Err(1)` (or `Err(3)`). `main_args` then turns the `Err(1)` into `Err(ErrorGuaranteed)`, because that's what `catch_with_exit_code` requires on error. But `catch_with_exit_code` doesn't do anything with the `ErrorGuaranteed`, it just exits with `EXIT_FAILURE`. We can avoid the creation of the `ErrorGuaranteed` (which requires an undesirable `unchecked_claim_error_was_emitted` call), by changing `from_matches` to instead eagerly abort if anything goes wrong. The behaviour from the user's point of view is the same: an early abort with an `EXIT_FAILURE` exit code. And we can also simplify `from_matches` to return an `Option` instead of a `Result`: - Old `Err(0)` case --> `None` - Old `Err(_)` case --> fatal error. This requires similar changes to `ScrapeExamplesOptions::new` and `load_call_locations`. --- src/librustdoc/config.rs | 96 +++++++++++-------------------- src/librustdoc/lib.rs | 11 +--- src/librustdoc/scrape_examples.rs | 49 +++++++--------- 3 files changed, 54 insertions(+), 102 deletions(-) diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index 3f7f9270b2da0..c46047aa0dbad 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -323,20 +323,20 @@ impl Options { early_dcx: &mut EarlyDiagCtxt, matches: &getopts::Matches, args: Vec, - ) -> Result<(Options, RenderOptions), i32> { + ) -> Option<(Options, RenderOptions)> { // Check for unstable options. nightly_options::check_nightly_options(early_dcx, matches, &opts()); if args.is_empty() || matches.opt_present("h") || matches.opt_present("help") { crate::usage("rustdoc"); - return Err(0); + return None; } else if matches.opt_present("version") { rustc_driver::version!(&early_dcx, "rustdoc", matches); - return Err(0); + return None; } if rustc_driver::describe_flag_categories(early_dcx, &matches) { - return Err(0); + return None; } let color = config::parse_color(early_dcx, matches); @@ -382,7 +382,7 @@ impl Options { } } - return Err(0); + return None; } let mut emit = Vec::new(); @@ -390,10 +390,7 @@ impl Options { for kind in list.split(',') { match kind.parse() { Ok(kind) => emit.push(kind), - Err(()) => { - dcx.err(format!("unrecognized emission type: {kind}")); - return Err(1); - } + Err(()) => dcx.fatal(format!("unrecognized emission type: {kind}")), } } } @@ -403,7 +400,7 @@ impl Options { && !matches.opt_present("show-coverage") && !nightly_options::is_unstable_enabled(matches) { - early_dcx.early_fatal( + dcx.fatal( "the -Z unstable-options flag must be passed to enable --output-format for documentation generation (see https://github.com/rust-lang/rust/issues/76578)", ); } @@ -420,10 +417,7 @@ impl Options { } let paths = match theme::load_css_paths(content) { Ok(p) => p, - Err(e) => { - dcx.err(e); - return Err(1); - } + Err(e) => dcx.fatal(e), }; let mut errors = 0; @@ -442,9 +436,9 @@ impl Options { } } if errors != 0 { - return Err(1); + dcx.fatal("[check-theme] one or more tests failed"); } - return Err(0); + return None; } let (lint_opts, describe_lints, lint_cap) = get_cmd_lint_options(early_dcx, matches); @@ -452,11 +446,9 @@ impl Options { let input = PathBuf::from(if describe_lints { "" // dummy, this won't be used } else if matches.free.is_empty() { - dcx.err("missing file operand"); - return Err(1); + dcx.fatal("missing file operand"); } else if matches.free.len() > 1 { - dcx.err("too many file operands"); - return Err(1); + dcx.fatal("too many file operands"); } else { &matches.free[0] }); @@ -466,10 +458,7 @@ impl Options { let externs = parse_externs(early_dcx, matches, &unstable_opts); let extern_html_root_urls = match parse_extern_html_roots(matches) { Ok(ex) => ex, - Err(err) => { - dcx.err(err); - return Err(1); - } + Err(err) => dcx.fatal(err), }; let default_settings: Vec> = vec![ @@ -526,16 +515,14 @@ impl Options { let no_run = matches.opt_present("no-run"); if !should_test && no_run { - dcx.err("the `--test` flag must be passed to enable `--no-run`"); - return Err(1); + dcx.fatal("the `--test` flag must be passed to enable `--no-run`"); } let out_dir = matches.opt_str("out-dir").map(|s| PathBuf::from(&s)); let output = matches.opt_str("output").map(|s| PathBuf::from(&s)); let output = match (out_dir, output) { (Some(_), Some(_)) => { - dcx.err("cannot use both 'out-dir' and 'output' at once"); - return Err(1); + dcx.fatal("cannot use both 'out-dir' and 'output' at once"); } (Some(out_dir), None) => out_dir, (None, Some(output)) => output, @@ -549,8 +536,7 @@ impl Options { if let Some(ref p) = extension_css { if !p.is_file() { - dcx.err("option --extend-css argument must be a file"); - return Err(1); + dcx.fatal("option --extend-css argument must be a file"); } } @@ -566,31 +552,25 @@ impl Options { } let paths = match theme::load_css_paths(content) { Ok(p) => p, - Err(e) => { - dcx.err(e); - return Err(1); - } + Err(e) => dcx.fatal(e), }; for (theme_file, theme_s) in matches.opt_strs("theme").iter().map(|s| (PathBuf::from(&s), s.to_owned())) { if !theme_file.is_file() { - dcx.struct_err(format!("invalid argument: \"{theme_s}\"")) + dcx.struct_fatal(format!("invalid argument: \"{theme_s}\"")) .with_help("arguments to --theme must be files") .emit(); - return Err(1); } if theme_file.extension() != Some(OsStr::new("css")) { - dcx.struct_err(format!("invalid argument: \"{theme_s}\"")) + dcx.struct_fatal(format!("invalid argument: \"{theme_s}\"")) .with_help("arguments to --theme must have a .css extension") .emit(); - return Err(1); } let (success, ret) = theme::test_theme_against(&theme_file, &paths, &dcx); if !success { - dcx.err(format!("error loading theme file: \"{theme_s}\"")); - return Err(1); + dcx.fatal(format!("error loading theme file: \"{theme_s}\"")); } else if !ret.is_empty() { dcx.struct_warn(format!( "theme file \"{theme_s}\" is missing CSS rules from the default theme", @@ -620,22 +600,18 @@ impl Options { edition, &None, ) else { - return Err(3); + dcx.fatal("`ExternalHtml::load` failed"); }; match matches.opt_str("r").as_deref() { Some("rust") | None => {} - Some(s) => { - dcx.err(format!("unknown input format: {s}")); - return Err(1); - } + Some(s) => dcx.fatal(format!("unknown input format: {s}")), } let index_page = matches.opt_str("index-page").map(|s| PathBuf::from(&s)); if let Some(ref index_page) = index_page { if !index_page.is_file() { - dcx.err("option `--index-page` argument must be a file"); - return Err(1); + dcx.fatal("option `--index-page` argument must be a file"); } } @@ -646,8 +622,7 @@ impl Options { let crate_types = match parse_crate_types_from_list(matches.opt_strs("crate-type")) { Ok(types) => types, Err(e) => { - dcx.err(format!("unknown crate type: {e}")); - return Err(1); + dcx.fatal(format!("unknown crate type: {e}")); } }; @@ -655,18 +630,13 @@ impl Options { Some(s) => match OutputFormat::try_from(s.as_str()) { Ok(out_fmt) => { if !out_fmt.is_json() && show_coverage { - dcx.struct_err( + dcx.fatal( "html output format isn't supported for the --show-coverage option", - ) - .emit(); - return Err(1); + ); } out_fmt } - Err(e) => { - dcx.err(e); - return Err(1); - } + Err(e) => dcx.fatal(e), }, None => OutputFormat::default(), }; @@ -709,16 +679,14 @@ impl Options { let html_no_source = matches.opt_present("html-no-source"); if generate_link_to_definition && (show_coverage || output_format != OutputFormat::Html) { - dcx.struct_err( + dcx.fatal( "--generate-link-to-definition option can only be used with HTML output format", - ) - .emit(); - return Err(1); + ); } - let scrape_examples_options = ScrapeExamplesOptions::new(matches, &dcx)?; + let scrape_examples_options = ScrapeExamplesOptions::new(matches, &dcx); let with_examples = matches.opt_strs("with-examples"); - let call_locations = crate::scrape_examples::load_call_locations(with_examples, &dcx)?; + let call_locations = crate::scrape_examples::load_call_locations(with_examples, &dcx); let unstable_features = rustc_feature::UnstableFeatures::from_environment(crate_name.as_deref()); @@ -793,7 +761,7 @@ impl Options { no_emit_shared: false, html_no_source, }; - Ok((options, render_options)) + Some((options, render_options)) } /// Returns `true` if the file given as `self.input` is a Markdown file. diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index 0fe3adadba347..097bbeb6d2857 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -720,15 +720,8 @@ fn main_args( // Note that we discard any distinction between different non-zero exit // codes from `from_matches` here. let (options, render_options) = match config::Options::from_matches(early_dcx, &matches, args) { - Ok(opts) => opts, - Err(code) => { - return if code == 0 { - Ok(()) - } else { - #[allow(deprecated)] - Err(ErrorGuaranteed::unchecked_claim_error_was_emitted()) - }; - } + Some(opts) => opts, + None => return Ok(()), }; let diag = diff --git a/src/librustdoc/scrape_examples.rs b/src/librustdoc/scrape_examples.rs index b7d9c16f34837..9c9b386edda7c 100644 --- a/src/librustdoc/scrape_examples.rs +++ b/src/librustdoc/scrape_examples.rs @@ -38,28 +38,23 @@ pub(crate) struct ScrapeExamplesOptions { } impl ScrapeExamplesOptions { - pub(crate) fn new( - matches: &getopts::Matches, - dcx: &rustc_errors::DiagCtxt, - ) -> Result, i32> { + pub(crate) fn new(matches: &getopts::Matches, dcx: &rustc_errors::DiagCtxt) -> Option { let output_path = matches.opt_str("scrape-examples-output-path"); let target_crates = matches.opt_strs("scrape-examples-target-crate"); let scrape_tests = matches.opt_present("scrape-tests"); match (output_path, !target_crates.is_empty(), scrape_tests) { - (Some(output_path), true, _) => Ok(Some(ScrapeExamplesOptions { + (Some(output_path), true, _) => Some(ScrapeExamplesOptions { output_path: PathBuf::from(output_path), target_crates, scrape_tests, - })), + }), (Some(_), false, _) | (None, true, _) => { - dcx.err("must use --scrape-examples-output-path and --scrape-examples-target-crate together"); - Err(1) + dcx.fatal("must use --scrape-examples-output-path and --scrape-examples-target-crate together"); } (None, false, true) => { - dcx.err("must use --scrape-examples-output-path and --scrape-examples-target-crate with --scrape-tests"); - Err(1) + dcx.fatal("must use --scrape-examples-output-path and --scrape-examples-target-crate with --scrape-tests"); } - (None, false, false) => Ok(None), + (None, false, false) => None, } } } @@ -342,24 +337,20 @@ pub(crate) fn run( pub(crate) fn load_call_locations( with_examples: Vec, dcx: &rustc_errors::DiagCtxt, -) -> Result { - let inner = || { - let mut all_calls: AllCallLocations = FxHashMap::default(); - for path in with_examples { - let bytes = fs::read(&path).map_err(|e| format!("{e} (for path {path})"))?; - let mut decoder = MemDecoder::new(&bytes, 0); - let calls = AllCallLocations::decode(&mut decoder); - - for (function, fn_calls) in calls.into_iter() { - all_calls.entry(function).or_default().extend(fn_calls.into_iter()); - } - } +) -> AllCallLocations { + let mut all_calls: AllCallLocations = FxHashMap::default(); + for path in with_examples { + let bytes = match fs::read(&path) { + Ok(bytes) => bytes, + Err(e) => dcx.fatal(format!("failed to load examples: {e}")), + }; + let mut decoder = MemDecoder::new(&bytes, 0); + let calls = AllCallLocations::decode(&mut decoder); - Ok(all_calls) - }; + for (function, fn_calls) in calls.into_iter() { + all_calls.entry(function).or_default().extend(fn_calls.into_iter()); + } + } - inner().map_err(|e: String| { - dcx.err(format!("failed to load examples: {e}")); - 1 - }) + all_calls } From 97c157fe1e52eeabe14a64865b4731794a23393a Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 7 Feb 2024 10:26:50 +1100 Subject: [PATCH 27/29] Tighten up `ErrorGuaranteed` handling. - In `emit_producing_error_guaranteed`, only allow `Level::Error`. - In `emit_diagnostic`, only produce `ErrorGuaranteed` for `Level` and `DelayedBug`. (Not `Bug` or `Fatal`. They don't need it, because the relevant `emit` methods abort.) - Add/update various comments. --- compiler/rustc_errors/src/diagnostic_builder.rs | 16 ++++++++++------ compiler/rustc_errors/src/lib.rs | 14 ++++++++++---- compiler/rustc_span/src/lib.rs | 5 ++--- 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_errors/src/diagnostic_builder.rs b/compiler/rustc_errors/src/diagnostic_builder.rs index faff7f0b52673..e484bef0e0bc9 100644 --- a/compiler/rustc_errors/src/diagnostic_builder.rs +++ b/compiler/rustc_errors/src/diagnostic_builder.rs @@ -99,16 +99,20 @@ impl<'a, G: EmissionGuarantee> DiagnosticBuilder<'a, G> { } /// `ErrorGuaranteed::emit_producing_guarantee` uses this. - // FIXME(eddyb) make `ErrorGuaranteed` impossible to create outside `.emit()`. fn emit_producing_error_guaranteed(mut self) -> ErrorGuaranteed { let diag = self.take_diag(); - // Only allow a guarantee if the `level` wasn't switched to a - // non-error. The field isn't `pub`, but the whole `Diagnostic` can be - // overwritten with a new one, thanks to `DerefMut`. + // The only error levels that produce `ErrorGuaranteed` are + // `Error` and `DelayedBug`. But `DelayedBug` should never occur here + // because delayed bugs have their level changed to `Bug` when they are + // actually printed, so they produce an ICE. + // + // (Also, even though `level` isn't `pub`, the whole `Diagnostic` could + // be overwritten with a new one thanks to `DerefMut`. So this assert + // protects against that, too.) assert!( - diag.is_error(), - "emitted non-error ({:?}) diagnostic from `DiagnosticBuilder`", + matches!(diag.level, Level::Error | Level::DelayedBug), + "invalid diagnostic level ({:?})", diag.level, ); diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 7c62e3aa42281..74cccda6cbff6 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -931,6 +931,7 @@ impl DiagCtxt { /// This excludes lint errors and delayed bugs. pub fn has_errors(&self) -> Option { self.inner.borrow().has_errors().then(|| { + // FIXME(nnethercote) find a way to store an `ErrorGuaranteed`. #[allow(deprecated)] ErrorGuaranteed::unchecked_claim_error_was_emitted() }) @@ -942,6 +943,7 @@ impl DiagCtxt { let inner = self.inner.borrow(); let result = inner.has_errors() || inner.lint_err_count > 0; result.then(|| { + // FIXME(nnethercote) find a way to store an `ErrorGuaranteed`. #[allow(deprecated)] ErrorGuaranteed::unchecked_claim_error_was_emitted() }) @@ -954,6 +956,7 @@ impl DiagCtxt { let result = inner.has_errors() || inner.lint_err_count > 0 || !inner.delayed_bugs.is_empty(); result.then(|| { + // FIXME(nnethercote) find a way to store an `ErrorGuaranteed`. #[allow(deprecated)] ErrorGuaranteed::unchecked_claim_error_was_emitted() }) @@ -1238,6 +1241,7 @@ impl DiagCtxtInner { } } + // Return value is only `Some` if the level is `Error` or `DelayedBug`. fn emit_diagnostic(&mut self, mut diagnostic: Diagnostic) -> Option { assert!(diagnostic.level.can_be_top_or_sub().0); @@ -1316,6 +1320,7 @@ impl DiagCtxtInner { !self.emitted_diagnostics.insert(diagnostic_hash) }; + let level = diagnostic.level; let is_error = diagnostic.is_error(); let is_lint = diagnostic.is_lint.is_some(); @@ -1352,6 +1357,7 @@ impl DiagCtxtInner { self.emitter.emit_diagnostic(diagnostic); } + if is_error { if is_lint { self.lint_err_count += 1; @@ -1359,11 +1365,11 @@ impl DiagCtxtInner { self.err_count += 1; } self.panic_if_treat_err_as_bug(); + } - #[allow(deprecated)] - { - guaranteed = Some(ErrorGuaranteed::unchecked_claim_error_was_emitted()); - } + #[allow(deprecated)] + if level == Level::Error { + guaranteed = Some(ErrorGuaranteed::unchecked_claim_error_was_emitted()); } }); diff --git a/compiler/rustc_span/src/lib.rs b/compiler/rustc_span/src/lib.rs index ea6766ea583be..1a1e1de6734ac 100644 --- a/compiler/rustc_span/src/lib.rs +++ b/compiler/rustc_span/src/lib.rs @@ -2477,9 +2477,8 @@ where pub struct ErrorGuaranteed(()); impl ErrorGuaranteed { - /// To be used only if you really know what you are doing... ideally, we would find a way to - /// eliminate all calls to this method. - #[deprecated = "`Session::span_delayed_bug` should be preferred over this function"] + /// Don't use this outside of `DiagCtxtInner::emit_diagnostic`! + #[deprecated = "should only be used in `DiagCtxtInner::emit_diagnostic`"] pub fn unchecked_claim_error_was_emitted() -> Self { ErrorGuaranteed(()) } From 6889fe3806fa44addca89193e5c45ced8c7f532d Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 7 Feb 2024 19:30:59 +1100 Subject: [PATCH 28/29] Rename `unchecked_claim_error_was_emitted` as `unchecked_error_guaranteed`. It's more to-the-point. --- compiler/rustc_errors/src/lib.rs | 10 +++++----- compiler/rustc_span/src/lib.rs | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 74cccda6cbff6..ec5029e505f96 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -933,7 +933,7 @@ impl DiagCtxt { self.inner.borrow().has_errors().then(|| { // FIXME(nnethercote) find a way to store an `ErrorGuaranteed`. #[allow(deprecated)] - ErrorGuaranteed::unchecked_claim_error_was_emitted() + ErrorGuaranteed::unchecked_error_guaranteed() }) } @@ -945,7 +945,7 @@ impl DiagCtxt { result.then(|| { // FIXME(nnethercote) find a way to store an `ErrorGuaranteed`. #[allow(deprecated)] - ErrorGuaranteed::unchecked_claim_error_was_emitted() + ErrorGuaranteed::unchecked_error_guaranteed() }) } @@ -958,7 +958,7 @@ impl DiagCtxt { result.then(|| { // FIXME(nnethercote) find a way to store an `ErrorGuaranteed`. #[allow(deprecated)] - ErrorGuaranteed::unchecked_claim_error_was_emitted() + ErrorGuaranteed::unchecked_error_guaranteed() }) } @@ -1286,7 +1286,7 @@ impl DiagCtxtInner { let backtrace = std::backtrace::Backtrace::capture(); self.delayed_bugs.push(DelayedDiagnostic::with_backtrace(diagnostic, backtrace)); #[allow(deprecated)] - return Some(ErrorGuaranteed::unchecked_claim_error_was_emitted()); + return Some(ErrorGuaranteed::unchecked_error_guaranteed()); } GoodPathDelayedBug => { let backtrace = std::backtrace::Backtrace::capture(); @@ -1369,7 +1369,7 @@ impl DiagCtxtInner { #[allow(deprecated)] if level == Level::Error { - guaranteed = Some(ErrorGuaranteed::unchecked_claim_error_was_emitted()); + guaranteed = Some(ErrorGuaranteed::unchecked_error_guaranteed()); } }); diff --git a/compiler/rustc_span/src/lib.rs b/compiler/rustc_span/src/lib.rs index 1a1e1de6734ac..bcf04a71ae223 100644 --- a/compiler/rustc_span/src/lib.rs +++ b/compiler/rustc_span/src/lib.rs @@ -2479,7 +2479,7 @@ pub struct ErrorGuaranteed(()); impl ErrorGuaranteed { /// Don't use this outside of `DiagCtxtInner::emit_diagnostic`! #[deprecated = "should only be used in `DiagCtxtInner::emit_diagnostic`"] - pub fn unchecked_claim_error_was_emitted() -> Self { + pub fn unchecked_error_guaranteed() -> Self { ErrorGuaranteed(()) } } From c5e6df0c78afb787ae5b121ee8394baa21bca555 Mon Sep 17 00:00:00 2001 From: klensy Date: Wed, 7 Feb 2024 10:47:52 +0300 Subject: [PATCH 29/29] MirPass: make name more const --- compiler/rustc_middle/src/lib.rs | 1 + compiler/rustc_middle/src/mir/mod.rs | 8 ++++++-- compiler/rustc_middle/src/util/common.rs | 16 ++++++++++++++++ compiler/rustc_mir_transform/src/pass_manager.rs | 12 ++---------- 4 files changed, 25 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_middle/src/lib.rs b/compiler/rustc_middle/src/lib.rs index ddfb2ece39f10..9f1609a559568 100644 --- a/compiler/rustc_middle/src/lib.rs +++ b/compiler/rustc_middle/src/lib.rs @@ -30,6 +30,7 @@ #![feature(assert_matches)] #![feature(box_patterns)] #![feature(core_intrinsics)] +#![feature(const_type_name)] #![feature(discriminant_kind)] #![feature(exhaustive_patterns)] #![feature(coroutines)] diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index c9e69253701c5..4ba96464c0e2f 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -140,8 +140,12 @@ fn to_profiler_name(type_name: &'static str) -> &'static str { /// loop that goes over each available MIR and applies `run_pass`. pub trait MirPass<'tcx> { fn name(&self) -> &'static str { - let name = std::any::type_name::(); - if let Some((_, tail)) = name.rsplit_once(':') { tail } else { name } + // FIXME Simplify the implementation once more `str` methods get const-stable. + // See copypaste in `MirLint` + const { + let name = std::any::type_name::(); + crate::util::common::c_name(name) + } } fn profiler_name(&self) -> &'static str { diff --git a/compiler/rustc_middle/src/util/common.rs b/compiler/rustc_middle/src/util/common.rs index df101a2f6e4c6..dd3a36c7bf853 100644 --- a/compiler/rustc_middle/src/util/common.rs +++ b/compiler/rustc_middle/src/util/common.rs @@ -65,3 +65,19 @@ pub fn indenter() -> Indenter { debug!(">>"); Indenter { _cannot_construct_outside_of_this_module: () } } + +// const wrapper for `if let Some((_, tail)) = name.rsplit_once(':') { tail } else { name }` +pub const fn c_name(name: &'static str) -> &'static str { + // FIXME Simplify the implementation once more `str` methods get const-stable. + // and inline into call site + let bytes = name.as_bytes(); + let mut i = bytes.len(); + while i > 0 && bytes[i - 1] != b':' { + i = i - 1; + } + let (_, bytes) = bytes.split_at(i); + match std::str::from_utf8(bytes) { + Ok(name) => name, + Err(_) => name, + } +} diff --git a/compiler/rustc_mir_transform/src/pass_manager.rs b/compiler/rustc_mir_transform/src/pass_manager.rs index c1ef2b9f887fb..211a4057ff000 100644 --- a/compiler/rustc_mir_transform/src/pass_manager.rs +++ b/compiler/rustc_mir_transform/src/pass_manager.rs @@ -8,18 +8,10 @@ use crate::{lint::lint_body, validate, MirPass}; pub trait MirLint<'tcx> { fn name(&self) -> &'static str { // FIXME Simplify the implementation once more `str` methods get const-stable. + // See copypaste in `MirPass` const { let name = std::any::type_name::(); - let bytes = name.as_bytes(); - let mut i = bytes.len(); - while i > 0 && bytes[i - 1] != b':' { - i = i - 1; - } - let (_, bytes) = bytes.split_at(i); - match std::str::from_utf8(bytes) { - Ok(name) => name, - Err(_) => name, - } + rustc_middle::util::common::c_name(name) } }