mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Rollup merge of #115869 - ferrocene:pa-fix-tests-cargo-remap, r=compiler-errors

Browse Source 
Avoid blessing cargo deps's source code in ui tests

Before this PR, the source code of dependencies was included in UI test error messages whenever possible. Unfortunately, "whenever possible" means in some cases the source code wouldn't be injected, resulting in a test failure.

One such case is when `$CARGO_HOME` is remapped to something that is not present on disk [^1]. As the remapped path doesn't exist on disk, the source code wouldn't be showed in `tests/ui/issues/issue-21763.rs`:

```diff
    = note: required for `hashbrown::raw::RawTable<(Rc<()>, Rc<()>)>` to implement `Send`
 note: required because it appears within the type `HashMap<Rc<()>, Rc<()>, RandomState>`
   --> $HASHBROWN_SRC_LOCATION
-   |
-LL | pub struct HashMap<K, V, S = DefaultHashBuilder, A: Allocator + Clone = Global> {
-   |            ^^^^^^^
 note: required because it appears within the type `HashMap<Rc<()>, Rc<()>>`
   --> $SRC_DIR/std/src/collections/hash/map.rs:LL:COL
 note: required by a bound in `foo`
```

This PR fixes the problem by always hiding dependencies source code in the error messages generated during UI tests. This is implemented with a new internal flag, `-Z ignore-directory-in-diagnostics-source-blocks=$path`, which compiletest passes during UI tests. Once this is merged, remapping the Cargo home will be supported.

This PR is best reviewed commit-by-commit.

[^1]: After being puzzled for a bit, I discovered why this never impacted `rust-lang/rust`: we don't remap `$CARGO_HOME` 😅. Instead, we set `$CARGO_HOME` to `/cargo` in CI, which sort-of-but-not-really achieves the same effect.
This commit is contained in:
Matthias Krüger 2023-09-19 01:29:41 +02:00 committed by GitHub
commit 0eec5e3d0c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 69 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
Cargo.lock
View File

@ -662,6 +662,7 @@ dependencies = [
"diff",
"getopts",
"glob",
"home",
"lazycell",
"libc",
"miow",
@ -1663,6 +1664,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"

1
compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs
View File

@ -169,6 +169,7 @@ fn emit_messages_default(
.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)),

27
compiler/rustc_errors/src/emitter.rs
View File

@ -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<String>,
diagnostic_width: Option<usize>,
macro_backtrace: bool,
@ -664,6 +665,7 @@ fn create(dst: Destination, fallback_bundle: LazyFallbackBundle) -> EmitterWrite
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 @@ fn get_multispan_max_line_num(&mut self, msp: &MultiSpan) -> usize {
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 @@ fn emit_message_default(
// 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)
}

16
compiler/rustc_errors/src/json.rs
View File

@ -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<String>,
json_rendered: HumanReadableErrorType,
diagnostic_width: Option<usize>,
macro_backtrace: bool,
@ -73,6 +74,7 @@ pub fn stderr(
fallback_bundle,
pretty,
ui_testing: false,
ignored_directories_in_source_blocks: Vec::new(),
json_rendered,
diagnostic_width,
macro_backtrace,
@ -127,6 +129,7 @@ pub fn new(
fallback_bundle,
pretty,
ui_testing: false,
ignored_directories_in_source_blocks: Vec::new(),
json_rendered,
diagnostic_width,
macro_backtrace,
@ -138,6 +141,10 @@ pub fn new(
pub fn ui_testing(self, ui_testing: bool) -> Self {
Self { ui_testing, ..self }
}
pub fn ignored_directories_in_source_blocks(self, value: Vec<String>) -> Self {
Self { ignored_directories_in_source_blocks: value, ..self }
}
}
impl Translate for JsonEmitter {
@ -381,6 +388,7 @@ fn reset(&mut self) -> io::Result<()> {
.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 @@ fn from_span(span: Span, je: &JsonEmitter) -> Vec<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![];
}

2
compiler/rustc_session/src/options.rs
View File

@ -1536,6 +1536,8 @@ pub(crate) fn parse_dump_solver_proof_tree(
"generate human-readable, predictable names for codegen units (default: no)"),
identify_regions: bool = (false, parse_bool, [UNTRACKED],
"display unnamed regions as `'<id>`, using a non-ident unique id (default: no)"),
ignore_directory_in_diagnostics_source_blocks: Vec<String> = (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],

10
compiler/rustc_session/src/session.rs
View File

@ -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(),
),
),
}
}

1
src/tools/compiletest/Cargo.toml
View File

@ -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"

9
src/tools/compiletest/src/runtest.rs
View File

@ -2335,6 +2335,15 @@ fn make_compile_args(
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")

3
tests/ui/issues/issue-21763.stderr
View File

@ -9,9 +9,6 @@ LL | foo::<HashMap<Rc<()>, Rc<()>>>();
= note: required for `hashbrown::raw::RawTable<(Rc<()>, Rc<()>)>` to implement `Send`
note: required because it appears within the type `HashMap<Rc<()>, Rc<()>, RandomState>`
--> $HASHBROWN_SRC_LOCATION
|
LL | pub struct HashMap<K, V, S = DefaultHashBuilder, A: Allocator + Clone = Global> {
| ^^^^^^^
note: required because it appears within the type `HashMap<Rc<()>, Rc<()>>`
--> $SRC_DIR/std/src/collections/hash/map.rs:LL:COL
note: required by a bound in `foo`