Skip to content

Commit

Permalink
Rollup merge of #102612 - JhonnyBillM:migrate-codegen-ssa-to-diagnost…
Browse files Browse the repository at this point in the history 
…ics-structs, r=davidtwco

Migrate `codegen_ssa` to diagnostics structs - [Part 1]

Initial migration of `codegen_ssa`. Going to split this crate migration in at least two PRs in order to avoid a huge PR and to quick off some questions around:

1. Translating messages from "external" crates.
2. Interfacing with OS messages.
3. Adding UI tests while migrating diagnostics.

_See comments below._
  • Loading branch information
@matthiaskrgr
matthiaskrgr authored Oct 11, 2022
2 parents d10b47e + 13d4f27 commit 24722e8
Show file tree
Hide file tree
Showing 7 changed files with 535 additions and 92 deletions.
82 changes: 30 additions & 52 deletions compiler/rustc_codegen_ssa/src/back/link.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ use super::command::Command;
use super::linker::{self, Linker};
use super::metadata::{create_rmeta_file, MetadataPosition};
use super::rpath::{self, RPathConfig};
use crate::{looks_like_rust_object_file, CodegenResults, CompiledModule, CrateInfo, NativeLib};
use crate::{
errors, looks_like_rust_object_file, CodegenResults, CompiledModule, CrateInfo, NativeLib,
};

use cc::windows_registry;
use regex::Regex;
Expand Down Expand Up @@ -93,7 +95,7 @@ pub fn link_binary<'a>(
let tmpdir = TempFileBuilder::new()
.prefix("rustc")
.tempdir()
.unwrap_or_else(|err| sess.fatal(&format!("couldn't create a temp dir: {}", err)));
.unwrap_or_else(|error| sess.emit_fatal(errors::CreateTempDir { error }));
let path = MaybeTempDir::new(tmpdir, sess.opts.cg.save_temps);
let out_filename = out_filename(
sess,
Expand Down Expand Up @@ -208,7 +210,7 @@ pub fn link_binary<'a>(
pub fn each_linked_rlib(
info: &CrateInfo,
f: &mut dyn FnMut(CrateNum, &Path),
) -> Result<(), String> {
) -> Result<(), errors::LinkRlibError> {
let crates = info.used_crates.iter();
let mut fmts = None;
for (ty, list) in info.dependency_formats.iter() {
Expand All @@ -224,26 +226,23 @@ pub fn each_linked_rlib(
}
}
let Some(fmts) = fmts else {
return Err("could not find formats for rlibs".to_string());
return Err(errors::LinkRlibError::MissingFormat);
};
for &cnum in crates {
match fmts.get(cnum.as_usize() - 1) {
Some(&Linkage::NotLinked | &Linkage::IncludedFromDylib) => continue,
Some(_) => {}
None => return Err("could not find formats for rlibs".to_string()),
None => return Err(errors::LinkRlibError::MissingFormat),
}
let name = info.crate_name[&cnum];
let crate_name = info.crate_name[&cnum];
let used_crate_source = &info.used_crate_source[&cnum];
if let Some((path, _)) = &used_crate_source.rlib {
f(cnum, &path);
} else {
if used_crate_source.rmeta.is_some() {
return Err(format!(
"could not find rlib for: `{}`, found rmeta (metadata) file",
name
));
return Err(errors::LinkRlibError::OnlyRmetaFound { crate_name });
} else {
return Err(format!("could not find rlib for: `{}`", name));
return Err(errors::LinkRlibError::NotFound { crate_name });
}
}
}
Expand Down Expand Up @@ -340,10 +339,7 @@ fn link_rlib<'a>(
// -whole-archive and it isn't clear how we can currently handle such a
// situation correctly.
// See https://github.com/rust-lang/rust/issues/88085#issuecomment-901050897
sess.err(
"the linking modifiers `+bundle` and `+whole-archive` are not compatible \
with each other when generating rlibs",
);
sess.emit_err(errors::IncompatibleLinkingModifiers);
}
NativeLibKind::Static { bundle: None | Some(true), .. } => {}
NativeLibKind::Static { bundle: Some(false), .. }
Expand All @@ -365,12 +361,8 @@ fn link_rlib<'a>(
));
continue;
}
ab.add_archive(&location, Box::new(|_| false)).unwrap_or_else(|e| {
sess.fatal(&format!(
"failed to add native library {}: {}",
location.to_string_lossy(),
e
));
ab.add_archive(&location, Box::new(|_| false)).unwrap_or_else(|error| {
sess.emit_fatal(errors::AddNativeLibrary { library_path: location, error });
});
}
}
Expand All @@ -385,8 +377,8 @@ fn link_rlib<'a>(
tmpdir.as_ref(),
);

