From d695b95e3beb872fef59d57bba0339f6b55f3e14 Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Fri, 15 Sep 2023 15:32:34 +0200 Subject: [PATCH 01/18] implement -Z ignore-directory-in-diagnostics-source-blocks --- .../src/annotate_snippet_emitter_writer.rs | 1 + compiler/rustc_errors/src/emitter.rs | 27 ++++++++++++++++--- compiler/rustc_errors/src/json.rs | 16 +++++++++-- compiler/rustc_session/src/options.rs | 2 ++ compiler/rustc_session/src/session.rs | 10 +++++-- 5 files changed, 49 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs index 5d3b2f45166b3..203e529120b3e 100644 --- a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs +++ b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs @@ -169,6 +169,7 @@ impl AnnotateSnippetEmitterWriter { .map(|line| { // Ensure the source file is present before we try // to load a string from it. + // FIXME(#115869): support -Z ignore-directory-in-diagnostics-source-blocks source_map.ensure_source_file_source_present(&file); ( format!("{}", source_map.filename_for_diagnostics(&file.name)), diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index 58be74f887bfd..d322cbe9d9bf5 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -8,7 +8,7 @@ //! The output types are defined in `rustc_session::config::ErrorOutputType`. use rustc_span::source_map::SourceMap; -use rustc_span::{FileLines, SourceFile, Span}; +use rustc_span::{FileLines, FileName, SourceFile, Span}; use crate::snippet::{ Annotation, AnnotationColumn, AnnotationType, Line, MultilineAnnotation, Style, StyledString, @@ -635,6 +635,7 @@ pub struct EmitterWriter { short_message: bool, teach: bool, ui_testing: bool, + ignored_directories_in_source_blocks: Vec, diagnostic_width: Option, macro_backtrace: bool, @@ -664,6 +665,7 @@ impl EmitterWriter { short_message: false, teach: false, ui_testing: false, + ignored_directories_in_source_blocks: Vec::new(), diagnostic_width: None, macro_backtrace: false, track_diagnostics: false, @@ -1193,7 +1195,7 @@ impl EmitterWriter { let will_be_emitted = |span: Span| { !span.is_dummy() && { let file = sm.lookup_source_file(span.hi()); - sm.ensure_source_file_source_present(&file) + should_show_source_code(&self.ignored_directories_in_source_blocks, sm, &file) } }; @@ -1388,7 +1390,11 @@ impl EmitterWriter { // Print out the annotate source lines that correspond with the error for annotated_file in annotated_files { // we can't annotate anything if the source is unavailable. - if !sm.ensure_source_file_source_present(&annotated_file.file) { + if !should_show_source_code( + &self.ignored_directories_in_source_blocks, + sm, + &annotated_file.file, + ) { if !self.short_message { // We'll just print an unannotated message. for (annotation_id, line) in annotated_file.lines.iter().enumerate() { @@ -2737,3 +2743,18 @@ pub fn is_case_difference(sm: &SourceMap, suggested: &str, sp: Span) -> bool { // bug, but be defensive against that here. && found != suggested } + +pub(crate) fn should_show_source_code( + ignored_directories: &[String], + sm: &SourceMap, + file: &SourceFile, +) -> bool { + if !sm.ensure_source_file_source_present(file) { + return false; + } + + let FileName::Real(name) = &file.name else { return true }; + name.local_path() + .map(|path| ignored_directories.iter().all(|dir| !path.starts_with(dir))) + .unwrap_or(true) +} diff --git a/compiler/rustc_errors/src/json.rs b/compiler/rustc_errors/src/json.rs index 38667c5ff819d..0cb75c71b7346 100644 --- a/compiler/rustc_errors/src/json.rs +++ b/compiler/rustc_errors/src/json.rs @@ -12,7 +12,7 @@ use rustc_span::source_map::{FilePathMapping, SourceMap}; use termcolor::{ColorSpec, WriteColor}; -use crate::emitter::{Emitter, HumanReadableErrorType}; +use crate::emitter::{should_show_source_code, Emitter, HumanReadableErrorType}; use crate::registry::Registry; use crate::translation::{to_fluent_args, Translate}; use crate::DiagnosticId; @@ -45,6 +45,7 @@ pub struct JsonEmitter { fallback_bundle: LazyFallbackBundle, pretty: bool, ui_testing: bool, + ignored_directories_in_source_blocks: Vec, json_rendered: HumanReadableErrorType, diagnostic_width: Option, macro_backtrace: bool, @@ -73,6 +74,7 @@ impl JsonEmitter { fallback_bundle, pretty, ui_testing: false, + ignored_directories_in_source_blocks: Vec::new(), json_rendered, diagnostic_width, macro_backtrace, @@ -127,6 +129,7 @@ impl JsonEmitter { fallback_bundle, pretty, ui_testing: false, + ignored_directories_in_source_blocks: Vec::new(), json_rendered, diagnostic_width, macro_backtrace, @@ -138,6 +141,10 @@ impl JsonEmitter { pub fn ui_testing(self, ui_testing: bool) -> Self { Self { ui_testing, ..self } } + + pub fn ignored_directories_in_source_blocks(self, value: Vec) -> Self { + Self { ignored_directories_in_source_blocks: value, ..self } + } } impl Translate for JsonEmitter { @@ -381,6 +388,7 @@ impl Diagnostic { .track_diagnostics(je.track_diagnostics) .terminal_url(je.terminal_url) .ui_testing(je.ui_testing) + .ignored_directories_in_source_blocks(je.ignored_directories_in_source_blocks.clone()) .emit_diagnostic(diag); let output = Arc::try_unwrap(output.0).unwrap().into_inner().unwrap(); let output = String::from_utf8(output).unwrap(); @@ -558,7 +566,11 @@ impl DiagnosticSpanLine { .span_to_lines(span) .map(|lines| { // We can't get any lines if the source is unavailable. - if !je.sm.ensure_source_file_source_present(&lines.file) { + if !should_show_source_code( + &je.ignored_directories_in_source_blocks, + &je.sm, + &lines.file, + ) { return vec![]; } diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index 73fd796589543..f2c8f0bc19894 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -1536,6 +1536,8 @@ options! { "generate human-readable, predictable names for codegen units (default: no)"), identify_regions: bool = (false, parse_bool, [UNTRACKED], "display unnamed regions as `'`, using a non-ident unique id (default: no)"), + ignore_directory_in_diagnostics_source_blocks: Vec = (Vec::new(), parse_string_push, [UNTRACKED], + "do not display the source code block in diagnostics for files in the directory"), incremental_ignore_spans: bool = (false, parse_bool, [TRACKED], "ignore spans during ICH computation -- used for testing (default: no)"), incremental_info: bool = (false, parse_bool, [UNTRACKED], diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index 9bff9017881a4..86f4e7b48dab5 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -1295,7 +1295,10 @@ fn default_emitter( .diagnostic_width(sopts.diagnostic_width) .macro_backtrace(macro_backtrace) .track_diagnostics(track_diagnostics) - .terminal_url(terminal_url); + .terminal_url(terminal_url) + .ignored_directories_in_source_blocks( + sopts.unstable_opts.ignore_directory_in_diagnostics_source_blocks.clone(), + ); Box::new(emitter.ui_testing(sopts.unstable_opts.ui_testing)) } } @@ -1312,7 +1315,10 @@ fn default_emitter( track_diagnostics, terminal_url, ) - .ui_testing(sopts.unstable_opts.ui_testing), + .ui_testing(sopts.unstable_opts.ui_testing) + .ignored_directories_in_source_blocks( + sopts.unstable_opts.ignore_directory_in_diagnostics_source_blocks.clone(), + ), ), } } From c230637b92a3101a5b18141b75f94ada2edee776 Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Fri, 15 Sep 2023 16:09:45 +0200 Subject: [PATCH 02/18] avoid blessing cargo deps's source code in ui tests --- Cargo.lock | 10 ++++++++++ src/tools/compiletest/Cargo.toml | 1 + src/tools/compiletest/src/runtest.rs | 9 +++++++++ tests/ui/issues/issue-21763.stderr | 3 --- 4 files changed, 20 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 386b57e6a44e9..d0dde610500c9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -662,6 +662,7 @@ dependencies = [ "diff", "getopts", "glob", + "home", "lazycell", "libc", "miow", @@ -1597,6 +1598,15 @@ version = "0.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7f24254aa9a54b5c858eaee2f5bccdb46aaf0e486a595ed5fd8f86ba55232a70" +[[package]] +name = "home" +version = "0.5.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5444c27eef6923071f7ebcc33e3444508466a76f7a2b93da00ed6e19f30c1ddb" +dependencies = [ + "windows-sys 0.48.0", +] + [[package]] name = "html-checker" version = "0.1.0" diff --git a/src/tools/compiletest/Cargo.toml b/src/tools/compiletest/Cargo.toml index ff1d5cecb7223..bb1fa6e9237df 100644 --- a/src/tools/compiletest/Cargo.toml +++ b/src/tools/compiletest/Cargo.toml @@ -24,6 +24,7 @@ walkdir = "2" glob = "0.3.0" lazycell = "1.3.0" anyhow = "1" +home = "0.5.5" [target.'cfg(unix)'.dependencies] libc = "0.2" diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 65af650f6e55a..855df42753893 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -2335,6 +2335,15 @@ impl<'test> TestCx<'test> { rustc.arg("-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX"); rustc.arg("-Ztranslate-remapped-path-to-local-path=no"); + // Hide Cargo dependency sources from ui tests to make sure the error message doesn't + // change depending on whether $CARGO_HOME is remapped or not. If this is not present, + // when $CARGO_HOME is remapped the source won't be shown, and when it's not remapped the + // source will be shown, causing a blessing hell. + rustc.arg("-Z").arg(format!( + "ignore-directory-in-diagnostics-source-blocks={}", + home::cargo_home().expect("failed to find cargo home").to_str().unwrap() + )); + // Optionally prevent default --sysroot if specified in test compile-flags. if !self.props.compile_flags.iter().any(|flag| flag.starts_with("--sysroot")) && !self.config.host_rustcflags.iter().any(|flag| flag == "--sysroot") diff --git a/tests/ui/issues/issue-21763.stderr b/tests/ui/issues/issue-21763.stderr index df50118ac4774..a887635d35466 100644 --- a/tests/ui/issues/issue-21763.stderr +++ b/tests/ui/issues/issue-21763.stderr @@ -9,9 +9,6 @@ LL | foo::, Rc<()>>>(); = note: required for `hashbrown::raw::RawTable<(Rc<()>, Rc<()>)>` to implement `Send` note: required because it appears within the type `HashMap, Rc<()>, RandomState>` --> $HASHBROWN_SRC_LOCATION - | -LL | pub struct HashMap { - | ^^^^^^^ note: required because it appears within the type `HashMap, Rc<()>>` --> $SRC_DIR/std/src/collections/hash/map.rs:LL:COL note: required by a bound in `foo` From 4690f97099c2f01a07a8fa272e94e3e5cba61f12 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Mon, 18 Sep 2023 21:11:14 +1000 Subject: [PATCH 03/18] coverage: Fix an unstable-sort inconsistency in coverage spans This code was calling `sort_unstable_by`, but failed to impose a total order on the initial spans. That resulted in unpredictable handling of closure spans, producing inconsistencies in the coverage maps and in user-visible coverage reports. This patch fixes the problem by always sorting closure spans before otherwise-identical non-closure spans, and also switches to a stable sort in case the ordering is still not total. --- compiler/rustc_mir_transform/src/coverage/spans.rs | 5 ++++- tests/coverage-map/status-quo/closure.cov-map | 14 +++++++------- tests/coverage-map/status-quo/generator.cov-map | 4 ++-- tests/run-coverage/closure.coverage | 14 +++++++------- 4 files changed, 20 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index 717763a94a0e3..d254c1662e49d 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -333,7 +333,7 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { initial_spans.push(CoverageSpan::for_fn_sig(self.fn_sig_span)); - initial_spans.sort_unstable_by(|a, b| { + initial_spans.sort_by(|a, b| { if a.span.lo() == b.span.lo() { if a.span.hi() == b.span.hi() { if a.is_in_same_bcb(b) { @@ -357,6 +357,9 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { a.span.lo().partial_cmp(&b.span.lo()) } .unwrap() + // If two spans are otherwise identical, put closure spans first, + // as this seems to be what the refinement step expects. + .then_with(|| Ord::cmp(&a.is_closure, &b.is_closure).reverse()) }); initial_spans diff --git a/tests/coverage-map/status-quo/closure.cov-map b/tests/coverage-map/status-quo/closure.cov-map index f3a32f091a91b..7dbf6ec834da2 100644 --- a/tests/coverage-map/status-quo/closure.cov-map +++ b/tests/coverage-map/status-quo/closure.cov-map @@ -1,5 +1,5 @@ Function name: closure::main -Raw bytes (170): 0x[01, 01, 17, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 05, 05, 5a, 01, 05, 18, 01, 08, 01, 0f, 0d, 03, 16, 0e, 06, 0a, 07, 10, 05, 13, 0d, 0b, 1a, 0e, 08, 09, 0f, 10, 05, 0e, 09, 13, 16, 05, 0d, 18, 17, 19, 09, 01, 21, 1b, 04, 09, 00, 29, 1f, 01, 09, 00, 2d, 23, 01, 09, 00, 24, 27, 05, 09, 00, 24, 2b, 02, 09, 00, 21, 2f, 04, 09, 00, 21, 33, 04, 09, 00, 28, 37, 09, 09, 00, 32, 3b, 04, 09, 00, 33, 3f, 07, 09, 00, 4b, 43, 08, 09, 01, 09, 47, 0a, 09, 01, 09, 4b, 08, 09, 01, 09, 4f, 0a, 08, 00, 10, 05, 00, 11, 04, 06, 5a, 04, 06, 00, 07, 57, 01, 05, 03, 02] +Raw bytes (170): 0x[01, 01, 17, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 05, 05, 5a, 01, 05, 18, 01, 08, 01, 0f, 0d, 03, 16, 0e, 06, 0a, 07, 10, 05, 13, 0d, 0b, 1a, 0e, 06, 0a, 0f, 10, 05, 0c, 16, 13, 16, 05, 0d, 18, 17, 19, 09, 01, 1e, 1b, 04, 09, 00, 29, 1f, 01, 09, 00, 2d, 23, 01, 09, 00, 24, 27, 05, 09, 00, 24, 2b, 02, 09, 00, 21, 2f, 04, 09, 00, 21, 33, 04, 09, 00, 28, 37, 09, 09, 00, 32, 3b, 04, 09, 00, 33, 3f, 07, 09, 00, 4b, 43, 08, 09, 00, 48, 47, 0a, 09, 00, 47, 4b, 08, 09, 00, 44, 4f, 0a, 08, 00, 10, 05, 00, 11, 04, 06, 5a, 04, 06, 00, 07, 57, 01, 05, 03, 02] Number of files: 1 - file 0 => global file 1 Number of expressions: 23 @@ -32,13 +32,13 @@ Number of file 0 mappings: 24 = (c0 + Zero) - Code(Expression(1, Add)) at (prev + 16, 5) to (start + 19, 13) = (c0 + Zero) -- Code(Expression(2, Add)) at (prev + 26, 14) to (start + 8, 9) +- Code(Expression(2, Add)) at (prev + 26, 14) to (start + 6, 10) = (c0 + Zero) -- Code(Expression(3, Add)) at (prev + 16, 5) to (start + 14, 9) +- Code(Expression(3, Add)) at (prev + 16, 5) to (start + 12, 22) = (c0 + Zero) - Code(Expression(4, Add)) at (prev + 22, 5) to (start + 13, 24) = (c0 + Zero) -- Code(Expression(5, Add)) at (prev + 25, 9) to (start + 1, 33) +- Code(Expression(5, Add)) at (prev + 25, 9) to (start + 1, 30) = (c0 + Zero) - Code(Expression(6, Add)) at (prev + 4, 9) to (start + 0, 41) = (c0 + Zero) @@ -60,11 +60,11 @@ Number of file 0 mappings: 24 = (c0 + Zero) - Code(Expression(15, Add)) at (prev + 7, 9) to (start + 0, 75) = (c0 + Zero) -- Code(Expression(16, Add)) at (prev + 8, 9) to (start + 1, 9) +- Code(Expression(16, Add)) at (prev + 8, 9) to (start + 0, 72) = (c0 + Zero) -- Code(Expression(17, Add)) at (prev + 10, 9) to (start + 1, 9) +- Code(Expression(17, Add)) at (prev + 10, 9) to (start + 0, 71) = (c0 + Zero) -- Code(Expression(18, Add)) at (prev + 8, 9) to (start + 1, 9) +- Code(Expression(18, Add)) at (prev + 8, 9) to (start + 0, 68) = (c0 + Zero) - Code(Expression(19, Add)) at (prev + 10, 8) to (start + 0, 16) = (c0 + Zero) diff --git a/tests/coverage-map/status-quo/generator.cov-map b/tests/coverage-map/status-quo/generator.cov-map index 6e10b58a94118..a66c1af30f992 100644 --- a/tests/coverage-map/status-quo/generator.cov-map +++ b/tests/coverage-map/status-quo/generator.cov-map @@ -14,7 +14,7 @@ Number of file 0 mappings: 4 = (c1 + (c0 - c1)) Function name: generator::main -Raw bytes (71): 0x[01, 01, 0b, 01, 00, 05, 0b, 09, 0d, 11, 00, 11, 15, 2a, 19, 11, 15, 15, 19, 26, 00, 2a, 19, 11, 15, 09, 01, 0f, 01, 02, 19, 03, 07, 0b, 00, 2e, 11, 01, 2b, 00, 2d, 07, 01, 0e, 00, 35, 0f, 02, 0b, 00, 2e, 2a, 01, 22, 00, 27, 26, 00, 2c, 00, 2e, 1f, 01, 0e, 00, 35, 23, 02, 01, 00, 02] +Raw bytes (71): 0x[01, 01, 0b, 01, 00, 05, 0b, 09, 0d, 11, 00, 11, 15, 2a, 19, 11, 15, 15, 19, 26, 00, 2a, 19, 11, 15, 09, 01, 0f, 01, 02, 16, 03, 07, 0b, 00, 2e, 11, 01, 2b, 00, 2d, 07, 01, 0e, 00, 35, 0f, 02, 0b, 00, 2e, 2a, 01, 22, 00, 27, 26, 00, 2c, 00, 2e, 1f, 01, 0e, 00, 35, 23, 02, 01, 00, 02] Number of files: 1 - file 0 => global file 1 Number of expressions: 11 @@ -30,7 +30,7 @@ Number of expressions: 11 - expression 9 operands: lhs = Expression(10, Sub), rhs = Counter(6) - expression 10 operands: lhs = Counter(4), rhs = Counter(5) Number of file 0 mappings: 9 -- Code(Counter(0)) at (prev + 15, 1) to (start + 2, 25) +- Code(Counter(0)) at (prev + 15, 1) to (start + 2, 22) - Code(Expression(0, Add)) at (prev + 7, 11) to (start + 0, 46) = (c0 + Zero) - Code(Counter(4)) at (prev + 1, 43) to (start + 0, 45) diff --git a/tests/run-coverage/closure.coverage b/tests/run-coverage/closure.coverage index 930348dc4315a..67014f792c818 100644 --- a/tests/run-coverage/closure.coverage +++ b/tests/run-coverage/closure.coverage @@ -76,8 +76,8 @@ LL| 1| some_string = None; LL| 1| let LL| 1| a - LL| 1| = - LL| 1| || + LL| | = + LL| | || LL| 1| { LL| 1| let mut countdown = 0; LL| 1| if is_false { @@ -98,8 +98,8 @@ LL| 1| LL| 1| let LL| 1| quote_closure - LL| 1| = - LL| 1| |val| + LL| | = + LL| | |val| LL| 5| { LL| 5| let mut countdown = 0; LL| 5| if is_false { @@ -186,7 +186,7 @@ LL| | ; LL| | LL| 1| let short_used_not_covered_closure_line_break_block_embedded_branch = - LL| 1| | _unused_arg: u8 | + LL| | | _unused_arg: u8 | LL| 0| { LL| 0| println!( LL| 0| "not called: {}", @@ -196,7 +196,7 @@ LL| | ; LL| | LL| 1| let short_used_covered_closure_line_break_no_block_embedded_branch = - LL| 1| | _unused_arg: u8 | + LL| | | _unused_arg: u8 | LL| 1| println!( LL| 1| "not called: {}", LL| 1| if is_true { "check" } else { "me" } @@ -205,7 +205,7 @@ LL| | ; LL| | LL| 1| let short_used_covered_closure_line_break_block_embedded_branch = - LL| 1| | _unused_arg: u8 | + LL| | | _unused_arg: u8 | LL| 1| { LL| 1| println!( LL| 1| "not called: {}", From a4cb31bb58112b65b4313a2764addf5b72ff7a68 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Mon, 18 Sep 2023 22:19:16 +1000 Subject: [PATCH 04/18] coverage: Regression test for inconsistent handling of closure spans --- .../status-quo/closure_bug.cov-map | 148 ++++++++++++++++++ tests/coverage-map/status-quo/closure_bug.rs | 44 ++++++ tests/run-coverage/closure_bug.coverage | 53 +++++++ tests/run-coverage/closure_bug.rs | 44 ++++++ 4 files changed, 289 insertions(+) create mode 100644 tests/coverage-map/status-quo/closure_bug.cov-map create mode 100644 tests/coverage-map/status-quo/closure_bug.rs create mode 100644 tests/run-coverage/closure_bug.coverage create mode 100644 tests/run-coverage/closure_bug.rs diff --git a/tests/coverage-map/status-quo/closure_bug.cov-map b/tests/coverage-map/status-quo/closure_bug.cov-map new file mode 100644 index 0000000000000..4fe2e5ad243dd --- /dev/null +++ b/tests/coverage-map/status-quo/closure_bug.cov-map @@ -0,0 +1,148 @@ +Function name: closure_bug::main +Raw bytes (241): 0x[01, 01, 34, 01, 00, 01, 05, 05, ce, 01, 01, 05, cb, 01, 00, 05, ce, 01, 01, 05, cb, 01, 09, 05, ce, 01, 01, 05, 09, c6, 01, cb, 01, 09, 05, ce, 01, 01, 05, c3, 01, 00, 09, c6, 01, cb, 01, 09, 05, ce, 01, 01, 05, c3, 01, 0d, 09, c6, 01, cb, 01, 09, 05, ce, 01, 01, 05, 0d, be, 01, c3, 01, 0d, 09, c6, 01, cb, 01, 09, 05, ce, 01, 01, 05, bb, 01, 00, 0d, be, 01, c3, 01, 0d, 09, c6, 01, cb, 01, 09, 05, ce, 01, 01, 05, bb, 01, 11, 0d, be, 01, c3, 01, 0d, 09, c6, 01, cb, 01, 09, 05, ce, 01, 01, 05, 11, b6, 01, bb, 01, 11, 0d, be, 01, c3, 01, 0d, 09, c6, 01, cb, 01, 09, 05, ce, 01, 01, 05, 11, 01, 06, 01, 03, 0a, 03, 09, 05, 01, 0e, 05, 01, 0f, 00, 17, ce, 01, 00, 17, 00, 18, cb, 01, 02, 09, 00, 0a, 13, 06, 05, 01, 0e, 09, 01, 0f, 00, 17, c6, 01, 00, 17, 00, 18, c3, 01, 02, 09, 00, 0a, 3b, 06, 05, 01, 0e, 0d, 01, 0f, 00, 17, be, 01, 00, 17, 00, 18, bb, 01, 02, 09, 00, 0a, 7b, 06, 05, 01, 0e, 11, 01, 0f, 00, 17, b6, 01, 00, 17, 00, 18, b3, 01, 01, 01, 00, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 52 +- expression 0 operands: lhs = Counter(0), rhs = Zero +- expression 1 operands: lhs = Counter(0), rhs = Counter(1) +- expression 2 operands: lhs = Counter(1), rhs = Expression(51, Sub) +- expression 3 operands: lhs = Counter(0), rhs = Counter(1) +- expression 4 operands: lhs = Expression(50, Add), rhs = Zero +- expression 5 operands: lhs = Counter(1), rhs = Expression(51, Sub) +- expression 6 operands: lhs = Counter(0), rhs = Counter(1) +- expression 7 operands: lhs = Expression(50, Add), rhs = Counter(2) +- expression 8 operands: lhs = Counter(1), rhs = Expression(51, Sub) +- expression 9 operands: lhs = Counter(0), rhs = Counter(1) +- expression 10 operands: lhs = Counter(2), rhs = Expression(49, Sub) +- expression 11 operands: lhs = Expression(50, Add), rhs = Counter(2) +- expression 12 operands: lhs = Counter(1), rhs = Expression(51, Sub) +- expression 13 operands: lhs = Counter(0), rhs = Counter(1) +- expression 14 operands: lhs = Expression(48, Add), rhs = Zero +- expression 15 operands: lhs = Counter(2), rhs = Expression(49, Sub) +- expression 16 operands: lhs = Expression(50, Add), rhs = Counter(2) +- expression 17 operands: lhs = Counter(1), rhs = Expression(51, Sub) +- expression 18 operands: lhs = Counter(0), rhs = Counter(1) +- expression 19 operands: lhs = Expression(48, Add), rhs = Counter(3) +- expression 20 operands: lhs = Counter(2), rhs = Expression(49, Sub) +- expression 21 operands: lhs = Expression(50, Add), rhs = Counter(2) +- expression 22 operands: lhs = Counter(1), rhs = Expression(51, Sub) +- expression 23 operands: lhs = Counter(0), rhs = Counter(1) +- expression 24 operands: lhs = Counter(3), rhs = Expression(47, Sub) +- expression 25 operands: lhs = Expression(48, Add), rhs = Counter(3) +- expression 26 operands: lhs = Counter(2), rhs = Expression(49, Sub) +- expression 27 operands: lhs = Expression(50, Add), rhs = Counter(2) +- expression 28 operands: lhs = Counter(1), rhs = Expression(51, Sub) +- expression 29 operands: lhs = Counter(0), rhs = Counter(1) +- expression 30 operands: lhs = Expression(46, Add), rhs = Zero +- expression 31 operands: lhs = Counter(3), rhs = Expression(47, Sub) +- expression 32 operands: lhs = Expression(48, Add), rhs = Counter(3) +- expression 33 operands: lhs = Counter(2), rhs = Expression(49, Sub) +- expression 34 operands: lhs = Expression(50, Add), rhs = Counter(2) +- expression 35 operands: lhs = Counter(1), rhs = Expression(51, Sub) +- expression 36 operands: lhs = Counter(0), rhs = Counter(1) +- expression 37 operands: lhs = Expression(46, Add), rhs = Counter(4) +- expression 38 operands: lhs = Counter(3), rhs = Expression(47, Sub) +- expression 39 operands: lhs = Expression(48, Add), rhs = Counter(3) +- expression 40 operands: lhs = Counter(2), rhs = Expression(49, Sub) +- expression 41 operands: lhs = Expression(50, Add), rhs = Counter(2) +- expression 42 operands: lhs = Counter(1), rhs = Expression(51, Sub) +- expression 43 operands: lhs = Counter(0), rhs = Counter(1) +- expression 44 operands: lhs = Counter(4), rhs = Expression(45, Sub) +- expression 45 operands: lhs = Expression(46, Add), rhs = Counter(4) +- expression 46 operands: lhs = Counter(3), rhs = Expression(47, Sub) +- expression 47 operands: lhs = Expression(48, Add), rhs = Counter(3) +- expression 48 operands: lhs = Counter(2), rhs = Expression(49, Sub) +- expression 49 operands: lhs = Expression(50, Add), rhs = Counter(2) +- expression 50 operands: lhs = Counter(1), rhs = Expression(51, Sub) +- expression 51 operands: lhs = Counter(0), rhs = Counter(1) +Number of file 0 mappings: 17 +- Code(Counter(0)) at (prev + 6, 1) to (start + 3, 10) +- Code(Expression(0, Add)) at (prev + 9, 5) to (start + 1, 14) + = (c0 + Zero) +- Code(Counter(1)) at (prev + 1, 15) to (start + 0, 23) +- Code(Expression(51, Sub)) at (prev + 0, 23) to (start + 0, 24) + = (c0 - c1) +- Code(Expression(50, Add)) at (prev + 2, 9) to (start + 0, 10) + = (c1 + (c0 - c1)) +- Code(Expression(4, Add)) at (prev + 6, 5) to (start + 1, 14) + = ((c1 + (c0 - c1)) + Zero) +- Code(Counter(2)) at (prev + 1, 15) to (start + 0, 23) +- Code(Expression(49, Sub)) at (prev + 0, 23) to (start + 0, 24) + = ((c1 + (c0 - c1)) - c2) +- Code(Expression(48, Add)) at (prev + 2, 9) to (start + 0, 10) + = (c2 + ((c1 + (c0 - c1)) - c2)) +- Code(Expression(14, Add)) at (prev + 6, 5) to (start + 1, 14) + = ((c2 + ((c1 + (c0 - c1)) - c2)) + Zero) +- Code(Counter(3)) at (prev + 1, 15) to (start + 0, 23) +- Code(Expression(47, Sub)) at (prev + 0, 23) to (start + 0, 24) + = ((c2 + ((c1 + (c0 - c1)) - c2)) - c3) +- Code(Expression(46, Add)) at (prev + 2, 9) to (start + 0, 10) + = (c3 + ((c2 + ((c1 + (c0 - c1)) - c2)) - c3)) +- Code(Expression(30, Add)) at (prev + 6, 5) to (start + 1, 14) + = ((c3 + ((c2 + ((c1 + (c0 - c1)) - c2)) - c3)) + Zero) +- Code(Counter(4)) at (prev + 1, 15) to (start + 0, 23) +- Code(Expression(45, Sub)) at (prev + 0, 23) to (start + 0, 24) + = ((c3 + ((c2 + ((c1 + (c0 - c1)) - c2)) - c3)) - c4) +- Code(Expression(44, Add)) at (prev + 1, 1) to (start + 0, 2) + = (c4 + ((c3 + ((c2 + ((c1 + (c0 - c1)) - c2)) - c3)) - c4)) + +Function name: closure_bug::main::{closure#0} +Raw bytes (28): 0x[01, 01, 02, 01, 05, 05, 02, 04, 01, 0d, 09, 00, 12, 05, 00, 15, 00, 19, 02, 00, 23, 00, 28, 07, 00, 29, 00, 2a] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 2 +- expression 0 operands: lhs = Counter(0), rhs = Counter(1) +- expression 1 operands: lhs = Counter(1), rhs = Expression(0, Sub) +Number of file 0 mappings: 4 +- Code(Counter(0)) at (prev + 13, 9) to (start + 0, 18) +- Code(Counter(1)) at (prev + 0, 21) to (start + 0, 25) +- Code(Expression(0, Sub)) at (prev + 0, 35) to (start + 0, 40) + = (c0 - c1) +- Code(Expression(1, Add)) at (prev + 0, 41) to (start + 0, 42) + = (c1 + (c0 - c1)) + +Function name: closure_bug::main::{closure#1} +Raw bytes (28): 0x[01, 01, 02, 01, 05, 05, 02, 04, 01, 16, 09, 00, 12, 05, 00, 15, 00, 19, 02, 00, 23, 00, 28, 07, 00, 29, 00, 2a] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 2 +- expression 0 operands: lhs = Counter(0), rhs = Counter(1) +- expression 1 operands: lhs = Counter(1), rhs = Expression(0, Sub) +Number of file 0 mappings: 4 +- Code(Counter(0)) at (prev + 22, 9) to (start + 0, 18) +- Code(Counter(1)) at (prev + 0, 21) to (start + 0, 25) +- Code(Expression(0, Sub)) at (prev + 0, 35) to (start + 0, 40) + = (c0 - c1) +- Code(Expression(1, Add)) at (prev + 0, 41) to (start + 0, 42) + = (c1 + (c0 - c1)) + +Function name: closure_bug::main::{closure#2} +Raw bytes (28): 0x[01, 01, 02, 01, 05, 05, 02, 04, 01, 1f, 09, 00, 12, 05, 00, 15, 00, 19, 02, 00, 23, 00, 28, 07, 00, 29, 00, 2a] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 2 +- expression 0 operands: lhs = Counter(0), rhs = Counter(1) +- expression 1 operands: lhs = Counter(1), rhs = Expression(0, Sub) +Number of file 0 mappings: 4 +- Code(Counter(0)) at (prev + 31, 9) to (start + 0, 18) +- Code(Counter(1)) at (prev + 0, 21) to (start + 0, 25) +- Code(Expression(0, Sub)) at (prev + 0, 35) to (start + 0, 40) + = (c0 - c1) +- Code(Expression(1, Add)) at (prev + 0, 41) to (start + 0, 42) + = (c1 + (c0 - c1)) + +Function name: closure_bug::main::{closure#3} +Raw bytes (28): 0x[01, 01, 02, 01, 05, 05, 02, 04, 01, 28, 09, 00, 12, 05, 00, 15, 00, 19, 02, 00, 23, 00, 28, 07, 00, 29, 00, 2a] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 2 +- expression 0 operands: lhs = Counter(0), rhs = Counter(1) +- expression 1 operands: lhs = Counter(1), rhs = Expression(0, Sub) +Number of file 0 mappings: 4 +- Code(Counter(0)) at (prev + 40, 9) to (start + 0, 18) +- Code(Counter(1)) at (prev + 0, 21) to (start + 0, 25) +- Code(Expression(0, Sub)) at (prev + 0, 35) to (start + 0, 40) + = (c0 - c1) +- Code(Expression(1, Add)) at (prev + 0, 41) to (start + 0, 42) + = (c1 + (c0 - c1)) + diff --git a/tests/coverage-map/status-quo/closure_bug.rs b/tests/coverage-map/status-quo/closure_bug.rs new file mode 100644 index 0000000000000..739bc5f0b51c9 --- /dev/null +++ b/tests/coverage-map/status-quo/closure_bug.rs @@ -0,0 +1,44 @@ +// Regression test for #115930. +// All of these closures are identical, and should produce identical output in +// the coverage report. However, an unstable sort was causing them to be treated +// inconsistently when preparing coverage spans. + +fn main() { + let truthy = std::env::args().len() == 1; + + let a + = + | + | + if truthy { true } else { false }; + + a(); + if truthy { a(); } + + let b + = + | + | + if truthy { true } else { false }; + + b(); + if truthy { b(); } + + let c + = + | + | + if truthy { true } else { false }; + + c(); + if truthy { c(); } + + let d + = + | + | + if truthy { true } else { false }; + + d(); + if truthy { d(); } +} diff --git a/tests/run-coverage/closure_bug.coverage b/tests/run-coverage/closure_bug.coverage new file mode 100644 index 0000000000000..f3299834bce19 --- /dev/null +++ b/tests/run-coverage/closure_bug.coverage @@ -0,0 +1,53 @@ + LL| |// Regression test for #115930. + LL| |// All of these closures are identical, and should produce identical output in + LL| |// the coverage report. However, an unstable sort was causing them to be treated + LL| |// inconsistently when preparing coverage spans. + LL| | + LL| 1|fn main() { + LL| 1| let truthy = std::env::args().len() == 1; + LL| 1| + LL| 1| let a + LL| | = + LL| | | + LL| | | + LL| 2| if truthy { true } else { false }; + ^0 + LL| | + LL| 1| a(); + LL| 1| if truthy { a(); } + ^0 + LL| | + LL| 1| let b + LL| | = + LL| | | + LL| | | + LL| 2| if truthy { true } else { false }; + ^0 + LL| | + LL| 1| b(); + LL| 1| if truthy { b(); } + ^0 + LL| | + LL| 1| let c + LL| | = + LL| | | + LL| | | + LL| 2| if truthy { true } else { false }; + ^0 + LL| | + LL| 1| c(); + LL| 1| if truthy { c(); } + ^0 + LL| | + LL| 1| let d + LL| | = + LL| | | + LL| | | + LL| 2| if truthy { true } else { false }; + ^0 + LL| | + LL| 1| d(); + LL| 1| if truthy { d(); } + ^0 + LL| 1|} + diff --git a/tests/run-coverage/closure_bug.rs b/tests/run-coverage/closure_bug.rs new file mode 100644 index 0000000000000..739bc5f0b51c9 --- /dev/null +++ b/tests/run-coverage/closure_bug.rs @@ -0,0 +1,44 @@ +// Regression test for #115930. +// All of these closures are identical, and should produce identical output in +// the coverage report. However, an unstable sort was causing them to be treated +// inconsistently when preparing coverage spans. + +fn main() { + let truthy = std::env::args().len() == 1; + + let a + = + | + | + if truthy { true } else { false }; + + a(); + if truthy { a(); } + + let b + = + | + | + if truthy { true } else { false }; + + b(); + if truthy { b(); } + + let c + = + | + | + if truthy { true } else { false }; + + c(); + if truthy { c(); } + + let d + = + | + | + if truthy { true } else { false }; + + d(); + if truthy { d(); } +} From 01b67f4b26f420b8713f11b04594f51e687f95fc Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 17 Sep 2023 14:21:19 +1000 Subject: [PATCH 05/18] coverage: Simplify sorting of coverage spans extracted from MIR Switching to `Ordering::then_with` makes control-flow less complicated, and there is no need to use `partial_cmp` here. --- .../src/graph/dominators/mod.rs | 6 +-- .../rustc_mir_transform/src/coverage/graph.rs | 8 +--- .../rustc_mir_transform/src/coverage/spans.rs | 41 +++++++------------ 3 files changed, 19 insertions(+), 36 deletions(-) diff --git a/compiler/rustc_data_structures/src/graph/dominators/mod.rs b/compiler/rustc_data_structures/src/graph/dominators/mod.rs index 85ef2de9b5e38..4075481e56160 100644 --- a/compiler/rustc_data_structures/src/graph/dominators/mod.rs +++ b/compiler/rustc_data_structures/src/graph/dominators/mod.rs @@ -51,7 +51,7 @@ pub fn dominators(graph: &G) -> Dominators { // Traverse the graph, collecting a number of things: // // * Preorder mapping (to it, and back to the actual ordering) - // * Postorder mapping (used exclusively for rank_partial_cmp on the final product) + // * Postorder mapping (used exclusively for `cmp_in_dominator_order` on the final product) // * Parents for each vertex in the preorder tree // // These are all done here rather than through one of the 'standard' @@ -342,8 +342,8 @@ impl Dominators { /// relationship, the dominator will always precede the dominated. (The relative ordering /// of two unrelated nodes will also be consistent, but otherwise the order has no /// meaning.) This method cannot be used to determine if either Node dominates the other. - pub fn rank_partial_cmp(&self, lhs: Node, rhs: Node) -> Option { - self.post_order_rank[rhs].partial_cmp(&self.post_order_rank[lhs]) + pub fn cmp_in_dominator_order(&self, lhs: Node, rhs: Node) -> Ordering { + self.post_order_rank[rhs].cmp(&self.post_order_rank[lhs]) } /// Returns true if `a` dominates `b`. diff --git a/compiler/rustc_mir_transform/src/coverage/graph.rs b/compiler/rustc_mir_transform/src/coverage/graph.rs index 60461691e7f51..b6b0463614d02 100644 --- a/compiler/rustc_mir_transform/src/coverage/graph.rs +++ b/compiler/rustc_mir_transform/src/coverage/graph.rs @@ -199,12 +199,8 @@ impl CoverageGraph { } #[inline(always)] - pub fn rank_partial_cmp( - &self, - a: BasicCoverageBlock, - b: BasicCoverageBlock, - ) -> Option { - self.dominators.as_ref().unwrap().rank_partial_cmp(a, b) + pub fn cmp_in_dominator_order(&self, a: BasicCoverageBlock, b: BasicCoverageBlock) -> Ordering { + self.dominators.as_ref().unwrap().cmp_in_dominator_order(a, b) } } diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index d254c1662e49d..32e8ca25d31f9 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -12,7 +12,6 @@ use rustc_span::source_map::original_sp; use rustc_span::{BytePos, ExpnKind, MacroKind, Span, Symbol}; use std::cell::OnceCell; -use std::cmp::Ordering; #[derive(Debug, Copy, Clone)] pub(super) enum CoverageStatement { @@ -334,32 +333,20 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { initial_spans.push(CoverageSpan::for_fn_sig(self.fn_sig_span)); initial_spans.sort_by(|a, b| { - if a.span.lo() == b.span.lo() { - if a.span.hi() == b.span.hi() { - if a.is_in_same_bcb(b) { - Some(Ordering::Equal) - } else { - // Sort equal spans by dominator relationship (so dominators always come - // before the dominated equal spans). When later comparing two spans in - // order, the first will either dominate the second, or they will have no - // dominator relationship. - self.basic_coverage_blocks.rank_partial_cmp(a.bcb, b.bcb) - } - } else { - // Sort hi() in reverse order so shorter spans are attempted after longer spans. - // This guarantees that, if a `prev` span overlaps, and is not equal to, a - // `curr` span, the prev span either extends further left of the curr span, or - // they start at the same position and the prev span extends further right of - // the end of the curr span. - b.span.hi().partial_cmp(&a.span.hi()) - } - } else { - a.span.lo().partial_cmp(&b.span.lo()) - } - .unwrap() - // If two spans are otherwise identical, put closure spans first, - // as this seems to be what the refinement step expects. - .then_with(|| Ord::cmp(&a.is_closure, &b.is_closure).reverse()) + // First sort by span start. + Ord::cmp(&a.span.lo(), &b.span.lo()) + // If span starts are the same, sort by span end in reverse order. + // This ensures that if spans A and B are adjacent in the list, + // and they overlap but are not equal, then either: + // - Span A extends further left, or + // - Both have the same start and span A extends further right + .then_with(|| Ord::cmp(&a.span.hi(), &b.span.hi()).reverse()) + // If both spans are equal, sort the BCBs in dominator order, + // so that dominating BCBs come before other BCBs they dominate. + .then_with(|| self.basic_coverage_blocks.cmp_in_dominator_order(a.bcb, b.bcb)) + // If two spans are otherwise identical, put closure spans first, + // as this seems to be what the refinement step expects. + .then_with(|| Ord::cmp(&a.is_closure, &b.is_closure).reverse()) }); initial_spans From 354397f04dff7769c70dbaff9505056523c0bd22 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 18 Sep 2023 15:22:40 +0200 Subject: [PATCH 06/18] Move mobile topbar title creation entirely into JS --- src/librustdoc/html/static/js/main.js | 8 +++++--- src/librustdoc/html/templates/page.html | 3 +-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/librustdoc/html/static/js/main.js b/src/librustdoc/html/static/js/main.js index 097aa0b940d93..eb256455b0878 100644 --- a/src/librustdoc/html/static/js/main.js +++ b/src/librustdoc/html/static/js/main.js @@ -49,10 +49,12 @@ window.currentCrate = getVar("current-crate"); function setMobileTopbar() { // FIXME: It would be nicer to generate this text content directly in HTML, // but with the current code it's hard to get the right information in the right place. - const mobileLocationTitle = document.querySelector(".mobile-topbar h2"); + const mobileTopbar = document.querySelector(".mobile-topbar"); const locationTitle = document.querySelector(".sidebar h2.location"); - if (mobileLocationTitle && locationTitle) { - mobileLocationTitle.innerHTML = locationTitle.innerHTML; + if (mobileTopbar && locationTitle) { + const mobileTitle = document.createElement("h2"); + mobileTitle.innerHTML = locationTitle.innerHTML; + mobileTopbar.appendChild(mobileTitle); } } diff --git a/src/librustdoc/html/templates/page.html b/src/librustdoc/html/templates/page.html index b4b9c31916532..579c782be0975 100644 --- a/src/librustdoc/html/templates/page.html +++ b/src/librustdoc/html/templates/page.html @@ -84,8 +84,7 @@ {# #} {% endif %} {# #} -

{# #} - {# #} + {% endif %}