From 1e33dd17115ca948c6e5ebf319695ed64490b2bf Mon Sep 17 00:00:00 2001 From: Harrison Kaiser Date: Sun, 15 Dec 2024 20:06:02 -0500 Subject: [PATCH 1/8] Fix logical error with what text is considered whitespace. There is a logical issue around what counts as leading white-space. There is code which does a subtraction assuming that no errors will be reported inside the leading whitespace. However we compute the length of that whitespace with std::char::is_whitespace and not rustc_lexer::is_whitespace. The former will include a no-break space while later will excluded it. We can only safely make the assumption that no errors will be reported in whitespace if it is all "Rust Standard" whitespace. Indeed an error does occur in unicode whitespace if it contains a no-break space. --- Cargo.lock | 1 + compiler/rustc_errors/Cargo.toml | 1 + compiler/rustc_errors/src/emitter.rs | 10 ++++++++-- tests/ui/errors/emitter-overflow-bad-whitespace.rs | 11 +++++++++++ .../errors/emitter-overflow-bad-whitespace.stderr | 13 +++++++++++++ 5 files changed, 34 insertions(+), 2 deletions(-) create mode 100644 tests/ui/errors/emitter-overflow-bad-whitespace.rs create mode 100644 tests/ui/errors/emitter-overflow-bad-whitespace.stderr diff --git a/Cargo.lock b/Cargo.lock index 5eeb855aacb87..b53d89ba88640 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3703,6 +3703,7 @@ dependencies = [ "rustc_fluent_macro", "rustc_hir", "rustc_index", + "rustc_lexer", "rustc_lint_defs", "rustc_macros", "rustc_serialize", diff --git a/compiler/rustc_errors/Cargo.toml b/compiler/rustc_errors/Cargo.toml index 06bae57638f3c..66b9adbead0b6 100644 --- a/compiler/rustc_errors/Cargo.toml +++ b/compiler/rustc_errors/Cargo.toml @@ -16,6 +16,7 @@ rustc_error_messages = { path = "../rustc_error_messages" } rustc_fluent_macro = { path = "../rustc_fluent_macro" } rustc_hir = { path = "../rustc_hir" } rustc_index = { path = "../rustc_index" } +rustc_lexer = { path = "../rustc_lexer" } rustc_lint_defs = { path = "../rustc_lint_defs" } rustc_macros = { path = "../rustc_macros" } rustc_serialize = { path = "../rustc_serialize" } diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index ac2f91cdeb3fe..17908bca26afc 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -19,6 +19,7 @@ use derive_setters::Setters; use rustc_data_structures::fx::{FxHashMap, FxIndexMap, FxIndexSet}; use rustc_data_structures::sync::{DynSend, IntoDynSyncSend, Lrc}; use rustc_error_messages::{FluentArgs, SpanLabel}; +use rustc_lexer; use rustc_lint_defs::pluralize; use rustc_span::hygiene::{ExpnKind, MacroKind}; use rustc_span::source_map::SourceMap; @@ -1698,9 +1699,14 @@ impl HumanEmitter { if let Some(source_string) = line.line_index.checked_sub(1).and_then(|l| file.get_line(l)) { + // Whitespace can only be removed (aka considered leading) + // if the lexer considers it whitespace. + // non-rustc_lexer::is_whitespace() chars are reported as an + // error (ex. no-break-spaces \u{a0}), and thus can't be considered + // for removal during error reporting. let leading_whitespace = source_string .chars() - .take_while(|c| c.is_whitespace()) + .take_while(|c| rustc_lexer::is_whitespace(*c)) .map(|c| { match c { // Tabs are displayed as 4 spaces @@ -1709,7 +1715,7 @@ impl HumanEmitter { } }) .sum(); - if source_string.chars().any(|c| !c.is_whitespace()) { + if source_string.chars().any(|c| !rustc_lexer::is_whitespace(c)) { whitespace_margin = min(whitespace_margin, leading_whitespace); } } diff --git a/tests/ui/errors/emitter-overflow-bad-whitespace.rs b/tests/ui/errors/emitter-overflow-bad-whitespace.rs new file mode 100644 index 0000000000000..d024d1f3921b2 --- /dev/null +++ b/tests/ui/errors/emitter-overflow-bad-whitespace.rs @@ -0,0 +1,11 @@ +// Invalid whitespace (not listed here: https://doc.rust-lang.org/reference/whitespace.html +// e.g. \u{a0}) before any other syntax on the line should not cause any integer overflow +// in the emitter, even when the terminal width causes the line to be truncated. +// +// issue #132918 + +//@ check-fail +//@ needs-rustc-debug-assertions +//@ compile-flags: --diagnostic-width=1 +                                        fn main() { return; } +//~^ ERROR unknown start of token: \u{a0} diff --git a/tests/ui/errors/emitter-overflow-bad-whitespace.stderr b/tests/ui/errors/emitter-overflow-bad-whitespace.stderr new file mode 100644 index 0000000000000..8c0c097bcb9c4 --- /dev/null +++ b/tests/ui/errors/emitter-overflow-bad-whitespace.stderr @@ -0,0 +1,13 @@ +error: unknown start of token: \u{a0} + --> $DIR/emitter-overflow-bad-whitespace.rs:10:1 + | +LL |     ... + | ^ + | +help: Unicode character ' ' (No-Break Space) looks like ' ' (Space), but it is not + | +LL |                                       fn main() { return; } + | + + +error: aborting due to 1 previous error + From b731c36faa2ecbd1043062ec7a226f17d289e00f Mon Sep 17 00:00:00 2001 From: MarcoIeni <11428655+MarcoIeni@users.noreply.github.com> Date: Thu, 19 Dec 2024 16:02:05 +0100 Subject: [PATCH 2/8] ci: use ubuntu `24` instead of `latest` --- .github/workflows/ci.yml | 4 ++-- .github/workflows/dependencies.yml | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 553ef676154bc..f0e151d25778c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -46,7 +46,7 @@ jobs: # If you want to modify CI jobs, take a look at src/ci/github-actions/jobs.yml. calculate_matrix: name: Calculate job matrix - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 outputs: jobs: ${{ steps.jobs.outputs.jobs }} run_type: ${{ steps.jobs.outputs.run_type }} @@ -243,7 +243,7 @@ jobs: # when a workflow is successful listening to webhooks only in our current bors implementation (homu). outcome: name: bors build finished - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 needs: [ calculate_matrix, job ] # !cancelled() executes the job regardless of whether the previous jobs passed or failed if: ${{ !cancelled() && contains(fromJSON('["auto", "try"]'), needs.calculate_matrix.outputs.run_type) }} diff --git a/.github/workflows/dependencies.yml b/.github/workflows/dependencies.yml index b7b5a03bd41f3..98d8c14f7d185 100644 --- a/.github/workflows/dependencies.yml +++ b/.github/workflows/dependencies.yml @@ -27,7 +27,7 @@ jobs: not-waiting-on-bors: if: github.repository_owner == 'rust-lang' name: skip if S-waiting-on-bors - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 steps: - env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} @@ -47,7 +47,7 @@ jobs: if: github.repository_owner == 'rust-lang' name: update dependencies needs: not-waiting-on-bors - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 steps: - name: checkout the source code uses: actions/checkout@v4 @@ -94,7 +94,7 @@ jobs: if: github.repository_owner == 'rust-lang' name: amend PR needs: update - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 permissions: contents: write pull-requests: write From 38bc902b1596f47b7e8c80760fb7250e0a4ea697 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 12 Dec 2024 12:25:41 +0000 Subject: [PATCH 3/8] Minor cleanup --- compiler/rustc_interface/src/passes.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_interface/src/passes.rs b/compiler/rustc_interface/src/passes.rs index 02905e632ab87..aff66e48fbbee 100644 --- a/compiler/rustc_interface/src/passes.rs +++ b/compiler/rustc_interface/src/passes.rs @@ -19,7 +19,7 @@ use rustc_incremental::setup_dep_graph; use rustc_lint::{BufferedEarlyLint, EarlyCheckNode, LintStore, unerased_lint_store}; use rustc_metadata::creader::CStore; use rustc_middle::arena::Arena; -use rustc_middle::ty::{self, GlobalCtxt, RegisteredTools, TyCtxt}; +use rustc_middle::ty::{self, CurrentGcx, GlobalCtxt, RegisteredTools, TyCtxt}; use rustc_middle::util::Providers; use rustc_parse::{ new_parser_from_file, new_parser_from_source_str, unwrap_or_emit_fatal, validate_attr, @@ -770,15 +770,14 @@ pub fn create_and_enter_global_ctxt FnOnce(TyCtxt<'tcx>) -> T>( // subtyping for GlobalCtxt::enter to be allowed. let inner: Box< dyn for<'tcx> FnOnce( - &'tcx Compiler, + &'tcx Session, + CurrentGcx, &'tcx OnceLock>, &'tcx WorkerLocal>, &'tcx WorkerLocal>, F, ) -> T, - > = Box::new(move |compiler, gcx_cell, arena, hir_arena, f| { - let sess = &compiler.sess; - + > = Box::new(move |sess, current_gcx, gcx_cell, arena, hir_arena, f| { TyCtxt::create_global_ctxt( gcx_cell, sess, @@ -796,7 +795,7 @@ pub fn create_and_enter_global_ctxt FnOnce(TyCtxt<'tcx>) -> T>( incremental, ), providers.hooks, - compiler.current_gcx.clone(), + current_gcx, |tcx| { let feed = tcx.create_crate_num(stable_crate_id).unwrap(); assert_eq!(feed.key(), LOCAL_CRATE); @@ -804,7 +803,7 @@ pub fn create_and_enter_global_ctxt FnOnce(TyCtxt<'tcx>) -> T>( let feed = tcx.feed_unit_query(); feed.features_query(tcx.arena.alloc(rustc_expand::config::features( - sess, + tcx.sess, &pre_configured_attrs, crate_name, ))); @@ -819,7 +818,7 @@ pub fn create_and_enter_global_ctxt FnOnce(TyCtxt<'tcx>) -> T>( ) }); - inner(compiler, &gcx_cell, &arena, &hir_arena, f) + inner(&compiler.sess, compiler.current_gcx.clone(), &gcx_cell, &arena, &hir_arena, f) } /// Runs all analyses that we guarantee to run, even if errors were reported in earlier analyses. From 7e6be136472a49c511a6861b9cbd9b6522c11762 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 12 Dec 2024 14:55:36 +0000 Subject: [PATCH 4/8] Make DependencyList an IndexVec --- .../rustc_codegen_cranelift/src/driver/jit.rs | 2 +- compiler/rustc_codegen_ssa/src/back/link.rs | 10 +-- compiler/rustc_codegen_ssa/src/back/linker.rs | 7 +- .../rustc_metadata/src/dependency_format.rs | 89 +++++++++++-------- compiler/rustc_metadata/src/rmeta/decoder.rs | 2 +- compiler/rustc_metadata/src/rmeta/encoder.rs | 12 +-- .../src/middle/dependency_format.rs | 6 +- src/tools/miri/src/helpers.rs | 14 +-- 8 files changed, 76 insertions(+), 66 deletions(-) diff --git a/compiler/rustc_codegen_cranelift/src/driver/jit.rs b/compiler/rustc_codegen_cranelift/src/driver/jit.rs index 4be4291021dfa..eaab3362c7e83 100644 --- a/compiler/rustc_codegen_cranelift/src/driver/jit.rs +++ b/compiler/rustc_codegen_cranelift/src/driver/jit.rs @@ -294,7 +294,7 @@ fn dep_symbol_lookup_fn( // search path. for &cnum in crate_info.used_crates.iter().rev() { let src = &crate_info.used_crate_source[&cnum]; - match data[cnum.as_usize() - 1] { + match data[cnum] { Linkage::NotLinked | Linkage::IncludedFromDylib => {} Linkage::Static => { let name = crate_info.crate_name[&cnum]; diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index f4f6161ebbccd..66258790c1ebd 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -262,7 +262,7 @@ pub fn each_linked_rlib( }; for &cnum in crates { - match fmts.get(cnum.as_usize() - 1) { + match fmts.get(cnum) { Some(&Linkage::NotLinked | &Linkage::Dynamic | &Linkage::IncludedFromDylib) => continue, Some(_) => {} None => return Err(errors::LinkRlibError::MissingFormat), @@ -624,7 +624,7 @@ fn link_staticlib( let mut all_rust_dylibs = vec![]; for &cnum in crates { - match fmts.get(cnum.as_usize() - 1) { + match fmts.get(cnum) { Some(&Linkage::Dynamic) => {} _ => continue, } @@ -2361,8 +2361,8 @@ fn linker_with_args( .crate_info .native_libraries .iter() - .filter_map(|(cnum, libraries)| { - (dependency_linkage[cnum.as_usize() - 1] != Linkage::Static).then_some(libraries) + .filter_map(|(&cnum, libraries)| { + (dependency_linkage[cnum] != Linkage::Static).then_some(libraries) }) .flatten() .collect::>(); @@ -2754,7 +2754,7 @@ fn add_upstream_rust_crates( // (e.g. `libstd` when `-C prefer-dynamic` is used). // FIXME: `dependency_formats` can report `profiler_builtins` as `NotLinked` for some // reason, it shouldn't do that because `profiler_builtins` should indeed be linked. - let linkage = data[cnum.as_usize() - 1]; + let linkage = data[cnum]; let link_static_crate = linkage == Linkage::Static || (linkage == Linkage::IncludedFromDylib || linkage == Linkage::NotLinked) && (codegen_results.crate_info.compiler_builtins == Some(cnum) diff --git a/compiler/rustc_codegen_ssa/src/back/linker.rs b/compiler/rustc_codegen_ssa/src/back/linker.rs index 8a2f3d73bc15c..3c6513ca26bb1 100644 --- a/compiler/rustc_codegen_ssa/src/back/linker.rs +++ b/compiler/rustc_codegen_ssa/src/back/linker.rs @@ -1744,15 +1744,10 @@ fn for_each_exported_symbols_include_dep<'tcx>( crate_type: CrateType, mut callback: impl FnMut(ExportedSymbol<'tcx>, SymbolExportInfo, CrateNum), ) { - for &(symbol, info) in tcx.exported_symbols(LOCAL_CRATE).iter() { - callback(symbol, info, LOCAL_CRATE); - } - let formats = tcx.dependency_formats(()); let deps = &formats[&crate_type]; - for (index, dep_format) in deps.iter().enumerate() { - let cnum = CrateNum::new(index + 1); + for (cnum, dep_format) in deps.iter_enumerated() { // For each dependency that we are linking to statically ... if *dep_format == Linkage::Static { for &(symbol, info) in tcx.exported_symbols(cnum).iter() { diff --git a/compiler/rustc_metadata/src/dependency_format.rs b/compiler/rustc_metadata/src/dependency_format.rs index e8de0acb7c9fe..1d4083a07d30b 100644 --- a/compiler/rustc_metadata/src/dependency_format.rs +++ b/compiler/rustc_metadata/src/dependency_format.rs @@ -52,7 +52,8 @@ //! than finding a number of solutions (there are normally quite a few). use rustc_data_structures::fx::{FxHashMap, FxHashSet}; -use rustc_hir::def_id::CrateNum; +use rustc_hir::def_id::{CrateNum, LOCAL_CRATE}; +use rustc_index::IndexVec; use rustc_middle::bug; use rustc_middle::middle::dependency_format::{Dependencies, DependencyList, Linkage}; use rustc_middle::ty::TyCtxt; @@ -84,7 +85,7 @@ fn calculate_type(tcx: TyCtxt<'_>, ty: CrateType) -> DependencyList { let sess = &tcx.sess; if !sess.opts.output_types.should_codegen() { - return Vec::new(); + return IndexVec::new(); } let preferred_linkage = match ty { @@ -131,7 +132,7 @@ fn calculate_type(tcx: TyCtxt<'_>, ty: CrateType) -> DependencyList { match preferred_linkage { // If the crate is not linked, there are no link-time dependencies. - Linkage::NotLinked => return Vec::new(), + Linkage::NotLinked => return IndexVec::new(), Linkage::Static => { // Attempt static linkage first. For dylibs and executables, we may be // able to retry below with dynamic linkage. @@ -156,7 +157,7 @@ fn calculate_type(tcx: TyCtxt<'_>, ty: CrateType) -> DependencyList { } sess.dcx().emit_err(RlibRequired { crate_name: tcx.crate_name(cnum) }); } - return Vec::new(); + return IndexVec::new(); } } Linkage::Dynamic | Linkage::IncludedFromDylib => {} @@ -210,13 +211,19 @@ fn calculate_type(tcx: TyCtxt<'_>, ty: CrateType) -> DependencyList { // Collect what we've got so far in the return vector. let last_crate = tcx.crates(()).len(); - let mut ret = (1..last_crate + 1) - .map(|cnum| match formats.get(&CrateNum::new(cnum)) { - Some(&RequireDynamic) => Linkage::Dynamic, - Some(&RequireStatic) => Linkage::IncludedFromDylib, - None => Linkage::NotLinked, - }) - .collect::>(); + let mut ret = IndexVec::new(); + assert_eq!(ret.push(Linkage::Static), LOCAL_CRATE); + for cnum in 1..last_crate + 1 { + let cnum = CrateNum::new(cnum); + assert_eq!( + ret.push(match formats.get(&cnum) { + Some(&RequireDynamic) => Linkage::Dynamic, + Some(&RequireStatic) => Linkage::IncludedFromDylib, + None => Linkage::NotLinked, + }), + cnum + ); + } // Run through the dependency list again, and add any missing libraries as // static libraries. @@ -232,7 +239,7 @@ fn calculate_type(tcx: TyCtxt<'_>, ty: CrateType) -> DependencyList { assert!(src.rlib.is_some() || src.rmeta.is_some()); info!("adding staticlib: {}", tcx.crate_name(cnum)); add_library(tcx, cnum, RequireStatic, &mut formats, &mut unavailable_as_static); - ret[cnum.as_usize() - 1] = Linkage::Static; + ret[cnum] = Linkage::Static; } } @@ -252,8 +259,10 @@ fn calculate_type(tcx: TyCtxt<'_>, ty: CrateType) -> DependencyList { // // For situations like this, we perform one last pass over the dependencies, // making sure that everything is available in the requested format. - for (cnum, kind) in ret.iter().enumerate() { - let cnum = CrateNum::new(cnum + 1); + for (cnum, kind) in ret.iter_enumerated() { + if cnum == LOCAL_CRATE { + continue; + } let src = tcx.used_crate_source(cnum); match *kind { Linkage::NotLinked | Linkage::IncludedFromDylib => {} @@ -334,14 +343,17 @@ fn attempt_static(tcx: TyCtxt<'_>, unavailable: &mut Vec) -> Option Linkage::Static, - CrateDepKind::MacrosOnly | CrateDepKind::Implicit => Linkage::NotLinked, - }) - .collect::>(); + let mut ret = IndexVec::new(); + assert_eq!(ret.push(Linkage::Static), LOCAL_CRATE); + for &cnum in tcx.crates(()) { + assert_eq!( + ret.push(match tcx.dep_kind(cnum) { + CrateDepKind::Explicit => Linkage::Static, + CrateDepKind::MacrosOnly | CrateDepKind::Implicit => Linkage::NotLinked, + }), + cnum + ); + } // Our allocator/panic runtime may not have been linked above if it wasn't // explicitly linked, which is the case for any injected dependency. Handle @@ -367,8 +379,7 @@ fn activate_injected_dep( list: &mut DependencyList, replaces_injected: &dyn Fn(CrateNum) -> bool, ) { - for (i, slot) in list.iter().enumerate() { - let cnum = CrateNum::new(i + 1); + for (cnum, slot) in list.iter_enumerated() { if !replaces_injected(cnum) { continue; } @@ -377,25 +388,23 @@ fn activate_injected_dep( } } if let Some(injected) = injected { - let idx = injected.as_usize() - 1; - assert_eq!(list[idx], Linkage::NotLinked); - list[idx] = Linkage::Static; + assert_eq!(list[injected], Linkage::NotLinked); + list[injected] = Linkage::Static; } } // After the linkage for a crate has been determined we need to verify that // there's only going to be one allocator in the output. -fn verify_ok(tcx: TyCtxt<'_>, list: &[Linkage]) { +fn verify_ok(tcx: TyCtxt<'_>, list: &DependencyList) { let sess = &tcx.sess; if list.is_empty() { return; } let mut panic_runtime = None; - for (i, linkage) in list.iter().enumerate() { + for (cnum, linkage) in list.iter_enumerated() { if let Linkage::NotLinked = *linkage { continue; } - let cnum = CrateNum::new(i + 1); if tcx.is_panic_runtime(cnum) { if let Some((prev, _)) = panic_runtime { @@ -431,11 +440,10 @@ fn verify_ok(tcx: TyCtxt<'_>, list: &[Linkage]) { // strategy. If the dep isn't linked, we ignore it, and if our strategy // is abort then it's compatible with everything. Otherwise all crates' // panic strategy must match our own. - for (i, linkage) in list.iter().enumerate() { + for (cnum, linkage) in list.iter_enumerated() { if let Linkage::NotLinked = *linkage { continue; } - let cnum = CrateNum::new(i + 1); if cnum == runtime_cnum || tcx.is_compiler_builtins(cnum) { continue; } @@ -450,13 +458,16 @@ fn verify_ok(tcx: TyCtxt<'_>, list: &[Linkage]) { }); } - let found_drop_strategy = tcx.panic_in_drop_strategy(cnum); - if tcx.sess.opts.unstable_opts.panic_in_drop != found_drop_strategy { - sess.dcx().emit_err(IncompatiblePanicInDropStrategy { - crate_name: tcx.crate_name(cnum), - found_strategy: found_drop_strategy, - desired_strategy: tcx.sess.opts.unstable_opts.panic_in_drop, - }); + // panic_in_drop_strategy isn't allowed for LOCAL_CRATE + if cnum != LOCAL_CRATE { + let found_drop_strategy = tcx.panic_in_drop_strategy(cnum); + if tcx.sess.opts.unstable_opts.panic_in_drop != found_drop_strategy { + sess.dcx().emit_err(IncompatiblePanicInDropStrategy { + crate_name: tcx.crate_name(cnum), + found_strategy: found_drop_strategy, + desired_strategy: tcx.sess.opts.unstable_opts.panic_in_drop, + }); + } } } } diff --git a/compiler/rustc_metadata/src/rmeta/decoder.rs b/compiler/rustc_metadata/src/rmeta/decoder.rs index 90b1d2952c599..c2b5e318bda72 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder.rs @@ -1474,7 +1474,7 @@ impl<'a> CrateMetadataRef<'a> { ) -> &'tcx [(CrateNum, LinkagePreference)] { tcx.arena.alloc_from_iter( self.root.dylib_dependency_formats.decode(self).enumerate().flat_map(|(i, link)| { - let cnum = CrateNum::new(i + 1); + let cnum = CrateNum::new(i + 1); // We skipped LOCAL_CRATE when encoding link.map(|link| (self.cnum_map[cnum], link)) }), ) diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index fff6f3f804fc3..c538ab99fb54b 100644 --- a/compiler/rustc_metadata/src/rmeta/encoder.rs +++ b/compiler/rustc_metadata/src/rmeta/encoder.rs @@ -2165,12 +2165,14 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { empty_proc_macro!(self); let formats = self.tcx.dependency_formats(()); if let Some(arr) = formats.get(&CrateType::Dylib) { - return self.lazy_array(arr.iter().map(|slot| match *slot { - Linkage::NotLinked | Linkage::IncludedFromDylib => None, + return self.lazy_array(arr.iter().skip(1 /* skip LOCAL_CRATE */).map( + |slot| match *slot { + Linkage::NotLinked | Linkage::IncludedFromDylib => None, - Linkage::Dynamic => Some(LinkagePreference::RequireDynamic), - Linkage::Static => Some(LinkagePreference::RequireStatic), - })); + Linkage::Dynamic => Some(LinkagePreference::RequireDynamic), + Linkage::Static => Some(LinkagePreference::RequireStatic), + }, + )); } LazyArray::default() } diff --git a/compiler/rustc_middle/src/middle/dependency_format.rs b/compiler/rustc_middle/src/middle/dependency_format.rs index e3b40b6415784..4f613e97631d0 100644 --- a/compiler/rustc_middle/src/middle/dependency_format.rs +++ b/compiler/rustc_middle/src/middle/dependency_format.rs @@ -8,13 +8,13 @@ // this will introduce circular dependency between rustc_metadata and rustc_middle use rustc_data_structures::fx::FxIndexMap; +use rustc_hir::def_id::CrateNum; +use rustc_index::IndexVec; use rustc_macros::{Decodable, Encodable, HashStable}; use rustc_session::config::CrateType; /// A list of dependencies for a certain crate type. -/// -/// The length of this vector is the same as the number of external crates used. -pub type DependencyList = Vec; +pub type DependencyList = IndexVec; /// A mapping of all required dependencies for a particular flavor of output. /// diff --git a/src/tools/miri/src/helpers.rs b/src/tools/miri/src/helpers.rs index ef4543dcee862..cb0040fcaf1d0 100644 --- a/src/tools/miri/src/helpers.rs +++ b/src/tools/miri/src/helpers.rs @@ -151,12 +151,14 @@ pub fn iter_exported_symbols<'tcx>( let dependency_format = dependency_formats .get(&CrateType::Executable) .expect("interpreting a non-executable crate"); - for cnum in dependency_format.iter().enumerate().filter_map(|(num, &linkage)| { - // We add 1 to the number because that's what rustc also does everywhere it - // calls `CrateNum::new`... - #[expect(clippy::arithmetic_side_effects)] - (linkage != Linkage::NotLinked).then_some(CrateNum::new(num + 1)) - }) { + for cnum in dependency_format + .iter_enumerated() + .filter_map(|(num, &linkage)| (linkage != Linkage::NotLinked).then_some(num)) + { + if cnum == LOCAL_CRATE { + continue; // Already handled above + } + // We can ignore `_export_info` here: we are a Rust crate, and everything is exported // from a Rust crate. for &(symbol, _export_info) in tcx.exported_symbols(cnum) { From 943f6a8ca9e9a52eb34ae927ecbd45d2d829d85d Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Fri, 13 Dec 2024 16:27:07 +0000 Subject: [PATCH 5/8] Update comments --- compiler/rustc_metadata/src/dependency_format.rs | 15 +++++++-------- compiler/rustc_session/src/cstore.rs | 2 +- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_metadata/src/dependency_format.rs b/compiler/rustc_metadata/src/dependency_format.rs index 1d4083a07d30b..6c5e59e49f7ad 100644 --- a/compiler/rustc_metadata/src/dependency_format.rs +++ b/compiler/rustc_metadata/src/dependency_format.rs @@ -229,7 +229,7 @@ fn calculate_type(tcx: TyCtxt<'_>, ty: CrateType) -> DependencyList { // static libraries. // // If the crate hasn't been included yet and it's not actually required - // (e.g., it's an allocator) then we skip it here as well. + // (e.g., it's a panic runtime) then we skip it here as well. for &cnum in tcx.crates(()).iter() { let src = tcx.used_crate_source(cnum); if src.dylib.is_none() @@ -247,8 +247,7 @@ fn calculate_type(tcx: TyCtxt<'_>, ty: CrateType) -> DependencyList { // artifact which means that we may need to inject dependencies of some // form. // - // Things like allocators and panic runtimes may not have been activated - // quite yet, so do so here. + // Things like panic runtimes may not have been activated quite yet, so do so here. activate_injected_dep(CStore::from_tcx(tcx).injected_panic_runtime(), &mut ret, &|cnum| { tcx.is_panic_runtime(cnum) }); @@ -355,9 +354,9 @@ fn attempt_static(tcx: TyCtxt<'_>, unavailable: &mut Vec) -> Option, list: &DependencyList) { let sess = &tcx.sess; if list.is_empty() { diff --git a/compiler/rustc_session/src/cstore.rs b/compiler/rustc_session/src/cstore.rs index beae9dc278c15..c8a5c22ad1230 100644 --- a/compiler/rustc_session/src/cstore.rs +++ b/compiler/rustc_session/src/cstore.rs @@ -42,7 +42,7 @@ pub enum CrateDepKind { /// A dependency that is only used for its macros. MacrosOnly, /// A dependency that is always injected into the dependency list and so - /// doesn't need to be linked to an rlib, e.g., the injected allocator. + /// doesn't need to be linked to an rlib, e.g., the injected panic runtime. Implicit, /// A dependency that is required by an rlib version of this crate. /// Ordinary `extern crate`s result in `Explicit` dependencies. From 544809e48a3de2de20c15bc5488b889cd375215e Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 18 Dec 2024 13:53:23 +1100 Subject: [PATCH 6/8] coverage: Rename `basic_coverage_blocks` to just `graph` During coverage instrumentation, this variable always holds the coverage graph, which is a simplified view of the MIR control-flow graph. The new name is clearer in context, and also shorter. --- .../src/coverage/mappings.rs | 20 +++-- .../rustc_mir_transform/src/coverage/mod.rs | 40 ++++----- .../rustc_mir_transform/src/coverage/spans.rs | 9 +-- .../src/coverage/spans/from_mir.rs | 4 +- .../rustc_mir_transform/src/coverage/tests.rs | 81 +++++++------------ 5 files changed, 61 insertions(+), 93 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/mappings.rs b/compiler/rustc_mir_transform/src/coverage/mappings.rs index 2db7c6cf1d6fd..4185b3f4d4d85 100644 --- a/compiler/rustc_mir_transform/src/coverage/mappings.rs +++ b/compiler/rustc_mir_transform/src/coverage/mappings.rs @@ -80,7 +80,7 @@ pub(super) fn extract_all_mapping_info_from_mir<'tcx>( tcx: TyCtxt<'tcx>, mir_body: &mir::Body<'tcx>, hir_info: &ExtractedHirInfo, - basic_coverage_blocks: &CoverageGraph, + graph: &CoverageGraph, ) -> ExtractedMappings { let mut code_mappings = vec![]; let mut branch_pairs = vec![]; @@ -102,23 +102,23 @@ pub(super) fn extract_all_mapping_info_from_mir<'tcx>( } } else { // Extract coverage spans from MIR statements/terminators as normal. - extract_refined_covspans(mir_body, hir_info, basic_coverage_blocks, &mut code_mappings); + extract_refined_covspans(mir_body, hir_info, graph, &mut code_mappings); } - branch_pairs.extend(extract_branch_pairs(mir_body, hir_info, basic_coverage_blocks)); + branch_pairs.extend(extract_branch_pairs(mir_body, hir_info, graph)); extract_mcdc_mappings( mir_body, tcx, hir_info.body_span, - basic_coverage_blocks, + graph, &mut mcdc_bitmap_bits, &mut mcdc_degraded_branches, &mut mcdc_mappings, ); ExtractedMappings { - num_bcbs: basic_coverage_blocks.num_nodes(), + num_bcbs: graph.num_nodes(), code_mappings, branch_pairs, mcdc_bitmap_bits, @@ -211,7 +211,7 @@ fn resolve_block_markers( pub(super) fn extract_branch_pairs( mir_body: &mir::Body<'_>, hir_info: &ExtractedHirInfo, - basic_coverage_blocks: &CoverageGraph, + graph: &CoverageGraph, ) -> Vec { let Some(coverage_info_hi) = mir_body.coverage_info_hi.as_deref() else { return vec![] }; @@ -228,8 +228,7 @@ pub(super) fn extract_branch_pairs( } let span = unexpand_into_body_span(raw_span, hir_info.body_span)?; - let bcb_from_marker = - |marker: BlockMarkerId| basic_coverage_blocks.bcb_from_bb(block_markers[marker]?); + let bcb_from_marker = |marker: BlockMarkerId| graph.bcb_from_bb(block_markers[marker]?); let true_bcb = bcb_from_marker(true_marker)?; let false_bcb = bcb_from_marker(false_marker)?; @@ -243,7 +242,7 @@ pub(super) fn extract_mcdc_mappings( mir_body: &mir::Body<'_>, tcx: TyCtxt<'_>, body_span: Span, - basic_coverage_blocks: &CoverageGraph, + graph: &CoverageGraph, mcdc_bitmap_bits: &mut usize, mcdc_degraded_branches: &mut impl Extend, mcdc_mappings: &mut impl Extend<(MCDCDecision, Vec)>, @@ -252,8 +251,7 @@ pub(super) fn extract_mcdc_mappings( let block_markers = resolve_block_markers(coverage_info_hi, mir_body); - let bcb_from_marker = - |marker: BlockMarkerId| basic_coverage_blocks.bcb_from_bb(block_markers[marker]?); + let bcb_from_marker = |marker: BlockMarkerId| graph.bcb_from_bb(block_markers[marker]?); let check_branch_bcb = |raw_span: Span, true_marker: BlockMarkerId, false_marker: BlockMarkerId| { diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index 83e7ff99639d1..57956448414b5 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -71,16 +71,15 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir: let _span = debug_span!("instrument_function_for_coverage", ?def_id).entered(); let hir_info = extract_hir_info(tcx, def_id.expect_local()); - let basic_coverage_blocks = CoverageGraph::from_mir(mir_body); + + // Build the coverage graph, which is a simplified view of the MIR control-flow + // graph that ignores some details not relevant to coverage instrumentation. + let graph = CoverageGraph::from_mir(mir_body); //////////////////////////////////////////////////// // Extract coverage spans and other mapping info from MIR. - let extracted_mappings = mappings::extract_all_mapping_info_from_mir( - tcx, - mir_body, - &hir_info, - &basic_coverage_blocks, - ); + let extracted_mappings = + mappings::extract_all_mapping_info_from_mir(tcx, mir_body, &hir_info, &graph); //////////////////////////////////////////////////// // Create an optimized mix of `Counter`s and `Expression`s for the `CoverageGraph`. Ensure @@ -94,7 +93,7 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir: } let coverage_counters = - CoverageCounters::make_bcb_counters(&basic_coverage_blocks, &bcbs_with_counter_mappings); + CoverageCounters::make_bcb_counters(&graph, &bcbs_with_counter_mappings); let mappings = create_mappings(&extracted_mappings, &coverage_counters); if mappings.is_empty() { @@ -103,14 +102,9 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir: return; } - inject_coverage_statements( - mir_body, - &basic_coverage_blocks, - &extracted_mappings, - &coverage_counters, - ); + inject_coverage_statements(mir_body, &graph, &extracted_mappings, &coverage_counters); - inject_mcdc_statements(mir_body, &basic_coverage_blocks, &extracted_mappings); + inject_mcdc_statements(mir_body, &graph, &extracted_mappings); let mcdc_num_condition_bitmaps = extracted_mappings .mcdc_mappings @@ -243,7 +237,7 @@ fn create_mappings( /// inject any necessary coverage statements into MIR. fn inject_coverage_statements<'tcx>( mir_body: &mut mir::Body<'tcx>, - basic_coverage_blocks: &CoverageGraph, + graph: &CoverageGraph, extracted_mappings: &ExtractedMappings, coverage_counters: &CoverageCounters, ) { @@ -253,12 +247,12 @@ fn inject_coverage_statements<'tcx>( // 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 site { - Site::Node { bcb } => basic_coverage_blocks[bcb].leader_bb(), + Site::Node { bcb } => graph[bcb].leader_bb(), Site::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 from_bb = graph[from_bcb].last_bb(); + let to_bb = graph[to_bcb].leader_bb(); let new_bb = inject_edge_counter_basic_block(mir_body, from_bb, to_bb); debug!( @@ -291,7 +285,7 @@ fn inject_coverage_statements<'tcx>( inject_statement( mir_body, CoverageKind::ExpressionUsed { id: expression_id }, - basic_coverage_blocks[bcb].leader_bb(), + graph[bcb].leader_bb(), ); } } @@ -300,13 +294,13 @@ fn inject_coverage_statements<'tcx>( /// For each decision inject statements to update test vector bitmap after it has been evaluated. fn inject_mcdc_statements<'tcx>( mir_body: &mut mir::Body<'tcx>, - basic_coverage_blocks: &CoverageGraph, + graph: &CoverageGraph, extracted_mappings: &ExtractedMappings, ) { for (decision, conditions) in &extracted_mappings.mcdc_mappings { // Inject test vector update first because `inject_statement` always insert new statement at head. for &end in &decision.end_bcbs { - let end_bb = basic_coverage_blocks[end].leader_bb(); + let end_bb = graph[end].leader_bb(); inject_statement( mir_body, CoverageKind::TestVectorBitmapUpdate { @@ -327,7 +321,7 @@ fn inject_mcdc_statements<'tcx>( } in conditions { for (index, bcb) in [(false_index, false_bcb), (true_index, true_bcb)] { - let bb = basic_coverage_blocks[bcb].leader_bb(); + let bb = graph[bcb].leader_bb(); inject_statement( mir_body, CoverageKind::CondBitmapUpdate { diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index 085c738f1f9fc..314a86ea52f0a 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -17,14 +17,13 @@ mod from_mir; pub(super) fn extract_refined_covspans( mir_body: &mir::Body<'_>, hir_info: &ExtractedHirInfo, - basic_coverage_blocks: &CoverageGraph, + graph: &CoverageGraph, code_mappings: &mut impl Extend, ) { - let ExtractedCovspans { mut covspans } = - extract_covspans_from_mir(mir_body, hir_info, basic_coverage_blocks); + let ExtractedCovspans { mut covspans } = extract_covspans_from_mir(mir_body, hir_info, graph); // First, perform the passes that need macro information. - covspans.sort_by(|a, b| basic_coverage_blocks.cmp_in_dominator_order(a.bcb, b.bcb)); + covspans.sort_by(|a, b| graph.cmp_in_dominator_order(a.bcb, b.bcb)); remove_unwanted_expansion_spans(&mut covspans); split_visible_macro_spans(&mut covspans); @@ -34,7 +33,7 @@ pub(super) fn extract_refined_covspans( let compare_covspans = |a: &Covspan, b: &Covspan| { compare_spans(a.span, b.span) // After deduplication, we want to keep only the most-dominated BCB. - .then_with(|| basic_coverage_blocks.cmp_in_dominator_order(a.bcb, b.bcb).reverse()) + .then_with(|| graph.cmp_in_dominator_order(a.bcb, b.bcb).reverse()) }; covspans.sort_by(compare_covspans); diff --git a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs index 824d657e1fc2d..26ce743be3613 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs @@ -22,13 +22,13 @@ pub(crate) struct ExtractedCovspans { pub(crate) fn extract_covspans_from_mir( mir_body: &mir::Body<'_>, hir_info: &ExtractedHirInfo, - basic_coverage_blocks: &CoverageGraph, + graph: &CoverageGraph, ) -> ExtractedCovspans { let &ExtractedHirInfo { body_span, .. } = hir_info; let mut covspans = vec![]; - for (bcb, bcb_data) in basic_coverage_blocks.iter_enumerated() { + for (bcb, bcb_data) in graph.iter_enumerated() { bcb_to_initial_coverage_spans(mir_body, body_span, bcb, bcb_data, &mut covspans); } diff --git a/compiler/rustc_mir_transform/src/coverage/tests.rs b/compiler/rustc_mir_transform/src/coverage/tests.rs index 233ca9981c50c..b2ee50de50a23 100644 --- a/compiler/rustc_mir_transform/src/coverage/tests.rs +++ b/compiler/rustc_mir_transform/src/coverage/tests.rs @@ -223,16 +223,12 @@ fn print_mir_graphviz(name: &str, mir_body: &Body<'_>) { } } -fn print_coverage_graphviz( - name: &str, - mir_body: &Body<'_>, - basic_coverage_blocks: &graph::CoverageGraph, -) { +fn print_coverage_graphviz(name: &str, mir_body: &Body<'_>, graph: &graph::CoverageGraph) { if PRINT_GRAPHS { println!( "digraph {} {{\n{}\n}}", name, - basic_coverage_blocks + graph .iter_enumerated() .map(|(bcb, bcb_data)| { format!( @@ -240,7 +236,7 @@ fn print_coverage_graphviz( bcb, bcb, mir_body[bcb_data.last_bb()].terminator().kind.name(), - basic_coverage_blocks + graph .successors(bcb) .map(|successor| { format!(" {:?} -> {:?};", bcb, successor) }) .join("\n") @@ -300,11 +296,11 @@ fn goto_switchint<'a>() -> Body<'a> { #[track_caller] fn assert_successors( - basic_coverage_blocks: &graph::CoverageGraph, + graph: &graph::CoverageGraph, bcb: BasicCoverageBlock, expected_successors: &[BasicCoverageBlock], ) { - let mut successors = basic_coverage_blocks.successors[bcb].clone(); + let mut successors = graph.successors[bcb].clone(); successors.sort_unstable(); assert_eq!(successors, expected_successors); } @@ -315,8 +311,8 @@ fn test_covgraph_goto_switchint() { if false { eprintln!("basic_blocks = {}", debug_basic_blocks(&mir_body)); } - let basic_coverage_blocks = graph::CoverageGraph::from_mir(&mir_body); - print_coverage_graphviz("covgraph_goto_switchint ", &mir_body, &basic_coverage_blocks); + let graph = graph::CoverageGraph::from_mir(&mir_body); + print_coverage_graphviz("covgraph_goto_switchint ", &mir_body, &graph); /* ┌──────────────┐ ┌─────────────────┐ │ bcb2: Return │ ◀── │ bcb0: SwitchInt │ @@ -328,16 +324,11 @@ fn test_covgraph_goto_switchint() { │ bcb1: Return │ └─────────────────┘ */ - assert_eq!( - basic_coverage_blocks.num_nodes(), - 3, - "basic_coverage_blocks: {:?}", - basic_coverage_blocks.iter_enumerated().collect::>() - ); + assert_eq!(graph.num_nodes(), 3, "graph: {:?}", graph.iter_enumerated().collect::>()); - assert_successors(&basic_coverage_blocks, bcb(0), &[bcb(1), bcb(2)]); - assert_successors(&basic_coverage_blocks, bcb(1), &[]); - assert_successors(&basic_coverage_blocks, bcb(2), &[]); + assert_successors(&graph, bcb(0), &[bcb(1), bcb(2)]); + assert_successors(&graph, bcb(1), &[]); + assert_successors(&graph, bcb(2), &[]); } /// Create a mock `Body` with a loop. @@ -383,12 +374,8 @@ fn switchint_then_loop_else_return<'a>() -> Body<'a> { #[test] fn test_covgraph_switchint_then_loop_else_return() { let mir_body = switchint_then_loop_else_return(); - let basic_coverage_blocks = graph::CoverageGraph::from_mir(&mir_body); - print_coverage_graphviz( - "covgraph_switchint_then_loop_else_return", - &mir_body, - &basic_coverage_blocks, - ); + let graph = graph::CoverageGraph::from_mir(&mir_body); + print_coverage_graphviz("covgraph_switchint_then_loop_else_return", &mir_body, &graph); /* ┌─────────────────┐ │ bcb0: Call │ @@ -408,17 +395,12 @@ fn test_covgraph_switchint_then_loop_else_return() { │ │ └─────────────────────────────────────┘ */ - assert_eq!( - basic_coverage_blocks.num_nodes(), - 4, - "basic_coverage_blocks: {:?}", - basic_coverage_blocks.iter_enumerated().collect::>() - ); + assert_eq!(graph.num_nodes(), 4, "graph: {:?}", graph.iter_enumerated().collect::>()); - assert_successors(&basic_coverage_blocks, bcb(0), &[bcb(1)]); - assert_successors(&basic_coverage_blocks, bcb(1), &[bcb(2), bcb(3)]); - assert_successors(&basic_coverage_blocks, bcb(2), &[]); - assert_successors(&basic_coverage_blocks, bcb(3), &[bcb(1)]); + assert_successors(&graph, bcb(0), &[bcb(1)]); + assert_successors(&graph, bcb(1), &[bcb(2), bcb(3)]); + assert_successors(&graph, bcb(2), &[]); + assert_successors(&graph, bcb(3), &[bcb(1)]); } /// Create a mock `Body` with nested loops. @@ -494,11 +476,11 @@ fn switchint_loop_then_inner_loop_else_break<'a>() -> Body<'a> { #[test] fn test_covgraph_switchint_loop_then_inner_loop_else_break() { let mir_body = switchint_loop_then_inner_loop_else_break(); - let basic_coverage_blocks = graph::CoverageGraph::from_mir(&mir_body); + let graph = graph::CoverageGraph::from_mir(&mir_body); print_coverage_graphviz( "covgraph_switchint_loop_then_inner_loop_else_break", &mir_body, - &basic_coverage_blocks, + &graph, ); /* ┌─────────────────┐ @@ -531,18 +513,13 @@ fn test_covgraph_switchint_loop_then_inner_loop_else_break() { │ │ └────────────────────────────────────────────┘ */ - assert_eq!( - basic_coverage_blocks.num_nodes(), - 7, - "basic_coverage_blocks: {:?}", - basic_coverage_blocks.iter_enumerated().collect::>() - ); - - assert_successors(&basic_coverage_blocks, bcb(0), &[bcb(1)]); - assert_successors(&basic_coverage_blocks, bcb(1), &[bcb(2), bcb(3)]); - assert_successors(&basic_coverage_blocks, bcb(2), &[]); - assert_successors(&basic_coverage_blocks, bcb(3), &[bcb(4)]); - assert_successors(&basic_coverage_blocks, bcb(4), &[bcb(5), bcb(6)]); - assert_successors(&basic_coverage_blocks, bcb(5), &[bcb(1)]); - assert_successors(&basic_coverage_blocks, bcb(6), &[bcb(4)]); + assert_eq!(graph.num_nodes(), 7, "graph: {:?}", graph.iter_enumerated().collect::>()); + + assert_successors(&graph, bcb(0), &[bcb(1)]); + assert_successors(&graph, bcb(1), &[bcb(2), bcb(3)]); + assert_successors(&graph, bcb(2), &[]); + assert_successors(&graph, bcb(3), &[bcb(4)]); + assert_successors(&graph, bcb(4), &[bcb(5), bcb(6)]); + assert_successors(&graph, bcb(5), &[bcb(1)]); + assert_successors(&graph, bcb(6), &[bcb(4)]); } From 0daa921f0e95acef303f3c091a09d667f9b63405 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Fri, 20 Dec 2024 08:35:02 +0000 Subject: [PATCH 7/8] Review comments --- compiler/rustc_codegen_ssa/src/back/link.rs | 5 ++--- compiler/rustc_metadata/src/dependency_format.rs | 7 +++++++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index 66258790c1ebd..562af0777f52f 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -234,8 +234,6 @@ pub fn each_linked_rlib( crate_type: Option, f: &mut dyn FnMut(CrateNum, &Path), ) -> Result<(), errors::LinkRlibError> { - let crates = info.used_crates.iter(); - let fmts = if let Some(crate_type) = crate_type { let Some(fmts) = info.dependency_formats.get(&crate_type) else { return Err(errors::LinkRlibError::MissingFormat); @@ -261,7 +259,8 @@ pub fn each_linked_rlib( info.dependency_formats.first().unwrap().1 }; - for &cnum in crates { + let used_dep_crates = info.used_crates.iter(); + for &cnum in used_dep_crates { match fmts.get(cnum) { Some(&Linkage::NotLinked | &Linkage::Dynamic | &Linkage::IncludedFromDylib) => continue, Some(_) => {} diff --git a/compiler/rustc_metadata/src/dependency_format.rs b/compiler/rustc_metadata/src/dependency_format.rs index 6c5e59e49f7ad..582c2215d92e1 100644 --- a/compiler/rustc_metadata/src/dependency_format.rs +++ b/compiler/rustc_metadata/src/dependency_format.rs @@ -212,7 +212,14 @@ fn calculate_type(tcx: TyCtxt<'_>, ty: CrateType) -> DependencyList { // Collect what we've got so far in the return vector. let last_crate = tcx.crates(()).len(); let mut ret = IndexVec::new(); + + // We need to fill in something for LOCAL_CRATE as IndexVec is a dense map. + // Linkage::Static semantically the most correct thing to use as the local + // crate is always statically linked into the linker output, even when + // linking a dylib. Using Linkage::Static also allow avoiding special cases + // for LOCAL_CRATE in some places. assert_eq!(ret.push(Linkage::Static), LOCAL_CRATE); + for cnum in 1..last_crate + 1 { let cnum = CrateNum::new(cnum); assert_eq!( From adf549808e42fe6155f043aee34a806fdc3de4c1 Mon Sep 17 00:00:00 2001 From: lcnr Date: Fri, 20 Dec 2024 11:45:52 +0100 Subject: [PATCH 8/8] add comments --- compiler/rustc_borrowck/src/region_infer/mod.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_borrowck/src/region_infer/mod.rs b/compiler/rustc_borrowck/src/region_infer/mod.rs index d39fbf32921ac..60f7770d3f792 100644 --- a/compiler/rustc_borrowck/src/region_infer/mod.rs +++ b/compiler/rustc_borrowck/src/region_infer/mod.rs @@ -795,7 +795,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { // If the member region lives in a higher universe, we currently choose // the most conservative option by leaving it unchanged. - if !self.constraint_sccs().annotation(scc).min_universe().is_root() { return; } @@ -823,12 +822,14 @@ impl<'tcx> RegionInferenceContext<'tcx> { } debug!(?choice_regions, "after ub"); - // At this point we can pick any member of `choice_regions`, but to avoid potential - // non-determinism we will pick the *unique minimum* choice. + // At this point we can pick any member of `choice_regions` and would like to choose + // it to be a small as possible. To avoid potential non-determinism we will pick the + // smallest such choice. // // Because universal regions are only partially ordered (i.e, not every two regions are // comparable), we will ignore any region that doesn't compare to all others when picking // the minimum choice. + // // For example, consider `choice_regions = ['static, 'a, 'b, 'c, 'd, 'e]`, where // `'static: 'a, 'static: 'b, 'a: 'c, 'b: 'c, 'c: 'd, 'c: 'e`. // `['d, 'e]` are ignored because they do not compare - the same goes for `['a, 'b]`. @@ -853,6 +854,8 @@ impl<'tcx> RegionInferenceContext<'tcx> { return; }; + // As we require `'scc: 'min_choice`, we have definitely already computed + // its `scc_values` at this point. let min_choice_scc = self.constraint_sccs.scc(min_choice); debug!(?min_choice, ?min_choice_scc); if self.scc_values.add_region(scc, min_choice_scc) {