ab.add_archive(&output_path, Box::new(|_| false)).unwrap_or_else(|e| {
sess.fatal(&format!("failed to add native library {}: {}", output_path.display(), e));
ab.add_archive(&output_path, Box::new(|_| false)).unwrap_or_else(|error| {
sess.emit_fatal(errors::AddNativeLibrary { library_path: output_path, error });
});
}

Expand Down Expand Up @@ -451,14 +443,11 @@ fn collate_raw_dylibs(
// FIXME: when we add support for ordinals, figure out if we need to do anything
// if we have two DllImport values with the same name but different ordinals.
if import.calling_convention != old_import.calling_convention {
sess.span_err(
import.span,
&format!(
"multiple declarations of external function `{}` from \
library `{}` have different calling conventions",
import.name, name,
),
);
sess.emit_err(errors::MultipleExternalFuncDecl {
span: import.span,
function: import.name,
library_name: &name,
});
}
}
}
Expand Down Expand Up @@ -560,7 +549,7 @@ fn link_staticlib<'a>(
all_native_libs.extend(codegen_results.crate_info.native_libraries[&cnum].iter().cloned());
});
if let Err(e) = res {
sess.fatal(&e);
sess.emit_fatal(e);
}

ab.build(out_filename);
Expand Down Expand Up @@ -673,9 +662,7 @@ fn link_dwarf_object<'a>(
}) {
Ok(()) => {}
Err(e) => {
sess.struct_err("linking dwarf objects with thorin failed")
.note(&format!("{:?}", e))
.emit();
sess.emit_err(errors::ThorinErrorWrapper(e));
sess.abort_if_errors();
}
}
Expand Down Expand Up @@ -879,23 +866,14 @@ fn link_natively<'a>(
let mut output = prog.stderr.clone();
output.extend_from_slice(&prog.stdout);
let escaped_output = escape_string(&output);
let mut err = sess.struct_err(&format!(
"linking with `{}` failed: {}",
linker_path.display(),
prog.status
));
err.note(&format!("{:?}", &cmd)).note(&escaped_output);
if escaped_output.contains("undefined reference to") {
err.help(
"some `extern` functions couldn't be found; some native libraries may \
need to be installed or have their path specified",
);
err.note("use the `-l` flag to specify native libraries to link");
err.note("use the `cargo:rustc-link-lib` directive to specify the native \
libraries to link with Cargo (see https://doc.rust-lang.org/cargo/reference/build-scripts.html#cargorustc-link-libkindname)");
}
err.emit();

// FIXME: Add UI tests for this error.
let err = errors::LinkingFailed {
linker_path: &linker_path,
exit_status: prog.status,
command: &cmd,
escaped_output: &escaped_output,
};
sess.diagnostic().emit_err(err);
// If MSVC's `link.exe` was expected but the return code
// is not a Microsoft LNK error then suggest a way to fix or
// install the Visual Studio build tools.
Expand Down
34 changes: 17 additions & 17 deletions compiler/rustc_codegen_ssa/src/back/linker.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use super::command::Command;
use super::symbol_export;
use crate::errors;
use rustc_span::symbol::sym;

