Skip to content

Commit

Permalink
don't ICE when emitting linker errors during -Z link-only
Browse files Browse the repository at this point in the history
note that this still ICEs when passed `-Z link-only --error-format json` because i can't be bothered to fix it right now
  • Loading branch information
jyn514 committed Dec 21, 2024
1 parent f89d8db commit d018f4d
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 21 deletions.
35 changes: 19 additions & 16 deletions compiler/rustc_errors/src/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ pub struct JsonEmitter {
#[setters(skip)]
dst: IntoDynSyncSend<Box<dyn Write + Send>>,
#[setters(skip)]
sm: Lrc<SourceMap>,
sm: Option<Lrc<SourceMap>>,
fluent_bundle: Option<Lrc<FluentBundle>>,
#[setters(skip)]
fallback_bundle: LazyFallbackBundle,
Expand All @@ -65,7 +65,7 @@ pub struct JsonEmitter {
impl JsonEmitter {
pub fn new(
dst: Box<dyn Write + Send>,
sm: Lrc<SourceMap>,
sm: Option<Lrc<SourceMap>>,
fallback_bundle: LazyFallbackBundle,
pretty: bool,
json_rendered: HumanReadableErrorType,
Expand Down Expand Up @@ -171,7 +171,7 @@ impl Emitter for JsonEmitter {
}

fn source_map(&self) -> Option<&SourceMap> {
Some(&self.sm)
self.sm.as_deref()
}

fn should_show_explain(&self) -> bool {
Expand Down Expand Up @@ -371,7 +371,7 @@ impl Diagnostic {
}
HumanEmitter::new(dst, Lrc::clone(&je.fallback_bundle))
.short_message(short)
.sm(Some(Lrc::clone(&je.sm)))
.sm(je.sm.clone())
.fluent_bundle(je.fluent_bundle.clone())
.diagnostic_width(je.diagnostic_width)
.macro_backtrace(je.macro_backtrace)
Expand Down Expand Up @@ -458,22 +458,23 @@ impl DiagnosticSpan {
mut backtrace: impl Iterator<Item = ExpnData>,
je: &JsonEmitter,
) -> DiagnosticSpan {
let start = je.sm.lookup_char_pos(span.lo());
let sm = je.sm.as_ref().expect("JSON spanned output not supported with -Zlink-only");
let start = sm.lookup_char_pos(span.lo());
// If this goes from the start of a line to the end and the replacement
// is an empty string, increase the length to include the newline so we don't
// leave an empty line
if start.col.0 == 0
&& suggestion.map_or(false, |(s, _)| s.is_empty())
&& let Ok(after) = je.sm.span_to_next_source(span)
&& let Ok(after) = sm.span_to_next_source(span)
&& after.starts_with('\n')
{
span = span.with_hi(span.hi() + rustc_span::BytePos(1));
}
let end = je.sm.lookup_char_pos(span.hi());
let end = sm.lookup_char_pos(span.hi());
let backtrace_step = backtrace.next().map(|bt| {
let call_site = Self::from_span_full(bt.call_site, false, None, None, backtrace, je);
let def_site_span = Self::from_span_full(
je.sm.guess_head_span(bt.def_site),
sm.guess_head_span(bt.def_site),
false,
None,
None,
Expand All @@ -488,7 +489,7 @@ impl DiagnosticSpan {
});

DiagnosticSpan {
file_name: je.sm.filename_for_diagnostics(&start.file.name).to_string(),
file_name: sm.filename_for_diagnostics(&start.file.name).to_string(),
byte_start: start.file.original_relative_byte_pos(span.lo()).0,
byte_end: start.file.original_relative_byte_pos(span.hi()).0,
line_start: start.line,
Expand Down Expand Up @@ -558,19 +559,20 @@ impl DiagnosticSpanLine {
/// `span` within the line.
fn from_span(span: Span, je: &JsonEmitter) -> Vec<DiagnosticSpanLine> {
je.sm
.span_to_lines(span)
.map(|lines| {
.as_ref()
.and_then(|sm| {
let lines = sm.span_to_lines(span).ok()?;
// We can't get any lines if the source is unavailable.
if !should_show_source_code(
&je.ignored_directories_in_source_blocks,
&je.sm,
&sm,
&lines.file,
) {
return vec![];
return None;
}

let sf = &*lines.file;
lines
let span_lines = lines
.lines
.iter()
.map(|line| {
Expand All @@ -581,8 +583,9 @@ impl DiagnosticSpanLine {
line.end_col.0 + 1,
)
})
.collect()
.collect();
Some(span_lines)
})
.unwrap_or_else(|_| vec![])
.unwrap_or_default()
}
}
2 changes: 1 addition & 1 deletion compiler/rustc_errors/src/json/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ fn test_positions(code: &str, span: (u32, u32), expected_output: SpanTestData) {
let output = Arc::new(Mutex::new(Vec::new()));
let je = JsonEmitter::new(
Box::new(Shared { data: output.clone() }),
sm,
Some(sm),
fallback_bundle,
true, // pretty
HumanReadableErrorType::Short,
Expand Down
9 changes: 6 additions & 3 deletions compiler/rustc_session/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -890,13 +890,16 @@ fn default_emitter(
}
t => t,
};

let source_map = if sopts.unstable_opts.link_only { None } else { Some(source_map) };

match sopts.error_format {
config::ErrorOutputType::HumanReadable(kind, color_config) => {
let short = kind.short();

if let HumanReadableErrorType::AnnotateSnippet = kind {
let emitter = AnnotateSnippetEmitter::new(
Some(source_map),
source_map,
bundle,
fallback_bundle,
short,
Expand All @@ -906,7 +909,7 @@ fn default_emitter(
} else {
let emitter = HumanEmitter::new(stderr_destination(color_config), fallback_bundle)
.fluent_bundle(bundle)
.sm(Some(source_map))
.sm(source_map)
.short_message(short)
.teach(sopts.unstable_opts.teach)
.diagnostic_width(sopts.diagnostic_width)
Expand Down Expand Up @@ -1437,7 +1440,7 @@ fn mk_emitter(output: ErrorOutputType) -> Box<DynEmitter> {
config::ErrorOutputType::Json { pretty, json_rendered, color_config } => {
Box::new(JsonEmitter::new(
Box::new(io::BufWriter::new(io::stderr())),
Lrc::new(SourceMap::new(FilePathMapping::empty())),
Some(Lrc::new(SourceMap::new(FilePathMapping::empty()))),
fallback_bundle,
pretty,
json_rendered,
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ pub(crate) fn new_dcx(
Box::new(
JsonEmitter::new(
Box::new(io::BufWriter::new(io::stderr())),
source_map,
Some(source_map),
fallback_bundle,
pretty,
json_rendered,
Expand Down
29 changes: 29 additions & 0 deletions tests/run-make/linker-warning/rmake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,33 @@ fn main() {
.assert_stderr_contains("object files omitted")
.assert_stderr_contains_regex(r"\{")
.assert_stderr_not_contains_regex(r"lib(/|\\\\)libstd");

// Make sure we show linker warnings even across `-Z no-link`
rustc()
.arg("-Zno-link")
.input("-")
.stdin_buf("#![deny(linker_messages)] \n fn main() {}")
.run()
.assert_stderr_equals("");
rustc()
.arg("-Zlink-only")
.arg("rust_out.rlink")
.linker("./fake-linker")
.link_arg("run_make_warn")
.run_fail()
// NOTE: the error message here is quite bad (we don't have a source
// span, but still try to print the lint source). But `-Z link-only` is
// unstable and this still shows the linker warning itself so this is
// probably good enough.
.assert_stderr_contains("linker stderr: bar");

// Same thing, but with json output. This currently isn't supported and just ICEs.
rustc()
.arg("-Zlink-only")
.arg("--error-format=json")
.arg("rust_out.rlink")
.linker("./fake-linker")
.link_arg("run_make_warn")
.run_fail()
.assert_stderr_contains("JSON spanned output not supported with -Zlink-only");
}

0 comments on commit d018f4d

Please sign in to comment.