use std::ffi::{OsStr, OsString};
Expand Down Expand Up @@ -434,11 +435,11 @@ impl<'a> Linker for GccLinker<'a> {
// FIXME(81490): ld64 doesn't support these flags but macOS 11
// has -needed-l{} / -needed_library {}
// but we have no way to detect that here.
self.sess.warn("`as-needed` modifier not implemented yet for ld64");
self.sess.emit_warning(errors::Ld64UnimplementedModifier);
} else if self.is_gnu && !self.sess.target.is_like_windows {
self.linker_arg("--no-as-needed");
} else {
self.sess.warn("`as-needed` modifier not supported for current linker");
self.sess.emit_warning(errors::LinkerUnsupportedModifier);
}
}
self.hint_dynamic();
Expand Down Expand Up @@ -492,7 +493,7 @@ impl<'a> Linker for GccLinker<'a> {
// FIXME(81490): ld64 as of macOS 11 supports the -needed_framework
// flag but we have no way to detect that here.
// self.cmd.arg("-needed_framework").arg(framework);
self.sess.warn("`as-needed` modifier not implemented yet for ld64");
self.sess.emit_warning(errors::Ld64UnimplementedModifier);
}
self.cmd.arg("-framework").arg(framework);
}
Expand Down Expand Up @@ -665,8 +666,8 @@ impl<'a> Linker for GccLinker<'a> {
writeln!(f, "_{}", sym)?;
}
};
if let Err(e) = res {
self.sess.fatal(&format!("failed to write lib.def file: {}", e));
if let Err(error) = res {
self.sess.emit_fatal(errors::LibDefWriteFailure { error });
}
} else if is_windows {
let res: io::Result<()> = try {
Expand All @@ -680,8 +681,8 @@ impl<'a> Linker for GccLinker<'a> {
writeln!(f, " {}", symbol)?;
}
};
if let Err(e) = res {
self.sess.fatal(&format!("failed to write list.def file: {}", e));
if let Err(error) = res {
self.sess.emit_fatal(errors::LibDefWriteFailure { error });
}
} else {
// Write an LD version script
Expand All @@ -697,8 +698,8 @@ impl<'a> Linker for GccLinker<'a> {
}
writeln!(f, "\n local:\n *;\n}};")?;
};
if let Err(e) = res {
self.sess.fatal(&format!("failed to write version script: {}", e));
if let Err(error) = res {
self.sess.emit_fatal(errors::VersionScriptWriteFailure { error });
}
}

Expand Down Expand Up @@ -915,9 +916,8 @@ impl<'a> Linker for MsvcLinker<'a> {
self.cmd.arg(arg);
}
}
Err(err) => {
self.sess
.warn(&format!("error enumerating natvis directory: {}", err));
Err(error) => {
self.sess.emit_warning(errors::NoNatvisDirectory { error });
}
}
}
Expand Down Expand Up @@ -971,8 +971,8 @@ impl<'a> Linker for MsvcLinker<'a> {
writeln!(f, " {}", symbol)?;
}
};
if let Err(e) = res {
self.sess.fatal(&format!("failed to write lib.def file: {}", e));
if let Err(error) = res {
self.sess.emit_fatal(errors::LibDefWriteFailure { error });
}
let mut arg = OsString::from("/DEF:");
arg.push(path);
Expand Down Expand Up @@ -1435,7 +1435,7 @@ impl<'a> Linker for L4Bender<'a> {

fn export_symbols(&mut self, _: &Path, _: CrateType, _: &[String]) {
// ToDo, not implemented, copy from GCC
self.sess.warn("exporting symbols not implemented yet for L4Bender");
self.sess.emit_warning(errors::L4BenderExportingSymbolsUnimplemented);
return;
}

Expand Down Expand Up @@ -1727,8 +1727,8 @@ impl<'a> Linker for BpfLinker<'a> {
writeln!(f, "{}", sym)?;
}
};
if let Err(e) = res {
self.sess.fatal(&format!("failed to write symbols file: {}", e));
if let Err(error) = res {
self.sess.emit_fatal(errors::SymbolFileWriteFailure { error });
} else {
self.cmd.arg("--export-symbols").arg(&path);
}
Expand Down
34 changes: 12 additions & 22 deletions compiler/rustc_codegen_ssa/src/back/write.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ use super::link::{self, ensure_removed};
use super::lto::{self, SerializedModule};
use super::symbol_export::symbol_name_for_instance_in_crate;

use crate::errors;
use crate::traits::*;
use crate::{
CachedModuleCodegen, CodegenResults, CompiledModule, CrateInfo, ModuleCodegen, ModuleKind,
};

use crate::traits::*;
use jobserver::{Acquired, Client};
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::memmap::Mmap;
Expand Down Expand Up @@ -530,7 +530,7 @@ fn produce_final_output_artifacts(
// Produce final compile outputs.
let copy_gracefully = |from: &Path, to: &Path| {
if let Err(e) = fs::copy(from, to) {
sess.err(&format!("could not copy {:?} to {:?}: {}", from, to, e));
sess.emit_err(errors::CopyPath::new(from, to, e));
}
};

Expand All @@ -546,7 +546,7 @@ fn produce_final_output_artifacts(
ensure_removed(sess.diagnostic(), &path);
}
} else {
let ext = crate_output
let extension = crate_output
.temp_path(output_type, None)
.extension()
.unwrap()
Expand All @@ -557,19 +557,11 @@ fn produce_final_output_artifacts(
if crate_output.outputs.contains_key(&output_type) {
// 2) Multiple codegen units, with `--emit foo=some_name`. We have
// no good solution for this case, so warn the user.
sess.warn(&format!(
"ignoring emit path because multiple .{} files \
were produced",
ext
));
sess.emit_warning(errors::IgnoringEmitPath { extension });
} else if crate_output.single_output_file.is_some() {
// 3) Multiple codegen units, with `-o some_name`. We have
// no good solution for this case, so warn the user.
sess.warn(&format!(
"ignoring -o because multiple .{} files \
were produced",
ext
));
sess.emit_warning(errors::IgnoringOutput { extension });
} else {
// 4) Multiple codegen units, but no explicit name. We
// just leave the `foo.0.x` files in place.
Expand Down Expand Up @@ -880,14 +872,12 @@ fn execute_copy_from_cache_work_item<B: ExtraBackendMethods>(
);
match link_or_copy(&source_file, &output_path) {
Ok(_) => Some(output_path),
Err(err) => {
let diag_handler = cgcx.create_diag_handler();
diag_handler.err(&format!(
"unable to copy {} to {}: {}",
source_file.display(),
output_path.display(),
err
));
Err(error) => {
cgcx.create_diag_handler().emit_err(errors::CopyPathBuf {
source_file,
output_path,
error,
});
None
}
}
Expand Down
Loading

0 comments on commit 24722e8

Please sign in to comment.