From 1390220ff25d61a36795eacdab3ea104dac6d957 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 26 Jul 2022 01:37:06 +0000 Subject: [PATCH 1/8] Use impl generics when suggesting fix on copy impl --- .../rustc_typeck/src/coherence/builtin.rs | 26 ++++++------------- .../missing-bound-in-manual-copy-impl-2.fixed | 19 ++++++++++++++ .../missing-bound-in-manual-copy-impl-2.rs | 19 ++++++++++++++ ...missing-bound-in-manual-copy-impl-2.stderr | 22 ++++++++++++++++ .../missing-bound-in-manual-copy-impl.fixed | 9 +++++++ .../missing-bound-in-manual-copy-impl.rs | 9 +++++++ .../missing-bound-in-manual-copy-impl.stderr | 17 ++++++++++++ 7 files changed, 103 insertions(+), 18 deletions(-) create mode 100644 src/test/ui/suggestions/missing-bound-in-manual-copy-impl-2.fixed create mode 100644 src/test/ui/suggestions/missing-bound-in-manual-copy-impl-2.rs create mode 100644 src/test/ui/suggestions/missing-bound-in-manual-copy-impl-2.stderr create mode 100644 src/test/ui/suggestions/missing-bound-in-manual-copy-impl.fixed create mode 100644 src/test/ui/suggestions/missing-bound-in-manual-copy-impl.rs create mode 100644 src/test/ui/suggestions/missing-bound-in-manual-copy-impl.stderr diff --git a/compiler/rustc_typeck/src/coherence/builtin.rs b/compiler/rustc_typeck/src/coherence/builtin.rs index d8e42729ff3..c9138bcdab2 100644 --- a/compiler/rustc_typeck/src/coherence/builtin.rs +++ b/compiler/rustc_typeck/src/coherence/builtin.rs @@ -94,14 +94,6 @@ fn visit_implementation_of_copy(tcx: TyCtxt<'_>, impl_did: LocalDefId) { // We'll try to suggest constraining type parameters to fulfill the requirements of // their `Copy` implementation. - let mut generics = None; - if let ty::Adt(def, _substs) = self_type.kind() { - let self_def_id = def.did(); - if let Some(local) = self_def_id.as_local() { - let self_item = tcx.hir().expect_item(local); - generics = self_item.kind.generics(); - } - } let mut errors: BTreeMap<_, Vec<_>> = Default::default(); let mut bounds = vec![]; @@ -163,16 +155,14 @@ fn visit_implementation_of_copy(tcx: TyCtxt<'_>, impl_did: LocalDefId) { &format!("the `Copy` impl for `{}` requires that `{}`", ty, error_predicate), ); } - if let Some(generics) = generics { - suggest_constraining_type_params( - tcx, - generics, - &mut err, - bounds.iter().map(|(param, constraint, def_id)| { - (param.as_str(), constraint.as_str(), *def_id) - }), - ); - } + suggest_constraining_type_params( + tcx, + tcx.hir().get_generics(impl_did).expect("impls always have generics"), + &mut err, + bounds.iter().map(|(param, constraint, def_id)| { + (param.as_str(), constraint.as_str(), *def_id) + }), + ); err.emit(); } Err(CopyImplementationError::NotAnAdt) => { diff --git a/src/test/ui/suggestions/missing-bound-in-manual-copy-impl-2.fixed b/src/test/ui/suggestions/missing-bound-in-manual-copy-impl-2.fixed new file mode 100644 index 00000000000..691e7553a09 --- /dev/null +++ b/src/test/ui/suggestions/missing-bound-in-manual-copy-impl-2.fixed @@ -0,0 +1,19 @@ +// run-rustfix + +#[derive(Clone)] +struct Wrapper(T); + +struct OnlyCopyIfDisplay(std::marker::PhantomData); + +impl Clone for OnlyCopyIfDisplay { + fn clone(&self) -> Self { + OnlyCopyIfDisplay(std::marker::PhantomData) + } +} + +impl Copy for OnlyCopyIfDisplay {} + +impl Copy for Wrapper> {} +//~^ ERROR the trait `Copy` may not be implemented for this type + +fn main() {} diff --git a/src/test/ui/suggestions/missing-bound-in-manual-copy-impl-2.rs b/src/test/ui/suggestions/missing-bound-in-manual-copy-impl-2.rs new file mode 100644 index 00000000000..e3185e7eff8 --- /dev/null +++ b/src/test/ui/suggestions/missing-bound-in-manual-copy-impl-2.rs @@ -0,0 +1,19 @@ +// run-rustfix + +#[derive(Clone)] +struct Wrapper(T); + +struct OnlyCopyIfDisplay(std::marker::PhantomData); + +impl Clone for OnlyCopyIfDisplay { + fn clone(&self) -> Self { + OnlyCopyIfDisplay(std::marker::PhantomData) + } +} + +impl Copy for OnlyCopyIfDisplay {} + +impl Copy for Wrapper> {} +//~^ ERROR the trait `Copy` may not be implemented for this type + +fn main() {} diff --git a/src/test/ui/suggestions/missing-bound-in-manual-copy-impl-2.stderr b/src/test/ui/suggestions/missing-bound-in-manual-copy-impl-2.stderr new file mode 100644 index 00000000000..e0f405eedfa --- /dev/null +++ b/src/test/ui/suggestions/missing-bound-in-manual-copy-impl-2.stderr @@ -0,0 +1,22 @@ +error[E0204]: the trait `Copy` may not be implemented for this type + --> $DIR/missing-bound-in-manual-copy-impl-2.rs:16:9 + | +LL | struct Wrapper(T); + | - this field does not implement `Copy` +... +LL | impl Copy for Wrapper> {} + | ^^^^ + | +note: the `Copy` impl for `OnlyCopyIfDisplay` requires that `S: std::fmt::Display` + --> $DIR/missing-bound-in-manual-copy-impl-2.rs:4:19 + | +LL | struct Wrapper(T); + | ^ +help: consider restricting type parameter `S` + | +LL | impl Copy for Wrapper> {} + | +++++++++++++++++++ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0204`. diff --git a/src/test/ui/suggestions/missing-bound-in-manual-copy-impl.fixed b/src/test/ui/suggestions/missing-bound-in-manual-copy-impl.fixed new file mode 100644 index 00000000000..32a7215c5bd --- /dev/null +++ b/src/test/ui/suggestions/missing-bound-in-manual-copy-impl.fixed @@ -0,0 +1,9 @@ +// run-rustfix + +#[derive(Clone)] +struct Wrapper(T); + +impl Copy for Wrapper {} +//~^ ERROR the trait `Copy` may not be implemented for this type + +fn main() {} diff --git a/src/test/ui/suggestions/missing-bound-in-manual-copy-impl.rs b/src/test/ui/suggestions/missing-bound-in-manual-copy-impl.rs new file mode 100644 index 00000000000..c688f4d41ee --- /dev/null +++ b/src/test/ui/suggestions/missing-bound-in-manual-copy-impl.rs @@ -0,0 +1,9 @@ +// run-rustfix + +#[derive(Clone)] +struct Wrapper(T); + +impl Copy for Wrapper {} +//~^ ERROR the trait `Copy` may not be implemented for this type + +fn main() {} diff --git a/src/test/ui/suggestions/missing-bound-in-manual-copy-impl.stderr b/src/test/ui/suggestions/missing-bound-in-manual-copy-impl.stderr new file mode 100644 index 00000000000..218988511db --- /dev/null +++ b/src/test/ui/suggestions/missing-bound-in-manual-copy-impl.stderr @@ -0,0 +1,17 @@ +error[E0204]: the trait `Copy` may not be implemented for this type + --> $DIR/missing-bound-in-manual-copy-impl.rs:6:9 + | +LL | struct Wrapper(T); + | - this field does not implement `Copy` +LL | +LL | impl Copy for Wrapper {} + | ^^^^ + | +help: consider restricting type parameter `S` + | +LL | impl Copy for Wrapper {} + | ++++++ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0204`. From 7c93154a30a640b8120c5aca68bffb886dcd02e6 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 28 Jul 2022 08:39:19 +0000 Subject: [PATCH 2/8] Move output argument from ArchiveBuilder::new to .build() --- .../rustc_codegen_cranelift/src/archive.rs | 12 ++++----- compiler/rustc_codegen_gcc/src/archive.rs | 18 +++++-------- .../rustc_codegen_llvm/src/back/archive.rs | 13 +++++----- .../rustc_codegen_ssa/src/back/archive.rs | 4 +-- compiler/rustc_codegen_ssa/src/back/link.rs | 26 +++++++------------ 5 files changed, 29 insertions(+), 44 deletions(-) diff --git a/compiler/rustc_codegen_cranelift/src/archive.rs b/compiler/rustc_codegen_cranelift/src/archive.rs index c92c1051139..9d39b4aa661 100644 --- a/compiler/rustc_codegen_cranelift/src/archive.rs +++ b/compiler/rustc_codegen_cranelift/src/archive.rs @@ -19,7 +19,6 @@ enum ArchiveEntry { pub(crate) struct ArArchiveBuilder<'a> { sess: &'a Session, - dst: PathBuf, use_gnu_style_archive: bool, no_builtin_ranlib: bool, @@ -30,10 +29,9 @@ pub(crate) struct ArArchiveBuilder<'a> { } impl<'a> ArchiveBuilder<'a> for ArArchiveBuilder<'a> { - fn new(sess: &'a Session, output: &Path) -> Self { + fn new(sess: &'a Session) -> Self { ArArchiveBuilder { sess, - dst: output.to_path_buf(), use_gnu_style_archive: sess.target.archive_format == "gnu", // FIXME fix builtin ranlib on macOS no_builtin_ranlib: sess.target.is_like_osx, @@ -74,7 +72,7 @@ impl<'a> ArchiveBuilder<'a> for ArArchiveBuilder<'a> { Ok(()) } - fn build(mut self) -> bool { + fn build(mut self, output: &Path) -> bool { enum BuilderKind { Bsd(ar::Builder), Gnu(ar::GnuBuilder), @@ -163,7 +161,7 @@ impl<'a> ArchiveBuilder<'a> for ArArchiveBuilder<'a> { let mut builder = if self.use_gnu_style_archive { BuilderKind::Gnu( ar::GnuBuilder::new( - File::create(&self.dst).unwrap_or_else(|err| { + File::create(output).unwrap_or_else(|err| { sess.fatal(&format!( "error opening destination during archive building: {}", err @@ -178,7 +176,7 @@ impl<'a> ArchiveBuilder<'a> for ArArchiveBuilder<'a> { } else { BuilderKind::Bsd( ar::Builder::new( - File::create(&self.dst).unwrap_or_else(|err| { + File::create(output).unwrap_or_else(|err| { sess.fatal(&format!( "error opening destination during archive building: {}", err @@ -209,7 +207,7 @@ impl<'a> ArchiveBuilder<'a> for ArArchiveBuilder<'a> { // Run ranlib to be able to link the archive let status = std::process::Command::new(ranlib) - .arg(self.dst) + .arg(output) .status() .expect("Couldn't run ranlib"); diff --git a/compiler/rustc_codegen_gcc/src/archive.rs b/compiler/rustc_codegen_gcc/src/archive.rs index 21f62a6b009..a8e94ad4b8d 100644 --- a/compiler/rustc_codegen_gcc/src/archive.rs +++ b/compiler/rustc_codegen_gcc/src/archive.rs @@ -8,7 +8,6 @@ use rustc_session::cstore::DllImport; struct ArchiveConfig<'a> { sess: &'a Session, - dst: PathBuf, use_native_ar: bool, use_gnu_style_archive: bool, } @@ -31,10 +30,9 @@ pub struct ArArchiveBuilder<'a> { } impl<'a> ArchiveBuilder<'a> for ArArchiveBuilder<'a> { - fn new(sess: &'a Session, output: &Path) -> Self { + fn new(sess: &'a Session) -> Self { let config = ArchiveConfig { sess, - dst: output.to_path_buf(), use_native_ar: false, // FIXME test for linux and System V derivatives instead use_gnu_style_archive: sess.target.options.archive_format == "gnu", @@ -77,7 +75,7 @@ impl<'a> ArchiveBuilder<'a> for ArArchiveBuilder<'a> { Ok(()) } - fn build(mut self) -> bool { + fn build(mut self, output: &Path) -> bool { use std::process::Command; fn add_file_using_ar(archive: &Path, file: &Path) { @@ -97,17 +95,17 @@ impl<'a> ArchiveBuilder<'a> for ArArchiveBuilder<'a> { } let mut builder = if self.config.use_native_ar { - BuilderKind::NativeAr(&self.config.dst) + BuilderKind::NativeAr(output) } else if self.config.use_gnu_style_archive { BuilderKind::Gnu(ar::GnuBuilder::new( - File::create(&self.config.dst).unwrap(), + File::create(output).unwrap(), self.entries .iter() .map(|(name, _)| name.as_bytes().to_vec()) .collect(), )) } else { - BuilderKind::Bsd(ar::Builder::new(File::create(&self.config.dst).unwrap())) + BuilderKind::Bsd(ar::Builder::new(File::create(output).unwrap())) }; let any_members = !self.entries.is_empty(); @@ -164,10 +162,8 @@ impl<'a> ArchiveBuilder<'a> for ArArchiveBuilder<'a> { std::mem::drop(builder); // Run ranlib to be able to link the archive - let status = std::process::Command::new("ranlib") - .arg(self.config.dst) - .status() - .expect("Couldn't run ranlib"); + let status = + std::process::Command::new("ranlib").arg(output).status().expect("Couldn't run ranlib"); if !status.success() { self.config.sess.fatal(&format!("Ranlib exited with code {:?}", status.code())); diff --git a/compiler/rustc_codegen_llvm/src/back/archive.rs b/compiler/rustc_codegen_llvm/src/back/archive.rs index baa858709a0..8d6e3673271 100644 --- a/compiler/rustc_codegen_llvm/src/back/archive.rs +++ b/compiler/rustc_codegen_llvm/src/back/archive.rs @@ -18,7 +18,6 @@ use rustc_session::Session; #[must_use = "must call build() to finish building the archive"] pub struct LlvmArchiveBuilder<'a> { sess: &'a Session, - dst: PathBuf, additions: Vec, } @@ -56,8 +55,8 @@ fn llvm_machine_type(cpu: &str) -> LLVMMachineType { impl<'a> ArchiveBuilder<'a> for LlvmArchiveBuilder<'a> { /// Creates a new static archive, ready for modifying the archive specified /// by `config`. - fn new(sess: &'a Session, output: &Path) -> LlvmArchiveBuilder<'a> { - LlvmArchiveBuilder { sess, dst: output.to_path_buf(), additions: Vec::new() } + fn new(sess: &'a Session) -> LlvmArchiveBuilder<'a> { + LlvmArchiveBuilder { sess, additions: Vec::new() } } fn add_archive(&mut self, archive: &Path, skip: F) -> io::Result<()> @@ -88,8 +87,8 @@ impl<'a> ArchiveBuilder<'a> for LlvmArchiveBuilder<'a> { /// Combine the provided files, rlibs, and native libraries into a single /// `Archive`. - fn build(mut self) -> bool { - match self.build_with_llvm() { + fn build(mut self, output: &Path) -> bool { + match self.build_with_llvm(output) { Ok(any_members) => any_members, Err(e) => self.sess.fatal(&format!("failed to build archive: {}", e)), } @@ -241,7 +240,7 @@ impl<'a> ArchiveBuilder<'a> for LlvmArchiveBuilder<'a> { } impl<'a> LlvmArchiveBuilder<'a> { - fn build_with_llvm(&mut self) -> io::Result { + fn build_with_llvm(&mut self, output: &Path) -> io::Result { let kind = &*self.sess.target.archive_format; let kind = kind.parse::().map_err(|_| kind).unwrap_or_else(|kind| { self.sess.fatal(&format!("Don't know how to build archive of type: {}", kind)) @@ -251,7 +250,7 @@ impl<'a> LlvmArchiveBuilder<'a> { let mut strings = Vec::new(); let mut members = Vec::new(); - let dst = CString::new(self.dst.to_str().unwrap())?; + let dst = CString::new(output.to_str().unwrap())?; unsafe { for addition in &mut additions { diff --git a/compiler/rustc_codegen_ssa/src/back/archive.rs b/compiler/rustc_codegen_ssa/src/back/archive.rs index 53550b049db..d511c340802 100644 --- a/compiler/rustc_codegen_ssa/src/back/archive.rs +++ b/compiler/rustc_codegen_ssa/src/back/archive.rs @@ -41,7 +41,7 @@ pub(super) fn find_library( } pub trait ArchiveBuilder<'a> { - fn new(sess: &'a Session, output: &Path) -> Self; + fn new(sess: &'a Session) -> Self; fn add_file(&mut self, path: &Path); @@ -49,7 +49,7 @@ pub trait ArchiveBuilder<'a> { where F: FnMut(&str) -> bool + 'static; - fn build(self) -> bool; + fn build(self, output: &Path) -> bool; fn sess(&self) -> &Session; diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index f0d320c7c21..b1d4012474a 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -101,14 +101,9 @@ pub fn link_binary<'a, B: ArchiveBuilder<'a>>( match crate_type { CrateType::Rlib => { let _timer = sess.timer("link_rlib"); - link_rlib::( - sess, - codegen_results, - RlibFlavor::Normal, - &out_filename, - &path, - )? - .build(); + info!("preparing rlib to {:?}", out_filename); + link_rlib::(sess, codegen_results, RlibFlavor::Normal, &path)? + .build(&out_filename); } CrateType::Staticlib => { link_staticlib::(sess, codegen_results, &out_filename, &path)?; @@ -249,14 +244,11 @@ fn link_rlib<'a, B: ArchiveBuilder<'a>>( sess: &'a Session, codegen_results: &CodegenResults, flavor: RlibFlavor, - out_filename: &Path, tmpdir: &MaybeTempDir, ) -> Result { - info!("preparing rlib to {:?}", out_filename); - let lib_search_paths = archive_search_paths(sess); - let mut ab = ::new(sess, out_filename); + let mut ab = ::new(sess); let trailing_metadata = match flavor { RlibFlavor::Normal => { @@ -451,8 +443,8 @@ fn link_staticlib<'a, B: ArchiveBuilder<'a>>( out_filename: &Path, tempdir: &MaybeTempDir, ) -> Result<(), ErrorGuaranteed> { - let mut ab = - link_rlib::(sess, codegen_results, RlibFlavor::StaticlibBase, out_filename, tempdir)?; + info!("preparing staticlib to {:?}", out_filename); + let mut ab = link_rlib::(sess, codegen_results, RlibFlavor::StaticlibBase, tempdir)?; let mut all_native_libs = vec![]; let res = each_linked_rlib(&codegen_results.crate_info, &mut |cnum, path| { @@ -514,7 +506,7 @@ fn link_staticlib<'a, B: ArchiveBuilder<'a>>( sess.fatal(&e); } - ab.build(); + ab.build(out_filename); if !all_native_libs.is_empty() { if sess.opts.prints.contains(&PrintRequest::NativeStaticLibs) { @@ -2479,7 +2471,7 @@ fn add_upstream_rust_crates<'a, B: ArchiveBuilder<'a>>( let is_builtins = sess.target.no_builtins || !codegen_results.crate_info.is_no_builtins.contains(&cnum); - let mut archive = ::new(sess, &dst); + let mut archive = ::new(sess); if let Err(e) = archive.add_archive(cratepath, move |f| { if f == METADATA_FILENAME { return true; @@ -2510,7 +2502,7 @@ fn add_upstream_rust_crates<'a, B: ArchiveBuilder<'a>>( }) { sess.fatal(&format!("failed to build archive from rlib: {}", e)); } - if archive.build() { + if archive.build(&dst) { link_upstream(&dst); } }); From 90da3c6f2b4db9bde02138830de1ea14982b1512 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 28 Jul 2022 08:43:15 +0000 Subject: [PATCH 3/8] Inline inject_dll_import_lib --- .../rustc_codegen_cranelift/src/archive.rs | 4 ---- compiler/rustc_codegen_gcc/src/archive.rs | 4 ---- .../rustc_codegen_llvm/src/back/archive.rs | 4 ---- .../rustc_codegen_ssa/src/back/archive.rs | 23 ------------------- compiler/rustc_codegen_ssa/src/back/link.rs | 7 +++++- 5 files changed, 6 insertions(+), 36 deletions(-) diff --git a/compiler/rustc_codegen_cranelift/src/archive.rs b/compiler/rustc_codegen_cranelift/src/archive.rs index 9d39b4aa661..e28c3813d3f 100644 --- a/compiler/rustc_codegen_cranelift/src/archive.rs +++ b/compiler/rustc_codegen_cranelift/src/archive.rs @@ -219,10 +219,6 @@ impl<'a> ArchiveBuilder<'a> for ArArchiveBuilder<'a> { any_members } - fn sess(&self) -> &Session { - self.sess - } - fn create_dll_import_lib( _sess: &Session, _lib_name: &str, diff --git a/compiler/rustc_codegen_gcc/src/archive.rs b/compiler/rustc_codegen_gcc/src/archive.rs index a8e94ad4b8d..f7745951d28 100644 --- a/compiler/rustc_codegen_gcc/src/archive.rs +++ b/compiler/rustc_codegen_gcc/src/archive.rs @@ -172,10 +172,6 @@ impl<'a> ArchiveBuilder<'a> for ArArchiveBuilder<'a> { any_members } - fn sess(&self) -> &Session { - self.config.sess - } - fn create_dll_import_lib( _sess: &Session, _lib_name: &str, diff --git a/compiler/rustc_codegen_llvm/src/back/archive.rs b/compiler/rustc_codegen_llvm/src/back/archive.rs index 8d6e3673271..21a1e3a3b82 100644 --- a/compiler/rustc_codegen_llvm/src/back/archive.rs +++ b/compiler/rustc_codegen_llvm/src/back/archive.rs @@ -94,10 +94,6 @@ impl<'a> ArchiveBuilder<'a> for LlvmArchiveBuilder<'a> { } } - fn sess(&self) -> &Session { - self.sess - } - fn create_dll_import_lib( sess: &Session, lib_name: &str, diff --git a/compiler/rustc_codegen_ssa/src/back/archive.rs b/compiler/rustc_codegen_ssa/src/back/archive.rs index d511c340802..4b55085d720 100644 --- a/compiler/rustc_codegen_ssa/src/back/archive.rs +++ b/compiler/rustc_codegen_ssa/src/back/archive.rs @@ -1,4 +1,3 @@ -use rustc_data_structures::temp_dir::MaybeTempDir; use rustc_session::cstore::DllImport; use rustc_session::Session; @@ -51,8 +50,6 @@ pub trait ArchiveBuilder<'a> { fn build(self, output: &Path) -> bool; - fn sess(&self) -> &Session; - /// Creates a DLL Import Library . /// and returns the path on disk to that import library. /// This functions doesn't take `self` so that it can be called from @@ -64,24 +61,4 @@ pub trait ArchiveBuilder<'a> { dll_imports: &[DllImport], tmpdir: &Path, ) -> PathBuf; - - /// Creates a DLL Import Library - /// and adds it to the current compilation's set of archives. - fn inject_dll_import_lib( - &mut self, - lib_name: &str, - dll_imports: &[DllImport], - tmpdir: &MaybeTempDir, - ) { - let output_path = - Self::create_dll_import_lib(self.sess(), lib_name, dll_imports, tmpdir.as_ref()); - - self.add_archive(&output_path, |_| false).unwrap_or_else(|e| { - self.sess().fatal(&format!( - "failed to add native library {}: {}", - output_path.display(), - e - )); - }); - } } diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index b1d4012474a..b9ad7c94773 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -346,7 +346,12 @@ fn link_rlib<'a, B: ArchiveBuilder<'a>>( for (raw_dylib_name, raw_dylib_imports) in collate_raw_dylibs(sess, &codegen_results.crate_info.used_libraries)? { - ab.inject_dll_import_lib(&raw_dylib_name, &raw_dylib_imports, tmpdir); + let output_path = + B::create_dll_import_lib(sess, &raw_dylib_name, &raw_dylib_imports, tmpdir.as_ref()); + + ab.add_archive(&output_path, |_| false).unwrap_or_else(|e| { + sess.fatal(&format!("failed to add native library {}: {}", output_path.display(), e)); + }); } if let Some(trailing_metadata) = trailing_metadata { From 7c6c7e8785fa553baf43ea82382a97ec01466b9b Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 28 Jul 2022 09:07:49 +0000 Subject: [PATCH 4/8] Introduce an ArchiveBuilderBuilder This avoids monomorphizing all linker code for each codegen backend and will allow passing in extra information to the archive builder from the codegen backend. --- .../rustc_codegen_cranelift/src/archive.rs | 60 +++--- compiler/rustc_codegen_cranelift/src/lib.rs | 2 +- compiler/rustc_codegen_gcc/src/archive.rs | 66 ++++--- compiler/rustc_codegen_gcc/src/lib.rs | 3 +- .../rustc_codegen_llvm/src/back/archive.rs | 28 +-- compiler/rustc_codegen_llvm/src/lib.rs | 4 +- .../rustc_codegen_ssa/src/back/archive.rs | 25 ++- compiler/rustc_codegen_ssa/src/back/link.rs | 174 +++++++++++------- 8 files changed, 214 insertions(+), 148 deletions(-) diff --git a/compiler/rustc_codegen_cranelift/src/archive.rs b/compiler/rustc_codegen_cranelift/src/archive.rs index e28c3813d3f..b4c79096170 100644 --- a/compiler/rustc_codegen_cranelift/src/archive.rs +++ b/compiler/rustc_codegen_cranelift/src/archive.rs @@ -5,7 +5,7 @@ use std::fs::File; use std::io::{self, Read, Seek}; use std::path::{Path, PathBuf}; -use rustc_codegen_ssa::back::archive::ArchiveBuilder; +use rustc_codegen_ssa::back::archive::{ArchiveBuilder, ArchiveBuilderBuilder}; use rustc_session::Session; use object::read::archive::ArchiveFile; @@ -17,6 +17,32 @@ enum ArchiveEntry { File(PathBuf), } +pub(crate) struct ArArchiveBuilderBuilder; + +impl ArchiveBuilderBuilder for ArArchiveBuilderBuilder { + fn new_archive_builder<'a>(&self, sess: &'a Session) -> Box + 'a> { + Box::new(ArArchiveBuilder { + sess, + use_gnu_style_archive: sess.target.archive_format == "gnu", + // FIXME fix builtin ranlib on macOS + no_builtin_ranlib: sess.target.is_like_osx, + + src_archives: vec![], + entries: vec![], + }) + } + + fn create_dll_import_lib( + &self, + _sess: &Session, + _lib_name: &str, + _dll_imports: &[rustc_session::cstore::DllImport], + _tmpdir: &Path, + ) -> PathBuf { + bug!("creating dll imports is not supported"); + } +} + pub(crate) struct ArArchiveBuilder<'a> { sess: &'a Session, use_gnu_style_archive: bool, @@ -29,18 +55,6 @@ pub(crate) struct ArArchiveBuilder<'a> { } impl<'a> ArchiveBuilder<'a> for ArArchiveBuilder<'a> { - fn new(sess: &'a Session) -> Self { - ArArchiveBuilder { - sess, - use_gnu_style_archive: sess.target.archive_format == "gnu", - // FIXME fix builtin ranlib on macOS - no_builtin_ranlib: sess.target.is_like_osx, - - src_archives: vec![], - entries: vec![], - } - } - fn add_file(&mut self, file: &Path) { self.entries.push(( file.file_name().unwrap().to_str().unwrap().to_string().into_bytes(), @@ -48,10 +62,11 @@ impl<'a> ArchiveBuilder<'a> for ArArchiveBuilder<'a> { )); } - fn add_archive(&mut self, archive_path: &Path, mut skip: F) -> std::io::Result<()> - where - F: FnMut(&str) -> bool + 'static, - { + fn add_archive( + &mut self, + archive_path: &Path, + mut skip: Box bool + 'static>, + ) -> std::io::Result<()> { let read_cache = ReadCache::new(std::fs::File::open(&archive_path)?); let archive = ArchiveFile::parse(&read_cache).unwrap(); let archive_index = self.src_archives.len(); @@ -72,7 +87,7 @@ impl<'a> ArchiveBuilder<'a> for ArArchiveBuilder<'a> { Ok(()) } - fn build(mut self, output: &Path) -> bool { + fn build(mut self: Box, output: &Path) -> bool { enum BuilderKind { Bsd(ar::Builder), Gnu(ar::GnuBuilder), @@ -218,13 +233,4 @@ impl<'a> ArchiveBuilder<'a> for ArArchiveBuilder<'a> { any_members } - - fn create_dll_import_lib( - _sess: &Session, - _lib_name: &str, - _dll_imports: &[rustc_session::cstore::DllImport], - _tmpdir: &Path, - ) -> PathBuf { - bug!("creating dll imports is not supported"); - } } diff --git a/compiler/rustc_codegen_cranelift/src/lib.rs b/compiler/rustc_codegen_cranelift/src/lib.rs index 568bb20a3f4..bb0793b1deb 100644 --- a/compiler/rustc_codegen_cranelift/src/lib.rs +++ b/compiler/rustc_codegen_cranelift/src/lib.rs @@ -226,7 +226,7 @@ impl CodegenBackend for CraneliftCodegenBackend { ) -> Result<(), ErrorGuaranteed> { use rustc_codegen_ssa::back::link::link_binary; - link_binary::>(sess, &codegen_results, outputs) + link_binary(sess, &crate::archive::ArArchiveBuilderBuilder, &codegen_results, outputs) } } diff --git a/compiler/rustc_codegen_gcc/src/archive.rs b/compiler/rustc_codegen_gcc/src/archive.rs index f7745951d28..f863abdcc97 100644 --- a/compiler/rustc_codegen_gcc/src/archive.rs +++ b/compiler/rustc_codegen_gcc/src/archive.rs @@ -1,7 +1,7 @@ use std::fs::File; use std::path::{Path, PathBuf}; -use rustc_codegen_ssa::back::archive::ArchiveBuilder; +use rustc_codegen_ssa::back::archive::{ArchiveBuilder, ArchiveBuilderBuilder}; use rustc_session::Session; use rustc_session::cstore::DllImport; @@ -21,6 +21,35 @@ enum ArchiveEntry { File(PathBuf), } +pub struct ArArchiveBuilderBuilder; + +impl ArchiveBuilderBuilder for ArArchiveBuilderBuilder { + fn new_archive_builder<'a>(&self, sess: &'a Session) -> Box + 'a> { + let config = ArchiveConfig { + sess, + use_native_ar: false, + // FIXME test for linux and System V derivatives instead + use_gnu_style_archive: sess.target.options.archive_format == "gnu", + }; + + Box::new(ArArchiveBuilder { + config, + src_archives: vec![], + entries: vec![], + }) + } + + fn create_dll_import_lib( + &self, + _sess: &Session, + _lib_name: &str, + _dll_imports: &[DllImport], + _tmpdir: &Path, + ) -> PathBuf { + unimplemented!(); + } +} + pub struct ArArchiveBuilder<'a> { config: ArchiveConfig<'a>, src_archives: Vec<(PathBuf, ar::Archive)>, @@ -30,21 +59,6 @@ pub struct ArArchiveBuilder<'a> { } impl<'a> ArchiveBuilder<'a> for ArArchiveBuilder<'a> { - fn new(sess: &'a Session) -> Self { - let config = ArchiveConfig { - sess, - use_native_ar: false, - // FIXME test for linux and System V derivatives instead - use_gnu_style_archive: sess.target.options.archive_format == "gnu", - }; - - ArArchiveBuilder { - config, - src_archives: vec![], - entries: vec![], - } - } - fn add_file(&mut self, file: &Path) { self.entries.push(( file.file_name().unwrap().to_str().unwrap().to_string(), @@ -52,10 +66,11 @@ impl<'a> ArchiveBuilder<'a> for ArArchiveBuilder<'a> { )); } - fn add_archive(&mut self, archive_path: &Path, mut skip: F) -> std::io::Result<()> - where - F: FnMut(&str) -> bool + 'static, - { + fn add_archive( + &mut self, + archive_path: &Path, + mut skip: Box bool + 'static>, + ) -> std::io::Result<()> { let mut archive = ar::Archive::new(std::fs::File::open(&archive_path)?); let archive_index = self.src_archives.len(); @@ -75,7 +90,7 @@ impl<'a> ArchiveBuilder<'a> for ArArchiveBuilder<'a> { Ok(()) } - fn build(mut self, output: &Path) -> bool { + fn build(mut self: Box, output: &Path) -> bool { use std::process::Command; fn add_file_using_ar(archive: &Path, file: &Path) { @@ -171,13 +186,4 @@ impl<'a> ArchiveBuilder<'a> for ArArchiveBuilder<'a> { any_members } - - fn create_dll_import_lib( - _sess: &Session, - _lib_name: &str, - _dll_imports: &[DllImport], - _tmpdir: &Path, - ) -> PathBuf { - unimplemented!(); - } } diff --git a/compiler/rustc_codegen_gcc/src/lib.rs b/compiler/rustc_codegen_gcc/src/lib.rs index c21e0c5a35b..8a206c0368f 100644 --- a/compiler/rustc_codegen_gcc/src/lib.rs +++ b/compiler/rustc_codegen_gcc/src/lib.rs @@ -133,8 +133,9 @@ impl CodegenBackend for GccCodegenBackend { fn link(&self, sess: &Session, codegen_results: CodegenResults, outputs: &OutputFilenames) -> Result<(), ErrorGuaranteed> { use rustc_codegen_ssa::back::link::link_binary; - link_binary::>( + link_binary( sess, + &crate::archive::ArArchiveBuilderBuilder, &codegen_results, outputs, ) diff --git a/compiler/rustc_codegen_llvm/src/back/archive.rs b/compiler/rustc_codegen_llvm/src/back/archive.rs index 21a1e3a3b82..27039cda253 100644 --- a/compiler/rustc_codegen_llvm/src/back/archive.rs +++ b/compiler/rustc_codegen_llvm/src/back/archive.rs @@ -10,7 +10,7 @@ use std::str; use crate::llvm::archive_ro::{ArchiveRO, Child}; use crate::llvm::{self, ArchiveKind, LLVMMachineType, LLVMRustCOFFShortExport}; -use rustc_codegen_ssa::back::archive::ArchiveBuilder; +use rustc_codegen_ssa::back::archive::{ArchiveBuilder, ArchiveBuilderBuilder}; use rustc_session::cstore::{DllCallingConvention, DllImport}; use rustc_session::Session; @@ -53,16 +53,11 @@ fn llvm_machine_type(cpu: &str) -> LLVMMachineType { } impl<'a> ArchiveBuilder<'a> for LlvmArchiveBuilder<'a> { - /// Creates a new static archive, ready for modifying the archive specified - /// by `config`. - fn new(sess: &'a Session) -> LlvmArchiveBuilder<'a> { - LlvmArchiveBuilder { sess, additions: Vec::new() } - } - - fn add_archive(&mut self, archive: &Path, skip: F) -> io::Result<()> - where - F: FnMut(&str) -> bool + 'static, - { + fn add_archive( + &mut self, + archive: &Path, + skip: Box bool + 'static>, + ) -> io::Result<()> { let archive_ro = match ArchiveRO::open(archive) { Ok(ar) => ar, Err(e) => return Err(io::Error::new(io::ErrorKind::Other, e)), @@ -87,14 +82,23 @@ impl<'a> ArchiveBuilder<'a> for LlvmArchiveBuilder<'a> { /// Combine the provided files, rlibs, and native libraries into a single /// `Archive`. - fn build(mut self, output: &Path) -> bool { + fn build(mut self: Box, output: &Path) -> bool { match self.build_with_llvm(output) { Ok(any_members) => any_members, Err(e) => self.sess.fatal(&format!("failed to build archive: {}", e)), } } +} + +pub struct LlvmArchiveBuilderBuilder; + +impl ArchiveBuilderBuilder for LlvmArchiveBuilderBuilder { + fn new_archive_builder<'a>(&self, sess: &'a Session) -> Box + 'a> { + Box::new(LlvmArchiveBuilder { sess, additions: Vec::new() }) + } fn create_dll_import_lib( + &self, sess: &Session, lib_name: &str, dll_imports: &[DllImport], diff --git a/compiler/rustc_codegen_llvm/src/lib.rs b/compiler/rustc_codegen_llvm/src/lib.rs index 25ce1cef944..eeb1ed61f28 100644 --- a/compiler/rustc_codegen_llvm/src/lib.rs +++ b/compiler/rustc_codegen_llvm/src/lib.rs @@ -370,12 +370,12 @@ impl CodegenBackend for LlvmCodegenBackend { codegen_results: CodegenResults, outputs: &OutputFilenames, ) -> Result<(), ErrorGuaranteed> { - use crate::back::archive::LlvmArchiveBuilder; + use crate::back::archive::LlvmArchiveBuilderBuilder; use rustc_codegen_ssa::back::link::link_binary; // Run the linker on any artifacts that resulted from the LLVM run. // This should produce either a finished executable or library. - link_binary::>(sess, &codegen_results, outputs) + link_binary(sess, &LlvmArchiveBuilderBuilder, &codegen_results, outputs) } } diff --git a/compiler/rustc_codegen_ssa/src/back/archive.rs b/compiler/rustc_codegen_ssa/src/back/archive.rs index 4b55085d720..0d2aa483d3d 100644 --- a/compiler/rustc_codegen_ssa/src/back/archive.rs +++ b/compiler/rustc_codegen_ssa/src/back/archive.rs @@ -39,16 +39,8 @@ pub(super) fn find_library( )); } -pub trait ArchiveBuilder<'a> { - fn new(sess: &'a Session) -> Self; - - fn add_file(&mut self, path: &Path); - - fn add_archive(&mut self, archive: &Path, skip: F) -> io::Result<()> - where - F: FnMut(&str) -> bool + 'static; - - fn build(self, output: &Path) -> bool; +pub trait ArchiveBuilderBuilder { + fn new_archive_builder<'a>(&self, sess: &'a Session) -> Box + 'a>; /// Creates a DLL Import Library . /// and returns the path on disk to that import library. @@ -56,9 +48,22 @@ pub trait ArchiveBuilder<'a> { /// `linker_with_args`, which is specialized on `ArchiveBuilder` but /// doesn't take or create an instance of that type. fn create_dll_import_lib( + &self, sess: &Session, lib_name: &str, dll_imports: &[DllImport], tmpdir: &Path, ) -> PathBuf; } + +pub trait ArchiveBuilder<'a> { + fn add_file(&mut self, path: &Path); + + fn add_archive( + &mut self, + archive: &Path, + skip: Box bool + 'static>, + ) -> io::Result<()>; + + fn build(self: Box, output: &Path) -> bool; +} diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index b9ad7c94773..7e6a5f0366a 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -24,7 +24,7 @@ use rustc_target::spec::crt_objects::{CrtObjects, CrtObjectsFallback}; use rustc_target::spec::{LinkOutputKind, LinkerFlavor, LldFlavor, SplitDebuginfo}; use rustc_target::spec::{PanicStrategy, RelocModel, RelroLevel, SanitizerSet, Target}; -use super::archive::{find_library, ArchiveBuilder}; +use super::archive::{find_library, ArchiveBuilder, ArchiveBuilderBuilder}; use super::command::Command; use super::linker::{self, Linker}; use super::metadata::{create_rmeta_file, MetadataPosition}; @@ -56,8 +56,9 @@ pub fn ensure_removed(diag_handler: &Handler, path: &Path) { /// Performs the linkage portion of the compilation phase. This will generate all /// of the requested outputs for this compilation session. -pub fn link_binary<'a, B: ArchiveBuilder<'a>>( +pub fn link_binary<'a>( sess: &'a Session, + archive_builder_builder: &dyn ArchiveBuilderBuilder, codegen_results: &CodegenResults, outputs: &OutputFilenames, ) -> Result<(), ErrorGuaranteed> { @@ -102,15 +103,28 @@ pub fn link_binary<'a, B: ArchiveBuilder<'a>>( CrateType::Rlib => { let _timer = sess.timer("link_rlib"); info!("preparing rlib to {:?}", out_filename); - link_rlib::(sess, codegen_results, RlibFlavor::Normal, &path)? - .build(&out_filename); + link_rlib( + sess, + archive_builder_builder, + codegen_results, + RlibFlavor::Normal, + &path, + )? + .build(&out_filename); } CrateType::Staticlib => { - link_staticlib::(sess, codegen_results, &out_filename, &path)?; + link_staticlib( + sess, + archive_builder_builder, + codegen_results, + &out_filename, + &path, + )?; } _ => { - link_natively::( + link_natively( sess, + archive_builder_builder, crate_type, &out_filename, codegen_results, @@ -240,15 +254,16 @@ pub fn each_linked_rlib( /// the object file of the crate, but it also contains all of the object files from native /// libraries. This is done by unzipping native libraries and inserting all of the contents into /// this archive. -fn link_rlib<'a, B: ArchiveBuilder<'a>>( +fn link_rlib<'a>( sess: &'a Session, + archive_builder_builder: &dyn ArchiveBuilderBuilder, codegen_results: &CodegenResults, flavor: RlibFlavor, tmpdir: &MaybeTempDir, -) -> Result { +) -> Result + 'a>, ErrorGuaranteed> { let lib_search_paths = archive_search_paths(sess); - let mut ab = ::new(sess); + let mut ab = archive_builder_builder.new_archive_builder(sess); let trailing_metadata = match flavor { RlibFlavor::Normal => { @@ -333,7 +348,7 @@ fn link_rlib<'a, B: ArchiveBuilder<'a>>( if let Some(name) = lib.name { let location = find_library(name.as_str(), lib.verbatim.unwrap_or(false), &lib_search_paths, sess); - ab.add_archive(&location, |_| false).unwrap_or_else(|e| { + ab.add_archive(&location, Box::new(|_| false)).unwrap_or_else(|e| { sess.fatal(&format!( "failed to add native library {}: {}", location.to_string_lossy(), @@ -346,10 +361,14 @@ fn link_rlib<'a, B: ArchiveBuilder<'a>>( for (raw_dylib_name, raw_dylib_imports) in collate_raw_dylibs(sess, &codegen_results.crate_info.used_libraries)? { - let output_path = - B::create_dll_import_lib(sess, &raw_dylib_name, &raw_dylib_imports, tmpdir.as_ref()); + let output_path = archive_builder_builder.create_dll_import_lib( + sess, + &raw_dylib_name, + &raw_dylib_imports, + tmpdir.as_ref(), + ); - ab.add_archive(&output_path, |_| false).unwrap_or_else(|e| { + ab.add_archive(&output_path, Box::new(|_| false)).unwrap_or_else(|e| { sess.fatal(&format!("failed to add native library {}: {}", output_path.display(), e)); }); } @@ -442,14 +461,21 @@ fn collate_raw_dylibs( /// /// There's no need to include metadata in a static archive, so ensure to not link in the metadata /// object file (and also don't prepare the archive with a metadata file). -fn link_staticlib<'a, B: ArchiveBuilder<'a>>( +fn link_staticlib<'a>( sess: &'a Session, + archive_builder_builder: &dyn ArchiveBuilderBuilder, codegen_results: &CodegenResults, out_filename: &Path, tempdir: &MaybeTempDir, ) -> Result<(), ErrorGuaranteed> { info!("preparing staticlib to {:?}", out_filename); - let mut ab = link_rlib::(sess, codegen_results, RlibFlavor::StaticlibBase, tempdir)?; + let mut ab = link_rlib( + sess, + archive_builder_builder, + codegen_results, + RlibFlavor::StaticlibBase, + tempdir, + )?; let mut all_native_libs = vec![]; let res = each_linked_rlib(&codegen_results.crate_info, &mut |cnum, path| { @@ -483,26 +509,29 @@ fn link_staticlib<'a, B: ArchiveBuilder<'a>>( // might be also an extra name suffix let obj_start = name.as_str().to_owned(); - ab.add_archive(path, move |fname: &str| { - // Ignore metadata files, no matter the name. - if fname == METADATA_FILENAME { - return true; - } + ab.add_archive( + path, + Box::new(move |fname: &str| { + // Ignore metadata files, no matter the name. + if fname == METADATA_FILENAME { + return true; + } - // Don't include Rust objects if LTO is enabled - if lto && looks_like_rust_object_file(fname) { - return true; - } + // Don't include Rust objects if LTO is enabled + if lto && looks_like_rust_object_file(fname) { + return true; + } - // Otherwise if this is *not* a rust object and we're skipping - // objects then skip this file - if skip_object_files && (!fname.starts_with(&obj_start) || !fname.ends_with(".o")) { - return true; - } + // Otherwise if this is *not* a rust object and we're skipping + // objects then skip this file + if skip_object_files && (!fname.starts_with(&obj_start) || !fname.ends_with(".o")) { + return true; + } - // ok, don't skip this - false - }) + // ok, don't skip this + false + }), + ) .unwrap(); all_native_libs.extend(codegen_results.crate_info.native_libraries[&cnum].iter().cloned()); @@ -641,8 +670,9 @@ fn link_dwarf_object<'a>( /// /// This will invoke the system linker/cc to create the resulting file. This links to all upstream /// files as well. -fn link_natively<'a, B: ArchiveBuilder<'a>>( +fn link_natively<'a>( sess: &'a Session, + archive_builder_builder: &dyn ArchiveBuilderBuilder, crate_type: CrateType, out_filename: &Path, codegen_results: &CodegenResults, @@ -650,10 +680,11 @@ fn link_natively<'a, B: ArchiveBuilder<'a>>( ) -> Result<(), ErrorGuaranteed> { info!("preparing {:?} to {:?}", crate_type, out_filename); let (linker_path, flavor) = linker_and_flavor(sess); - let mut cmd = linker_with_args::( + let mut cmd = linker_with_args( &linker_path, flavor, sess, + archive_builder_builder, crate_type, tmpdir, out_filename, @@ -1839,10 +1870,11 @@ fn add_rpath_args( /// to the linking process as a whole. /// Order-independent options may still override each other in order-dependent fashion, /// e.g `--foo=yes --foo=no` may be equivalent to `--foo=no`. -fn linker_with_args<'a, B: ArchiveBuilder<'a>>( +fn linker_with_args<'a>( path: &Path, flavor: LinkerFlavor, sess: &'a Session, + archive_builder_builder: &dyn ArchiveBuilderBuilder, crate_type: CrateType, tmpdir: &Path, out_filename: &Path, @@ -1943,7 +1975,14 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>( } // Upstream rust libraries and their non-bundled static libraries - add_upstream_rust_crates::(cmd, sess, codegen_results, crate_type, tmpdir); + add_upstream_rust_crates( + cmd, + sess, + archive_builder_builder, + codegen_results, + crate_type, + tmpdir, + ); // Upstream dynamic native libraries linked with `#[link]` attributes at and `-l` // command line options. @@ -1958,7 +1997,7 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>( for (raw_dylib_name, raw_dylib_imports) in collate_raw_dylibs(sess, &codegen_results.crate_info.used_libraries)? { - cmd.add_object(&B::create_dll_import_lib( + cmd.add_object(&archive_builder_builder.create_dll_import_lib( sess, &raw_dylib_name, &raw_dylib_imports, @@ -2248,9 +2287,10 @@ fn add_local_native_libraries( /// /// Rust crates are not considered at all when creating an rlib output. All dependencies will be /// linked when producing the final output (instead of the intermediate rlib version). -fn add_upstream_rust_crates<'a, B: ArchiveBuilder<'a>>( +fn add_upstream_rust_crates<'a>( cmd: &mut dyn Linker, sess: &'a Session, + archive_builder_builder: &dyn ArchiveBuilderBuilder, codegen_results: &CodegenResults, crate_type: CrateType, tmpdir: &Path, @@ -2339,7 +2379,7 @@ fn add_upstream_rust_crates<'a, B: ArchiveBuilder<'a>>( let src = &codegen_results.crate_info.used_crate_source[&cnum]; match data[cnum.as_usize() - 1] { _ if codegen_results.crate_info.profiler_runtime == Some(cnum) => { - add_static_crate::(cmd, sess, codegen_results, tmpdir, cnum); + add_static_crate(cmd, sess, archive_builder_builder, codegen_results, tmpdir, cnum); } // compiler-builtins are always placed last to ensure that they're // linked correctly. @@ -2349,7 +2389,7 @@ fn add_upstream_rust_crates<'a, B: ArchiveBuilder<'a>>( } Linkage::NotLinked | Linkage::IncludedFromDylib => {} Linkage::Static => { - add_static_crate::(cmd, sess, codegen_results, tmpdir, cnum); + add_static_crate(cmd, sess, archive_builder_builder, codegen_results, tmpdir, cnum); // Link static native libs with "-bundle" modifier only if the crate they originate from // is being linked statically to the current crate. If it's linked dynamically @@ -2408,7 +2448,7 @@ fn add_upstream_rust_crates<'a, B: ArchiveBuilder<'a>>( // was already "included" in a dylib (e.g., `libstd` when `-C prefer-dynamic` // is used) if let Some(cnum) = compiler_builtins { - add_static_crate::(cmd, sess, codegen_results, tmpdir, cnum); + add_static_crate(cmd, sess, archive_builder_builder, codegen_results, tmpdir, cnum); } // Converts a library file-stem into a cc -l argument @@ -2434,9 +2474,10 @@ fn add_upstream_rust_crates<'a, B: ArchiveBuilder<'a>>( // Note, however, that if we're not doing LTO we can just pass the rlib // blindly to the linker (fast) because it's fine if it's not actually // included as we're at the end of the dependency chain. - fn add_static_crate<'a, B: ArchiveBuilder<'a>>( + fn add_static_crate<'a>( cmd: &mut dyn Linker, sess: &'a Session, + archive_builder_builder: &dyn ArchiveBuilderBuilder, codegen_results: &CodegenResults, tmpdir: &Path, cnum: CrateNum, @@ -2476,35 +2517,38 @@ fn add_upstream_rust_crates<'a, B: ArchiveBuilder<'a>>( let is_builtins = sess.target.no_builtins || !codegen_results.crate_info.is_no_builtins.contains(&cnum); - let mut archive = ::new(sess); - if let Err(e) = archive.add_archive(cratepath, move |f| { - if f == METADATA_FILENAME { - return true; - } + let mut archive = archive_builder_builder.new_archive_builder(sess); + if let Err(e) = archive.add_archive( + cratepath, + Box::new(move |f| { + if f == METADATA_FILENAME { + return true; + } - let canonical = f.replace('-', "_"); + let canonical = f.replace('-', "_"); - let is_rust_object = - canonical.starts_with(&canonical_name) && looks_like_rust_object_file(&f); + let is_rust_object = + canonical.starts_with(&canonical_name) && looks_like_rust_object_file(&f); - // If we've been requested to skip all native object files - // (those not generated by the rust compiler) then we can skip - // this file. See above for why we may want to do this. - let skip_because_cfg_say_so = skip_native && !is_rust_object; + // If we've been requested to skip all native object files + // (those not generated by the rust compiler) then we can skip + // this file. See above for why we may want to do this. + let skip_because_cfg_say_so = skip_native && !is_rust_object; - // If we're performing LTO and this is a rust-generated object - // file, then we don't need the object file as it's part of the - // LTO module. Note that `#![no_builtins]` is excluded from LTO, - // though, so we let that object file slide. - let skip_because_lto = - upstream_rust_objects_already_included && is_rust_object && is_builtins; + // If we're performing LTO and this is a rust-generated object + // file, then we don't need the object file as it's part of the + // LTO module. Note that `#![no_builtins]` is excluded from LTO, + // though, so we let that object file slide. + let skip_because_lto = + upstream_rust_objects_already_included && is_rust_object && is_builtins; - if skip_because_cfg_say_so || skip_because_lto { - return true; - } + if skip_because_cfg_say_so || skip_because_lto { + return true; + } - false - }) { + false + }), + ) { sess.fatal(&format!("failed to build archive from rlib: {}", e)); } if archive.build(&dst) { From 6f194d70d26451bbdc865551552606d98fb3586b Mon Sep 17 00:00:00 2001 From: Martin Nordholts Date: Fri, 29 Jul 2022 22:04:06 +0200 Subject: [PATCH 5/8] triagebot.yml: CC Enselic when rustdoc-json-types changes --- triagebot.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/triagebot.toml b/triagebot.toml index f2930b50e3e..ea654192bbf 100644 --- a/triagebot.toml +++ b/triagebot.toml @@ -323,6 +323,7 @@ otherwise, make sure you bump the `FORMAT_VERSION` constant. cc = [ "@CraftSpider", "@aDotInTheVoid", + "@Enselic", ] [mentions."src/tools/cargo"] From cf2433a74f7474f7d41e9842d7de465b0966831a Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Tue, 12 Jul 2022 09:10:22 -0500 Subject: [PATCH 6/8] Use LocalDefId for closures more --- .../src/diagnostics/conflict_errors.rs | 16 ++--- .../rustc_borrowck/src/diagnostics/mod.rs | 31 ++++---- .../src/diagnostics/mutability_errors.rs | 10 +-- .../src/diagnostics/region_name.rs | 2 +- compiler/rustc_borrowck/src/lib.rs | 4 +- compiler/rustc_borrowck/src/type_check/mod.rs | 17 ++--- .../src/interpret/validity.rs | 2 +- compiler/rustc_middle/src/mir/mod.rs | 70 ++++++++----------- compiler/rustc_middle/src/mir/syntax.rs | 16 +++-- compiler/rustc_middle/src/mir/tcx.rs | 4 +- compiler/rustc_middle/src/query/mod.rs | 4 +- compiler/rustc_middle/src/thir.rs | 3 +- compiler/rustc_middle/src/ty/closure.rs | 4 +- compiler/rustc_middle/src/ty/context.rs | 6 +- .../src/build/expr/as_place.rs | 18 ++--- compiler/rustc_mir_build/src/build/mod.rs | 4 +- .../rustc_mir_build/src/check_unsafety.rs | 1 - compiler/rustc_mir_build/src/thir/cx/expr.rs | 1 + .../rustc_mir_transform/src/check_unsafety.rs | 2 +- compiler/rustc_monomorphize/src/util.rs | 4 +- compiler/rustc_passes/src/liveness.rs | 2 +- compiler/rustc_query_impl/src/keys.rs | 10 +++ compiler/rustc_typeck/src/check/callee.rs | 12 ++-- .../rustc_typeck/src/check/fn_ctxt/_impl.rs | 5 +- compiler/rustc_typeck/src/check/inherited.rs | 5 +- compiler/rustc_typeck/src/check/upvar.rs | 47 ++++++------- compiler/rustc_typeck/src/check/writeback.rs | 25 +++---- compiler/rustc_typeck/src/expr_use_visitor.rs | 8 +-- src/tools/clippy/clippy_utils/src/lib.rs | 2 +- 29 files changed, 164 insertions(+), 171 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 1b3c6cac9c4..826e9181589 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -6,7 +6,6 @@ use rustc_errors::{ struct_span_err, Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed, MultiSpan, }; use rustc_hir as hir; -use rustc_hir::def_id::DefId; use rustc_hir::intravisit::{walk_block, walk_expr, Visitor}; use rustc_hir::{AsyncGeneratorKind, GeneratorKind}; use rustc_infer::infer::TyCtxtInferExt; @@ -21,6 +20,7 @@ use rustc_middle::ty::{ self, subst::Subst, suggest_constraining_type_params, EarlyBinder, PredicateKind, Ty, }; use rustc_mir_dataflow::move_paths::{InitKind, MoveOutIndex, MovePathIndex}; +use rustc_span::def_id::LocalDefId; use rustc_span::hygiene::DesugaringKind; use rustc_span::symbol::sym; use rustc_span::{BytePos, Span, Symbol}; @@ -2224,7 +2224,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { let ty = self.infcx.tcx.type_of(self.mir_def_id()); match ty.kind() { ty::FnDef(_, _) | ty::FnPtr(_) => self.annotate_fn_sig( - self.mir_def_id().to_def_id(), + self.mir_def_id(), self.infcx.tcx.fn_sig(self.mir_def_id()), ), _ => None, @@ -2268,8 +2268,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { // Check if our `target` was captured by a closure. if let Rvalue::Aggregate( box AggregateKind::Closure(def_id, substs), - operands, - ) = rvalue + ref operands, + ) = *rvalue { for operand in operands { let (Operand::Copy(assigned_from) | Operand::Move(assigned_from)) = operand else { @@ -2293,7 +2293,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { // into a place then we should annotate the closure in // case it ends up being assigned into the return place. annotated_closure = - self.annotate_fn_sig(*def_id, substs.as_closure().sig()); + self.annotate_fn_sig(def_id, substs.as_closure().sig()); debug!( "annotate_argument_and_return_for_borrow: \ annotated_closure={:?} assigned_from_local={:?} \ @@ -2415,12 +2415,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { /// references. fn annotate_fn_sig( &self, - did: DefId, + did: LocalDefId, sig: ty::PolyFnSig<'tcx>, ) -> Option> { debug!("annotate_fn_sig: did={:?} sig={:?}", did, sig); - let is_closure = self.infcx.tcx.is_closure(did); - let fn_hir_id = self.infcx.tcx.hir().local_def_id_to_hir_id(did.as_local()?); + let is_closure = self.infcx.tcx.is_closure(did.to_def_id()); + let fn_hir_id = self.infcx.tcx.hir().local_def_id_to_hir_id(did); let fn_decl = self.infcx.tcx.hir().fn_decl_by_hir_id(fn_hir_id)?; // We need to work out which arguments to highlight. We do this by looking diff --git a/compiler/rustc_borrowck/src/diagnostics/mod.rs b/compiler/rustc_borrowck/src/diagnostics/mod.rs index 53c07a3d481..0300180f80a 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mod.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mod.rs @@ -5,9 +5,9 @@ use rustc_const_eval::util::{call_kind, CallDesugaringKind}; use rustc_errors::{Applicability, Diagnostic}; use rustc_hir as hir; use rustc_hir::def::Namespace; -use rustc_hir::def_id::DefId; use rustc_hir::GeneratorKind; use rustc_infer::infer::TyCtxtInferExt; +use rustc_middle::mir::tcx::PlaceTy; use rustc_middle::mir::{ AggregateKind, Constant, FakeReadCause, Field, Local, LocalInfo, LocalKind, Location, Operand, Place, PlaceRef, ProjectionElem, Rvalue, Statement, StatementKind, Terminator, TerminatorKind, @@ -15,6 +15,7 @@ use rustc_middle::mir::{ use rustc_middle::ty::print::Print; use rustc_middle::ty::{self, DefIdTree, Instance, Ty, TyCtxt}; use rustc_mir_dataflow::move_paths::{InitLocation, LookupResult}; +use rustc_span::def_id::LocalDefId; use rustc_span::{symbol::sym, Span, DUMMY_SP}; use rustc_target::abi::VariantIdx; use rustc_trait_selection::traits::type_known_to_meet_bound_modulo_regions; @@ -41,7 +42,6 @@ pub(crate) use outlives_suggestion::OutlivesSuggestionBuilder; pub(crate) use region_errors::{ErrorConstraintInfo, RegionErrorKind, RegionErrors}; pub(crate) use region_name::{RegionName, RegionNameSource}; pub(crate) use rustc_const_eval::util::CallKind; -use rustc_middle::mir::tcx::PlaceTy; pub(super) struct IncludingDowncast(pub(super) bool); @@ -325,10 +325,11 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { // so it's safe to call `expect_local`. // // We know the field exists so it's safe to call operator[] and `unwrap` here. + let def_id = def_id.expect_local(); let var_id = self .infcx .tcx - .typeck(def_id.expect_local()) + .typeck(def_id) .closure_min_captures_flattened(def_id) .nth(field.index()) .unwrap() @@ -715,12 +716,11 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { debug!("move_spans: moved_place={:?} location={:?} stmt={:?}", moved_place, location, stmt); if let StatementKind::Assign(box (_, Rvalue::Aggregate(ref kind, ref places))) = stmt.kind { - match kind { - box AggregateKind::Closure(def_id, _) - | box AggregateKind::Generator(def_id, _, _) => { + match **kind { + AggregateKind::Closure(def_id, _) | AggregateKind::Generator(def_id, _, _) => { debug!("move_spans: def_id={:?} places={:?}", def_id, places); if let Some((args_span, generator_kind, capture_kind_span, path_span)) = - self.closure_span(*def_id, moved_place, places) + self.closure_span(def_id, moved_place, places) { return ClosureUse { generator_kind, @@ -847,7 +847,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { if let StatementKind::Assign(box (_, Rvalue::Aggregate(ref kind, ref places))) = stmt.kind { - let (def_id, is_generator) = match kind { + let (&def_id, is_generator) = match kind { box AggregateKind::Closure(def_id, _) => (def_id, false), box AggregateKind::Generator(def_id, _, _) => (def_id, true), _ => continue, @@ -858,7 +858,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { def_id, is_generator, places ); if let Some((args_span, generator_kind, capture_kind_span, path_span)) = - self.closure_span(*def_id, Place::from(target).as_ref(), places) + self.closure_span(def_id, Place::from(target).as_ref(), places) { return ClosureUse { generator_kind, args_span, capture_kind_span, path_span }; } else { @@ -879,7 +879,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { /// The second span is the location the use resulting in the captured path of the capture fn closure_span( &self, - def_id: DefId, + def_id: LocalDefId, target_place: PlaceRef<'tcx>, places: &[Operand<'tcx>], ) -> Option<(Span, Option, Span, Span)> { @@ -887,17 +887,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { "closure_span: def_id={:?} target_place={:?} places={:?}", def_id, target_place, places ); - let local_did = def_id.as_local()?; - let hir_id = self.infcx.tcx.hir().local_def_id_to_hir_id(local_did); + let hir_id = self.infcx.tcx.hir().local_def_id_to_hir_id(def_id); let expr = &self.infcx.tcx.hir().expect_expr(hir_id).kind; debug!("closure_span: hir_id={:?} expr={:?}", hir_id, expr); if let hir::ExprKind::Closure(&hir::Closure { body, fn_decl_span, .. }) = expr { - for (captured_place, place) in self - .infcx - .tcx - .typeck(def_id.expect_local()) - .closure_min_captures_flattened(def_id) - .zip(places) + for (captured_place, place) in + self.infcx.tcx.typeck(def_id).closure_min_captures_flattened(def_id).zip(places) { match place { Operand::Copy(place) | Operand::Move(place) diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs index 8134e122662..ec95b786707 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs @@ -343,7 +343,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { ); let tcx = self.infcx.tcx; if let ty::Closure(id, _) = *the_place_err.ty(self.body, tcx).ty.kind() { - self.show_mutating_upvar(tcx, id, the_place_err, &mut err); + self.show_mutating_upvar(tcx, id.expect_local(), the_place_err, &mut err); } } @@ -382,7 +382,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { if let ty::Ref(_, ty, Mutability::Mut) = the_place_err.ty(self.body, tcx).ty.kind() && let ty::Closure(id, _) = *ty.kind() { - self.show_mutating_upvar(tcx, id, the_place_err, &mut err); + self.show_mutating_upvar(tcx, id.expect_local(), the_place_err, &mut err); } } @@ -685,11 +685,10 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { fn show_mutating_upvar( &self, tcx: TyCtxt<'_>, - id: hir::def_id::DefId, + closure_local_def_id: hir::def_id::LocalDefId, the_place_err: PlaceRef<'tcx>, err: &mut Diagnostic, ) { - let closure_local_def_id = id.expect_local(); let tables = tcx.typeck(closure_local_def_id); let closure_hir_id = tcx.hir().local_def_id_to_hir_id(closure_local_def_id); if let Some((span, closure_kind_origin)) = @@ -699,7 +698,8 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { let upvar = ty::place_to_string_for_capture(tcx, closure_kind_origin); let root_hir_id = upvar_id.var_path.hir_id; // we have an origin for this closure kind starting at this root variable so it's safe to unwrap here - let captured_places = tables.closure_min_captures[&id].get(&root_hir_id).unwrap(); + let captured_places = + tables.closure_min_captures[&closure_local_def_id].get(&root_hir_id).unwrap(); let origin_projection = closure_kind_origin .projections diff --git a/compiler/rustc_borrowck/src/diagnostics/region_name.rs b/compiler/rustc_borrowck/src/diagnostics/region_name.rs index 0662d4d882f..f68358ecfe6 100644 --- a/compiler/rustc_borrowck/src/diagnostics/region_name.rs +++ b/compiler/rustc_borrowck/src/diagnostics/region_name.rs @@ -189,7 +189,7 @@ impl Display for RegionName { impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { pub(crate) fn mir_def_id(&self) -> hir::def_id::LocalDefId { - self.body.source.def_id().as_local().unwrap() + self.body.source.def_id().expect_local() } pub(crate) fn mir_hir_id(&self) -> hir::HirId { diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index 3e2d57ae00c..4f2a7bccefb 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -189,7 +189,7 @@ fn do_mir_borrowck<'a, 'tcx>( errors.set_tainted_by_errors(); } let upvars: Vec<_> = tables - .closure_min_captures_flattened(def.did.to_def_id()) + .closure_min_captures_flattened(def.did) .map(|captured_place| { let capture = captured_place.info.capture_kind; let by_ref = match capture { @@ -1295,7 +1295,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { match **aggregate_kind { AggregateKind::Closure(def_id, _) | AggregateKind::Generator(def_id, _, _) => { let BorrowCheckResult { used_mut_upvars, .. } = - self.infcx.tcx.mir_borrowck(def_id.expect_local()); + self.infcx.tcx.mir_borrowck(def_id); debug!("{:?} used_mut_upvars={:?}", def_id, used_mut_upvars); for field in used_mut_upvars { self.propagate_closure_used_mut_upvar(&operands[field.index()]); diff --git a/compiler/rustc_borrowck/src/type_check/mod.rs b/compiler/rustc_borrowck/src/type_check/mod.rs index cf2140097e6..f7d35da0259 100644 --- a/compiler/rustc_borrowck/src/type_check/mod.rs +++ b/compiler/rustc_borrowck/src/type_check/mod.rs @@ -1847,14 +1847,11 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { let tcx = self.tcx(); let def_id = uv.def.def_id_for_type_of(); if tcx.def_kind(def_id) == DefKind::InlineConst { - let predicates = self.prove_closure_bounds( - tcx, - def_id.expect_local(), - uv.substs, - location, - ); + let def_id = def_id.expect_local(); + let predicates = + self.prove_closure_bounds(tcx, def_id, uv.substs, location); self.normalize_and_prove_instantiated_predicates( - def_id, + def_id.to_def_id(), predicates, location.to_locations(), ); @@ -2514,9 +2511,9 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { aggregate_kind, location ); - let (def_id, instantiated_predicates) = match aggregate_kind { + let (def_id, instantiated_predicates) = match *aggregate_kind { AggregateKind::Adt(adt_did, _, substs, _, _) => { - (*adt_did, tcx.predicates_of(*adt_did).instantiate(tcx, substs)) + (adt_did, tcx.predicates_of(adt_did).instantiate(tcx, substs)) } // For closures, we have some **extra requirements** we @@ -2541,7 +2538,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { // clauses on the struct. AggregateKind::Closure(def_id, substs) | AggregateKind::Generator(def_id, substs, _) => { - (*def_id, self.prove_closure_bounds(tcx, def_id.expect_local(), substs, location)) + (def_id.to_def_id(), self.prove_closure_bounds(tcx, def_id, substs, location)) } AggregateKind::Array(_) | AggregateKind::Tuple => { diff --git a/compiler/rustc_const_eval/src/interpret/validity.rs b/compiler/rustc_const_eval/src/interpret/validity.rs index d20f16755c3..6f6717721fb 100644 --- a/compiler/rustc_const_eval/src/interpret/validity.rs +++ b/compiler/rustc_const_eval/src/interpret/validity.rs @@ -238,7 +238,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, ' if let Some(local_def_id) = def_id.as_local() { let tables = self.ecx.tcx.typeck(local_def_id); if let Some(captured_place) = - tables.closure_min_captures_flattened(*def_id).nth(field) + tables.closure_min_captures_flattened(local_def_id).nth(field) { // Sometimes the index is beyond the number of upvars (seen // for a generator). diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index f7311ebdabf..64e158ba348 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -1983,53 +1983,45 @@ impl<'tcx> Debug for Rvalue<'tcx> { } AggregateKind::Closure(def_id, substs) => ty::tls::with(|tcx| { - if let Some(def_id) = def_id.as_local() { - let name = if tcx.sess.opts.unstable_opts.span_free_formats { - let substs = tcx.lift(substs).unwrap(); - format!( - "[closure@{}]", - tcx.def_path_str_with_substs(def_id.to_def_id(), substs), - ) - } else { - let span = tcx.def_span(def_id); - format!( - "[closure@{}]", - tcx.sess.source_map().span_to_diagnostic_string(span) - ) - }; - let mut struct_fmt = fmt.debug_struct(&name); - - // FIXME(project-rfc-2229#48): This should be a list of capture names/places - if let Some(upvars) = tcx.upvars_mentioned(def_id) { - for (&var_id, place) in iter::zip(upvars.keys(), places) { - let var_name = tcx.hir().name(var_id); - struct_fmt.field(var_name.as_str(), place); - } - } - - struct_fmt.finish() + let name = if tcx.sess.opts.unstable_opts.span_free_formats { + let substs = tcx.lift(substs).unwrap(); + format!( + "[closure@{}]", + tcx.def_path_str_with_substs(def_id.to_def_id(), substs), + ) } else { - write!(fmt, "[closure]") + let span = tcx.def_span(def_id); + format!( + "[closure@{}]", + tcx.sess.source_map().span_to_diagnostic_string(span) + ) + }; + let mut struct_fmt = fmt.debug_struct(&name); + + // FIXME(project-rfc-2229#48): This should be a list of capture names/places + if let Some(upvars) = tcx.upvars_mentioned(def_id) { + for (&var_id, place) in iter::zip(upvars.keys(), places) { + let var_name = tcx.hir().name(var_id); + struct_fmt.field(var_name.as_str(), place); + } } + + struct_fmt.finish() }), AggregateKind::Generator(def_id, _, _) => ty::tls::with(|tcx| { - if let Some(def_id) = def_id.as_local() { - let name = format!("[generator@{:?}]", tcx.def_span(def_id)); - let mut struct_fmt = fmt.debug_struct(&name); + let name = format!("[generator@{:?}]", tcx.def_span(def_id)); + let mut struct_fmt = fmt.debug_struct(&name); - // FIXME(project-rfc-2229#48): This should be a list of capture names/places - if let Some(upvars) = tcx.upvars_mentioned(def_id) { - for (&var_id, place) in iter::zip(upvars.keys(), places) { - let var_name = tcx.hir().name(var_id); - struct_fmt.field(var_name.as_str(), place); - } + // FIXME(project-rfc-2229#48): This should be a list of capture names/places + if let Some(upvars) = tcx.upvars_mentioned(def_id) { + for (&var_id, place) in iter::zip(upvars.keys(), places) { + let var_name = tcx.hir().name(var_id); + struct_fmt.field(var_name.as_str(), place); } - - struct_fmt.finish() - } else { - write!(fmt, "[generator]") } + + struct_fmt.finish() }), } } diff --git a/compiler/rustc_middle/src/mir/syntax.rs b/compiler/rustc_middle/src/mir/syntax.rs index 510316c778b..8e3c2283efc 100644 --- a/compiler/rustc_middle/src/mir/syntax.rs +++ b/compiler/rustc_middle/src/mir/syntax.rs @@ -18,6 +18,7 @@ use rustc_hir::{self, GeneratorKind}; use rustc_target::abi::VariantIdx; use rustc_ast::Mutability; +use rustc_span::def_id::LocalDefId; use rustc_span::symbol::Symbol; use rustc_span::Span; use rustc_target::asm::InlineAsmRegOrRegClass; @@ -340,8 +341,11 @@ pub enum FakeReadCause { /// If a closure pattern matches a Place starting with an Upvar, then we introduce a /// FakeRead for that Place outside the closure, in such a case this option would be /// Some(closure_def_id). - /// Otherwise, the value of the optional DefId will be None. - ForMatchedPlace(Option), + /// Otherwise, the value of the optional LocalDefId will be None. + // + // We can use LocaDefId here since fake read statements are removed + // before codegen in the `CleanupNonCodegenStatements` pass. + ForMatchedPlace(Option), /// A fake read of the RefWithinGuard version of a bind-by-value variable /// in a match guard to ensure that its value hasn't change by the time @@ -365,7 +369,7 @@ pub enum FakeReadCause { /// FakeRead for that Place outside the closure, in such a case this option would be /// Some(closure_def_id). /// Otherwise, the value of the optional DefId will be None. - ForLet(Option), + ForLet(Option), /// If we have an index expression like /// @@ -1095,8 +1099,10 @@ pub enum AggregateKind<'tcx> { /// active field index would identity the field `c` Adt(DefId, VariantIdx, SubstsRef<'tcx>, Option, Option), - Closure(DefId, SubstsRef<'tcx>), - Generator(DefId, SubstsRef<'tcx>, hir::Movability), + // Note: We can use LocalDefId since closures and generators a deaggregated + // before codegen. + Closure(LocalDefId, SubstsRef<'tcx>), + Generator(LocalDefId, SubstsRef<'tcx>, hir::Movability), } #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] diff --git a/compiler/rustc_middle/src/mir/tcx.rs b/compiler/rustc_middle/src/mir/tcx.rs index fd3359ea80f..405003156c4 100644 --- a/compiler/rustc_middle/src/mir/tcx.rs +++ b/compiler/rustc_middle/src/mir/tcx.rs @@ -205,9 +205,9 @@ impl<'tcx> Rvalue<'tcx> { AggregateKind::Adt(did, _, substs, _, _) => { tcx.bound_type_of(did).subst(tcx, substs) } - AggregateKind::Closure(did, substs) => tcx.mk_closure(did, substs), + AggregateKind::Closure(did, substs) => tcx.mk_closure(did.to_def_id(), substs), AggregateKind::Generator(did, substs, movability) => { - tcx.mk_generator(did, substs, movability) + tcx.mk_generator(did.to_def_id(), substs, movability) } }, Rvalue::ShallowInitBox(_, ty) => tcx.mk_box(ty), diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 466a0fc25f7..ffa66b79dbf 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -413,12 +413,12 @@ rustc_queries! { } query symbols_for_closure_captures( - key: (LocalDefId, DefId) + key: (LocalDefId, LocalDefId) ) -> Vec { storage(ArenaCacheSelector<'tcx>) desc { |tcx| "symbols for captures of closure `{}` in `{}`", - tcx.def_path_str(key.1), + tcx.def_path_str(key.1.to_def_id()), tcx.def_path_str(key.0.to_def_id()) } } diff --git a/compiler/rustc_middle/src/thir.rs b/compiler/rustc_middle/src/thir.rs index 36db8d04918..8b27ca57046 100644 --- a/compiler/rustc_middle/src/thir.rs +++ b/compiler/rustc_middle/src/thir.rs @@ -27,6 +27,7 @@ use rustc_span::{Span, Symbol, DUMMY_SP}; use rustc_target::abi::VariantIdx; use rustc_target::asm::InlineAsmRegOrRegClass; +use rustc_span::def_id::LocalDefId; use std::fmt; use std::ops::Index; @@ -405,7 +406,7 @@ pub enum ExprKind<'tcx> { }, /// A closure definition. Closure { - closure_id: DefId, + closure_id: LocalDefId, substs: UpvarSubsts<'tcx>, upvars: Box<[ExprId]>, movability: Option, diff --git a/compiler/rustc_middle/src/ty/closure.rs b/compiler/rustc_middle/src/ty/closure.rs index 27b9d27b8bb..0d6c26a5822 100644 --- a/compiler/rustc_middle/src/ty/closure.rs +++ b/compiler/rustc_middle/src/ty/closure.rs @@ -59,7 +59,7 @@ pub type UpvarCaptureMap = FxHashMap; /// Given the closure DefId this map provides a map of root variables to minimum /// set of `CapturedPlace`s that need to be tracked to support all captures of that closure. -pub type MinCaptureInformationMap<'tcx> = FxHashMap>; +pub type MinCaptureInformationMap<'tcx> = FxHashMap>; /// Part of `MinCaptureInformationMap`; Maps a root variable to the list of `CapturedPlace`. /// Used to track the minimum set of `Place`s that need to be captured to support all @@ -253,7 +253,7 @@ impl<'tcx> CapturedPlace<'tcx> { fn symbols_for_closure_captures<'tcx>( tcx: TyCtxt<'tcx>, - def_id: (LocalDefId, DefId), + def_id: (LocalDefId, LocalDefId), ) -> Vec { let typeck_results = tcx.typeck(def_id.0); let captures = typeck_results.closure_min_captures_flattened(def_id.1); diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index 0f98d19820e..2714493b9fc 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -570,7 +570,7 @@ pub struct TypeckResults<'tcx> { /// we never capture `t`. This becomes an issue when we build MIR as we require /// information on `t` in order to create place `t.0` and `t.1`. We can solve this /// issue by fake reading `t`. - pub closure_fake_reads: FxHashMap, FakeReadCause, hir::HirId)>>, + pub closure_fake_reads: FxHashMap, FakeReadCause, hir::HirId)>>, /// Tracks the rvalue scoping rules which defines finer scoping for rvalue expressions /// by applying extended parameter rules. @@ -589,7 +589,7 @@ pub struct TypeckResults<'tcx> { /// Contains the data for evaluating the effect of feature `capture_disjoint_fields` /// on closure size. - pub closure_size_eval: FxHashMap>, + pub closure_size_eval: FxHashMap>, } impl<'tcx> TypeckResults<'tcx> { @@ -811,7 +811,7 @@ impl<'tcx> TypeckResults<'tcx> { /// by the closure. pub fn closure_min_captures_flattened( &self, - closure_def_id: DefId, + closure_def_id: LocalDefId, ) -> impl Iterator> { self.closure_min_captures .get(&closure_def_id) diff --git a/compiler/rustc_mir_build/src/build/expr/as_place.rs b/compiler/rustc_mir_build/src/build/expr/as_place.rs index e88f9dc1f08..0c06aad4e44 100644 --- a/compiler/rustc_mir_build/src/build/expr/as_place.rs +++ b/compiler/rustc_mir_build/src/build/expr/as_place.rs @@ -3,7 +3,7 @@ use crate::build::expr::category::Category; use crate::build::ForGuard::{OutsideGuard, RefWithinGuard}; use crate::build::{BlockAnd, BlockAndExtension, Builder}; -use rustc_hir::def_id::{DefId, LocalDefId}; +use rustc_hir::def_id::LocalDefId; use rustc_middle::hir::place::Projection as HirProjection; use rustc_middle::hir::place::ProjectionKind as HirProjectionKind; use rustc_middle::middle::region; @@ -58,7 +58,7 @@ pub(crate) enum PlaceBase { /// HirId of the upvar var_hir_id: LocalVarId, /// DefId of the closure - closure_def_id: DefId, + closure_def_id: LocalDefId, /// The trait closure implements, `Fn`, `FnMut`, `FnOnce` closure_kind: ty::ClosureKind, }, @@ -176,7 +176,7 @@ fn compute_capture_idx<'tcx>( fn find_capture_matching_projections<'a, 'tcx>( typeck_results: &'a ty::TypeckResults<'tcx>, var_hir_id: LocalVarId, - closure_def_id: DefId, + closure_def_id: LocalDefId, projections: &[PlaceElem<'tcx>], ) -> Option<(usize, &'a ty::CapturedPlace<'tcx>)> { let closure_min_captures = typeck_results.closure_min_captures.get(&closure_def_id)?; @@ -242,7 +242,7 @@ fn to_upvars_resolved_place_builder<'a, 'tcx>( }; // We won't be building MIR if the closure wasn't local - let closure_hir_id = tcx.hir().local_def_id_to_hir_id(closure_def_id.expect_local()); + let closure_hir_id = tcx.hir().local_def_id_to_hir_id(closure_def_id); let closure_ty = typeck_results.node_type(closure_hir_id); let substs = match closure_ty.kind() { @@ -626,11 +626,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { fn lower_captured_upvar( &mut self, block: BasicBlock, - closure_expr_id: LocalDefId, + closure_def_id: LocalDefId, var_hir_id: LocalVarId, ) -> BlockAnd> { let closure_ty = - self.typeck_results.node_type(self.tcx.hir().local_def_id_to_hir_id(closure_expr_id)); + self.typeck_results.node_type(self.tcx.hir().local_def_id_to_hir_id(closure_def_id)); let closure_kind = if let ty::Closure(_, closure_substs) = closure_ty.kind() { self.infcx.closure_kind(closure_substs).unwrap() @@ -639,11 +639,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ty::ClosureKind::FnOnce }; - block.and(PlaceBuilder::from(PlaceBase::Upvar { - var_hir_id, - closure_def_id: closure_expr_id.to_def_id(), - closure_kind, - })) + block.and(PlaceBuilder::from(PlaceBase::Upvar { var_hir_id, closure_def_id, closure_kind })) } /// Lower an index expression diff --git a/compiler/rustc_mir_build/src/build/mod.rs b/compiler/rustc_mir_build/src/build/mod.rs index 7ae26cccb38..12b8ceede0f 100644 --- a/compiler/rustc_mir_build/src/build/mod.rs +++ b/compiler/rustc_mir_build/src/build/mod.rs @@ -672,7 +672,7 @@ where Some(builder.in_scope(arg_scope_s, LintLevel::Inherited, |builder| { builder.args_and_body( START_BLOCK, - fn_def.did.to_def_id(), + fn_def.did, &arguments, arg_scope, &thir[expr], @@ -895,7 +895,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { fn args_and_body( &mut self, mut block: BasicBlock, - fn_def_id: DefId, + fn_def_id: LocalDefId, arguments: &[ArgInfo<'tcx>], argument_scope: region::Scope, expr: &Expr<'tcx>, diff --git a/compiler/rustc_mir_build/src/check_unsafety.rs b/compiler/rustc_mir_build/src/check_unsafety.rs index 1f0d0ce04aa..951247dd3b6 100644 --- a/compiler/rustc_mir_build/src/check_unsafety.rs +++ b/compiler/rustc_mir_build/src/check_unsafety.rs @@ -408,7 +408,6 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> { movability: _, fake_reads: _, } => { - let closure_id = closure_id.expect_local(); let closure_def = if let Some((did, const_param_id)) = ty::WithOptConstParam::try_lookup(closure_id, self.tcx) { diff --git a/compiler/rustc_mir_build/src/thir/cx/expr.rs b/compiler/rustc_mir_build/src/thir/cx/expr.rs index 4eb3607e9cc..985601712c4 100644 --- a/compiler/rustc_mir_build/src/thir/cx/expr.rs +++ b/compiler/rustc_mir_build/src/thir/cx/expr.rs @@ -523,6 +523,7 @@ impl<'tcx> Cx<'tcx> { span_bug!(expr.span, "closure expr w/o closure type: {:?}", closure_ty); } }; + let def_id = def_id.expect_local(); let upvars = self .typeck_results diff --git a/compiler/rustc_mir_transform/src/check_unsafety.rs b/compiler/rustc_mir_transform/src/check_unsafety.rs index a2ad96cfc16..ede44628919 100644 --- a/compiler/rustc_mir_transform/src/check_unsafety.rs +++ b/compiler/rustc_mir_transform/src/check_unsafety.rs @@ -129,7 +129,7 @@ impl<'tcx> Visitor<'tcx> for UnsafetyChecker<'_, 'tcx> { } &AggregateKind::Closure(def_id, _) | &AggregateKind::Generator(def_id, _, _) => { let UnsafetyCheckResult { violations, used_unsafe_blocks, .. } = - self.tcx.unsafety_check_result(def_id.expect_local()); + self.tcx.unsafety_check_result(def_id); self.register_violations( violations, used_unsafe_blocks.iter().map(|(&h, &d)| (h, d)), diff --git a/compiler/rustc_monomorphize/src/util.rs b/compiler/rustc_monomorphize/src/util.rs index d3aaad46015..847e64dc2a2 100644 --- a/compiler/rustc_monomorphize/src/util.rs +++ b/compiler/rustc_monomorphize/src/util.rs @@ -17,8 +17,8 @@ pub(crate) fn dump_closure_profile<'tcx>(tcx: TyCtxt<'tcx>, closure_instance: In return; }; - let closure_def_id = closure_instance.def_id(); - let typeck_results = tcx.typeck(closure_def_id.expect_local()); + let closure_def_id = closure_instance.def_id().expect_local(); + let typeck_results = tcx.typeck(closure_def_id); if typeck_results.closure_size_eval.contains_key(&closure_def_id) { let param_env = ty::ParamEnv::reveal_all(); diff --git a/compiler/rustc_passes/src/liveness.rs b/compiler/rustc_passes/src/liveness.rs index eed3e1579a2..461dd52b9f2 100644 --- a/compiler/rustc_passes/src/liveness.rs +++ b/compiler/rustc_passes/src/liveness.rs @@ -519,7 +519,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { fn new(ir: &'a mut IrMaps<'tcx>, body_owner: LocalDefId) -> Liveness<'a, 'tcx> { let typeck_results = ir.tcx.typeck(body_owner); let param_env = ir.tcx.param_env(body_owner); - let closure_min_captures = typeck_results.closure_min_captures.get(&body_owner.to_def_id()); + let closure_min_captures = typeck_results.closure_min_captures.get(&body_owner); let closure_ln = ir.add_live_node(ClosureNode); let exit_ln = ir.add_live_node(ExitNode); diff --git a/compiler/rustc_query_impl/src/keys.rs b/compiler/rustc_query_impl/src/keys.rs index 54774314313..49175e97f41 100644 --- a/compiler/rustc_query_impl/src/keys.rs +++ b/compiler/rustc_query_impl/src/keys.rs @@ -191,6 +191,16 @@ impl Key for (LocalDefId, DefId) { } } +impl Key for (LocalDefId, LocalDefId) { + #[inline(always)] + fn query_crate_is_local(&self) -> bool { + true + } + fn default_span(&self, tcx: TyCtxt<'_>) -> Span { + self.0.default_span(tcx) + } +} + impl Key for (DefId, Option) { #[inline(always)] fn query_crate_is_local(&self) -> bool { diff --git a/compiler/rustc_typeck/src/check/callee.rs b/compiler/rustc_typeck/src/check/callee.rs index 0836f15a122..75f5aced855 100644 --- a/compiler/rustc_typeck/src/check/callee.rs +++ b/compiler/rustc_typeck/src/check/callee.rs @@ -5,7 +5,7 @@ use crate::type_error_struct; use rustc_errors::{struct_span_err, Applicability, Diagnostic}; use rustc_hir as hir; use rustc_hir::def::{self, Namespace, Res}; -use rustc_hir::def_id::{DefId, LOCAL_CRATE}; +use rustc_hir::def_id::DefId; use rustc_infer::{ infer, traits::{self, Obligation}, @@ -19,11 +19,13 @@ use rustc_middle::ty::adjustment::{ }; use rustc_middle::ty::subst::{Subst, SubstsRef}; use rustc_middle::ty::{self, Ty, TyCtxt, TypeVisitable}; +use rustc_span::def_id::LocalDefId; use rustc_span::symbol::{sym, Ident}; use rustc_span::Span; use rustc_target::spec::abi; use rustc_trait_selection::autoderef::Autoderef; use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt; + use std::iter; /// Checks that it is legal to call methods of the trait corresponding @@ -59,7 +61,7 @@ pub fn check_legal_trait_for_method_call( enum CallStep<'tcx> { Builtin(Ty<'tcx>), - DeferredClosure(DefId, ty::FnSig<'tcx>), + DeferredClosure(LocalDefId, ty::FnSig<'tcx>), /// E.g., enum variant constructors. Overloaded(MethodCallee<'tcx>), } @@ -145,7 +147,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } ty::Closure(def_id, substs) => { - assert_eq!(def_id.krate, LOCAL_CRATE); + let def_id = def_id.expect_local(); // Check whether this is a call to a closure where we // haven't yet decided on whether the closure is fn vs @@ -558,7 +560,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { call_expr: &'tcx hir::Expr<'tcx>, arg_exprs: &'tcx [hir::Expr<'tcx>], expected: Expectation<'tcx>, - closure_def_id: DefId, + closure_def_id: LocalDefId, fn_sig: ty::FnSig<'tcx>, ) -> Ty<'tcx> { // `fn_sig` is the *signature* of the closure being called. We @@ -581,7 +583,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { arg_exprs, fn_sig.c_variadic, TupleArgumentsFlag::TupleArguments, - Some(closure_def_id), + Some(closure_def_id.to_def_id()), ); fn_sig.output() diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs b/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs index d1c10a3b63c..22087219667 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs @@ -29,6 +29,7 @@ use rustc_middle::ty::{ ToPredicate, Ty, UserType, }; use rustc_session::lint; +use rustc_span::def_id::LocalDefId; use rustc_span::hygiene::DesugaringKind; use rustc_span::symbol::{kw, sym, Ident}; use rustc_span::{Span, DUMMY_SP}; @@ -114,7 +115,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { pub(in super::super) fn record_deferred_call_resolution( &self, - closure_def_id: DefId, + closure_def_id: LocalDefId, r: DeferredCallResolution<'tcx>, ) { let mut deferred_call_resolutions = self.deferred_call_resolutions.borrow_mut(); @@ -123,7 +124,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { pub(in super::super) fn remove_deferred_call_resolutions( &self, - closure_def_id: DefId, + closure_def_id: LocalDefId, ) -> Vec> { let mut deferred_call_resolutions = self.deferred_call_resolutions.borrow_mut(); deferred_call_resolutions.remove(&closure_def_id).unwrap_or_default() diff --git a/compiler/rustc_typeck/src/check/inherited.rs b/compiler/rustc_typeck/src/check/inherited.rs index a499179b95f..6312b8d49f4 100644 --- a/compiler/rustc_typeck/src/check/inherited.rs +++ b/compiler/rustc_typeck/src/check/inherited.rs @@ -2,13 +2,14 @@ use super::callee::DeferredCallResolution; use rustc_data_structures::fx::FxHashSet; use rustc_hir as hir; -use rustc_hir::def_id::{DefIdMap, LocalDefId}; +use rustc_hir::def_id::LocalDefId; use rustc_hir::HirIdMap; use rustc_infer::infer; use rustc_infer::infer::{InferCtxt, InferOk, TyCtxtInferExt}; use rustc_middle::ty::fold::TypeFoldable; use rustc_middle::ty::visit::TypeVisitable; use rustc_middle::ty::{self, Ty, TyCtxt}; +use rustc_span::def_id::LocalDefIdMap; use rustc_span::{self, Span}; use rustc_trait_selection::infer::InferCtxtExt as _; use rustc_trait_selection::traits::{self, ObligationCause, TraitEngine, TraitEngineExt}; @@ -46,7 +47,7 @@ pub struct Inherited<'a, 'tcx> { // decision. We keep these deferred resolutions grouped by the // def-id of the closure, so that once we decide, we can easily go // back and process them. - pub(super) deferred_call_resolutions: RefCell>>>, + pub(super) deferred_call_resolutions: RefCell>>>, pub(super) deferred_cast_checks: RefCell>>, diff --git a/compiler/rustc_typeck/src/check/upvar.rs b/compiler/rustc_typeck/src/check/upvar.rs index d72e215934a..dd8f943b985 100644 --- a/compiler/rustc_typeck/src/check/upvar.rs +++ b/compiler/rustc_typeck/src/check/upvar.rs @@ -35,7 +35,6 @@ use super::FnCtxt; use crate::expr_use_visitor as euv; use rustc_errors::{Applicability, MultiSpan}; use rustc_hir as hir; -use rustc_hir::def_id::DefId; use rustc_hir::def_id::LocalDefId; use rustc_hir::intravisit::{self, Visitor}; use rustc_infer::infer::UpvarRegion; @@ -186,6 +185,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ); } }; + let closure_def_id = closure_def_id.expect_local(); let infer_kind = if let UpvarSubsts::Closure(closure_substs) = substs { self.closure_kind(closure_substs).is_none().then_some(closure_substs) @@ -193,20 +193,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { None }; - let local_def_id = closure_def_id.expect_local(); - - let body_owner_def_id = self.tcx.hir().body_owner_def_id(body.id()); - assert_eq!(body_owner_def_id.to_def_id(), closure_def_id); + assert_eq!(self.tcx.hir().body_owner_def_id(body.id()), closure_def_id); let mut delegate = InferBorrowKind { fcx: self, - closure_def_id: local_def_id, + closure_def_id, capture_information: Default::default(), fake_reads: Default::default(), }; euv::ExprUseVisitor::new( &mut delegate, &self.infcx, - body_owner_def_id, + closure_def_id, self.param_env, &self.typeck_results.borrow(), ) @@ -224,7 +221,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.compute_min_captures(closure_def_id, capture_information, span); - let closure_hir_id = self.tcx.hir().local_def_id_to_hir_id(local_def_id); + let closure_hir_id = self.tcx.hir().local_def_id_to_hir_id(closure_def_id); if should_do_rust_2021_incompatible_closure_captures_analysis(self.tcx, closure_hir_id) { self.perform_2229_migration_anaysis(closure_def_id, body_id, capture_clause, span); @@ -239,7 +236,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { if let Some(upvars) = self.tcx.upvars_mentioned(closure_def_id) { for var_hir_id in upvars.keys() { - let place = self.place_for_root_variable(local_def_id, *var_hir_id); + let place = self.place_for_root_variable(closure_def_id, *var_hir_id); debug!("seed place {:?}", place); @@ -333,7 +330,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } // Returns a list of `Ty`s for each upvar. - fn final_upvar_tys(&self, closure_id: DefId) -> Vec> { + fn final_upvar_tys(&self, closure_id: LocalDefId) -> Vec> { self.typeck_results .borrow() .closure_min_captures_flattened(closure_id) @@ -511,7 +508,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { /// ``` fn compute_min_captures( &self, - closure_def_id: DefId, + closure_def_id: LocalDefId, capture_information: InferredCaptureInformation<'tcx>, closure_span: Span, ) { @@ -730,7 +727,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { /// `disjoint_capture_drop_reorder` if needed. fn perform_2229_migration_anaysis( &self, - closure_def_id: DefId, + closure_def_id: LocalDefId, body_id: hir::BodyId, capture_clause: hir::CaptureBy, span: Span, @@ -746,8 +743,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let (migration_string, migrated_variables_concat) = migration_suggestion_for_2229(self.tcx, &need_migrations); - let closure_hir_id = - self.tcx.hir().local_def_id_to_hir_id(closure_def_id.expect_local()); + let closure_hir_id = self.tcx.hir().local_def_id_to_hir_id(closure_def_id); let closure_head_span = self.tcx.def_span(closure_def_id); self.tcx.struct_span_lint_hir( lint::builtin::RUST_2021_INCOMPATIBLE_CLOSURE_CAPTURES, @@ -1058,7 +1054,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { #[instrument(level = "debug", skip(self))] fn compute_2229_migrations_for_drop( &self, - closure_def_id: DefId, + closure_def_id: LocalDefId, closure_span: Span, min_captures: Option<&ty::RootVariableMinCaptureList<'tcx>>, closure_clause: hir::CaptureBy, @@ -1066,7 +1062,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ) -> Option> { let ty = self.resolve_vars_if_possible(self.node_ty(var_hir_id)); - if !ty.has_significant_drop(self.tcx, self.tcx.param_env(closure_def_id.expect_local())) { + if !ty.has_significant_drop(self.tcx, self.tcx.param_env(closure_def_id)) { debug!("does not have significant drop"); return None; } @@ -1160,7 +1156,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { #[instrument(level = "debug", skip(self))] fn compute_2229_migrations( &self, - closure_def_id: DefId, + closure_def_id: LocalDefId, closure_span: Span, closure_clause: hir::CaptureBy, min_captures: Option<&ty::RootVariableMinCaptureList<'tcx>>, @@ -1343,14 +1339,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { /// implements Drop which will be affected since `y` isn't completely captured. fn has_significant_drop_outside_of_captures( &self, - closure_def_id: DefId, + closure_def_id: LocalDefId, closure_span: Span, base_path_ty: Ty<'tcx>, captured_by_move_projs: Vec<&[Projection<'tcx>]>, ) -> bool { - let needs_drop = |ty: Ty<'tcx>| { - ty.has_significant_drop(self.tcx, self.tcx.param_env(closure_def_id.expect_local())) - }; + let needs_drop = + |ty: Ty<'tcx>| ty.has_significant_drop(self.tcx, self.tcx.param_env(closure_def_id)); let is_drop_defined_for_ty = |ty: Ty<'tcx>| { let drop_trait = self.tcx.require_lang_item(hir::LangItem::Drop, Some(closure_span)); @@ -1360,7 +1355,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { drop_trait, ty, ty_params, - self.tcx.param_env(closure_def_id.expect_local()), + self.tcx.param_env(closure_def_id), ) .must_apply_modulo_regions() }; @@ -1518,13 +1513,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } - fn should_log_capture_analysis(&self, closure_def_id: DefId) -> bool { - self.tcx.has_attr(closure_def_id, sym::rustc_capture_analysis) + fn should_log_capture_analysis(&self, closure_def_id: LocalDefId) -> bool { + self.tcx.has_attr(closure_def_id.to_def_id(), sym::rustc_capture_analysis) } fn log_capture_analysis_first_pass( &self, - closure_def_id: rustc_hir::def_id::DefId, + closure_def_id: LocalDefId, capture_information: &InferredCaptureInformation<'tcx>, closure_span: Span, ) { @@ -1543,7 +1538,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } - fn log_closure_min_capture_info(&self, closure_def_id: DefId, closure_span: Span) { + fn log_closure_min_capture_info(&self, closure_def_id: LocalDefId, closure_span: Span) { if self.should_log_capture_analysis(closure_def_id) { if let Some(min_captures) = self.typeck_results.borrow().closure_min_captures.get(&closure_def_id) diff --git a/compiler/rustc_typeck/src/check/writeback.rs b/compiler/rustc_typeck/src/check/writeback.rs index fa6053ac395..f549807c39c 100644 --- a/compiler/rustc_typeck/src/check/writeback.rs +++ b/compiler/rustc_typeck/src/check/writeback.rs @@ -8,7 +8,6 @@ use hir::def_id::LocalDefId; use rustc_data_structures::fx::FxHashMap; use rustc_errors::ErrorGuaranteed; use rustc_hir as hir; -use rustc_hir::def_id::DefId; use rustc_hir::intravisit::{self, Visitor}; use rustc_infer::infer::error_reporting::TypeAnnotationNeeded::E0282; use rustc_infer::infer::InferCtxt; @@ -348,14 +347,13 @@ impl<'cx, 'tcx> Visitor<'tcx> for WritebackCx<'cx, 'tcx> { impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> { fn eval_closure_size(&mut self) { - let mut res: FxHashMap> = Default::default(); - for (closure_def_id, data) in self.fcx.typeck_results.borrow().closure_size_eval.iter() { - let closure_hir_id = - self.tcx().hir().local_def_id_to_hir_id(closure_def_id.expect_local()); + let mut res: FxHashMap> = Default::default(); + for (&closure_def_id, data) in self.fcx.typeck_results.borrow().closure_size_eval.iter() { + let closure_hir_id = self.tcx().hir().local_def_id_to_hir_id(closure_def_id); let data = self.resolve(*data, &closure_hir_id); - res.insert(*closure_def_id, data); + res.insert(closure_def_id, data); } self.typeck_results.closure_size_eval = res; @@ -365,7 +363,7 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> { self.fcx.typeck_results.borrow().closure_min_captures.len(), Default::default(), ); - for (closure_def_id, root_min_captures) in + for (&closure_def_id, root_min_captures) in self.fcx.typeck_results.borrow().closure_min_captures.iter() { let mut root_var_map_wb = ty::RootVariableMinCaptureList::with_capacity_and_hasher( @@ -377,7 +375,7 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> { .iter() .map(|captured_place| { let locatable = captured_place.info.path_expr_id.unwrap_or_else(|| { - self.tcx().hir().local_def_id_to_hir_id(closure_def_id.expect_local()) + self.tcx().hir().local_def_id_to_hir_id(closure_def_id) }); self.resolve(captured_place.clone(), &locatable) @@ -385,7 +383,7 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> { .collect(); root_var_map_wb.insert(*var_hir_id, min_list_wb); } - min_captures_wb.insert(*closure_def_id, root_var_map_wb); + min_captures_wb.insert(closure_def_id, root_var_map_wb); } self.typeck_results.closure_min_captures = min_captures_wb; @@ -393,21 +391,20 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> { fn visit_fake_reads_map(&mut self) { let mut resolved_closure_fake_reads: FxHashMap< - DefId, + LocalDefId, Vec<(HirPlace<'tcx>, FakeReadCause, hir::HirId)>, > = Default::default(); - for (closure_def_id, fake_reads) in + for (&closure_def_id, fake_reads) in self.fcx.typeck_results.borrow().closure_fake_reads.iter() { let mut resolved_fake_reads = Vec::<(HirPlace<'tcx>, FakeReadCause, hir::HirId)>::new(); for (place, cause, hir_id) in fake_reads.iter() { - let locatable = - self.tcx().hir().local_def_id_to_hir_id(closure_def_id.expect_local()); + let locatable = self.tcx().hir().local_def_id_to_hir_id(closure_def_id); let resolved_fake_read = self.resolve(place.clone(), &locatable); resolved_fake_reads.push((resolved_fake_read, *cause, *hir_id)); } - resolved_closure_fake_reads.insert(*closure_def_id, resolved_fake_reads); + resolved_closure_fake_reads.insert(closure_def_id, resolved_fake_reads); } self.typeck_results.closure_fake_reads = resolved_closure_fake_reads; } diff --git a/compiler/rustc_typeck/src/expr_use_visitor.rs b/compiler/rustc_typeck/src/expr_use_visitor.rs index 9d7420acd26..74a5b6e42c3 100644 --- a/compiler/rustc_typeck/src/expr_use_visitor.rs +++ b/compiler/rustc_typeck/src/expr_use_visitor.rs @@ -468,7 +468,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { self.borrow_expr(discr, ty::ImmBorrow); } else { let closure_def_id = match discr_place.place.base { - PlaceBase::Upvar(upvar_id) => Some(upvar_id.closure_expr_id.to_def_id()), + PlaceBase::Upvar(upvar_id) => Some(upvar_id.closure_expr_id), _ => None, }; @@ -642,7 +642,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { fn walk_arm(&mut self, discr_place: &PlaceWithHirId<'tcx>, arm: &hir::Arm<'_>) { let closure_def_id = match discr_place.place.base { - PlaceBase::Upvar(upvar_id) => Some(upvar_id.closure_expr_id.to_def_id()), + PlaceBase::Upvar(upvar_id) => Some(upvar_id.closure_expr_id), _ => None, }; @@ -666,7 +666,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { /// let binding, and *not* a match arm or nested pat.) fn walk_irrefutable_pat(&mut self, discr_place: &PlaceWithHirId<'tcx>, pat: &hir::Pat<'_>) { let closure_def_id = match discr_place.place.base { - PlaceBase::Upvar(upvar_id) => Some(upvar_id.closure_expr_id.to_def_id()), + PlaceBase::Upvar(upvar_id) => Some(upvar_id.closure_expr_id), _ => None, }; @@ -763,7 +763,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { debug!("walk_captures({:?})", closure_expr); let tcx = self.tcx(); - let closure_def_id = tcx.hir().local_def_id(closure_expr.hir_id).to_def_id(); + let closure_def_id = tcx.hir().local_def_id(closure_expr.hir_id); let upvars = tcx.upvars_mentioned(self.body_owner); // For purposes of this function, generator and closures are equivalent. diff --git a/src/tools/clippy/clippy_utils/src/lib.rs b/src/tools/clippy/clippy_utils/src/lib.rs index 34a1cdaf1d5..018059facbd 100644 --- a/src/tools/clippy/clippy_utils/src/lib.rs +++ b/src/tools/clippy/clippy_utils/src/lib.rs @@ -968,7 +968,7 @@ pub fn can_move_expr_to_closure<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<' } }, ExprKind::Closure { .. } => { - let closure_id = self.cx.tcx.hir().local_def_id(e.hir_id).to_def_id(); + let closure_id = self.cx.tcx.hir().local_def_id(e.hir_id); for capture in self.cx.typeck_results().closure_min_captures_flattened(closure_id) { let local_id = match capture.place.base { PlaceBase::Local(id) => id, From 03622552ecdbac78072ea5e4aa4d7389d606c25c Mon Sep 17 00:00:00 2001 From: Takayuki Maeda Date: Sun, 31 Jul 2022 11:33:01 +0900 Subject: [PATCH 7/8] use appropriate `HirID` for finding `else_span` --- .../rustc_infer/src/infer/error_reporting/mod.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs index 39faed0bf36..20864c657ff 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -712,7 +712,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { opt_suggest_box_span, }) => { let then_span = self.find_block_span_from_hir_id(then_id); - let else_span = self.find_block_span_from_hir_id(then_id); + let else_span = self.find_block_span_from_hir_id(else_id); err.span_label(then_span, "expected because of this"); if let Some(sp) = outer_span { err.span_label(sp, "`if` and `else` have incompatible types"); @@ -760,11 +760,15 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { second_ty: Ty<'tcx>, second_span: Span, ) { - let remove_semicolon = - [(first_id, second_ty), (second_id, first_ty)].into_iter().find_map(|(id, ty)| { - let hir::Node::Block(blk) = self.tcx.hir().get(id?) else { return None }; - self.could_remove_semicolon(blk, ty) - }); + let remove_semicolon = [ + (first_id, self.resolve_vars_if_possible(second_ty)), + (second_id, self.resolve_vars_if_possible(first_ty)), + ] + .into_iter() + .find_map(|(id, ty)| { + let hir::Node::Block(blk) = self.tcx.hir().get(id?) else { return None }; + self.could_remove_semicolon(blk, ty) + }); match remove_semicolon { Some((sp, StatementAsExpression::NeedsBoxing)) => { err.multipart_suggestion( From f6908be329d2be65b4de0f36393b98757c13ecad Mon Sep 17 00:00:00 2001 From: Takayuki Maeda Date: Sun, 31 Jul 2022 11:36:04 +0900 Subject: [PATCH 8/8] add a test to check if `suggest_remove_semi_or_return_binding` is working well for if-else --- .../ui/suggestions/if-then-neeing-semi.rs | 70 ++++++++++ .../ui/suggestions/if-then-neeing-semi.stderr | 130 ++++++++++++++++++ 2 files changed, 200 insertions(+) create mode 100644 src/test/ui/suggestions/if-then-neeing-semi.rs create mode 100644 src/test/ui/suggestions/if-then-neeing-semi.stderr diff --git a/src/test/ui/suggestions/if-then-neeing-semi.rs b/src/test/ui/suggestions/if-then-neeing-semi.rs new file mode 100644 index 00000000000..b487f013d27 --- /dev/null +++ b/src/test/ui/suggestions/if-then-neeing-semi.rs @@ -0,0 +1,70 @@ +// edition:2018 + +fn dummy() -> i32 { + 42 +} + +fn extra_semicolon() { + let _ = if true { + //~^ NOTE `if` and `else` have incompatible types + dummy(); //~ NOTE expected because of this + //~^ HELP consider removing this semicolon + } else { + dummy() //~ ERROR `if` and `else` have incompatible types + //~^ NOTE expected `()`, found `i32` + }; +} + +async fn async_dummy() {} //~ NOTE checked the `Output` of this `async fn`, found opaque type +//~| NOTE while checking the return type of the `async fn` +//~| NOTE in this expansion of desugaring of `async` block or function +//~| NOTE checked the `Output` of this `async fn`, expected opaque type +//~| NOTE while checking the return type of the `async fn` +//~| NOTE in this expansion of desugaring of `async` block or function +async fn async_dummy2() {} //~ NOTE checked the `Output` of this `async fn`, found opaque type +//~| NOTE checked the `Output` of this `async fn`, found opaque type +//~| NOTE while checking the return type of the `async fn` +//~| NOTE in this expansion of desugaring of `async` block or function +//~| NOTE while checking the return type of the `async fn` +//~| NOTE in this expansion of desugaring of `async` block or function + +async fn async_extra_semicolon_same() { + let _ = if true { + //~^ NOTE `if` and `else` have incompatible types + async_dummy(); //~ NOTE expected because of this + //~^ HELP consider removing this semicolon + } else { + async_dummy() //~ ERROR `if` and `else` have incompatible types + //~^ NOTE expected `()`, found opaque type + //~| NOTE expected unit type `()` + //~| HELP consider `await`ing on the `Future` + }; +} + +async fn async_extra_semicolon_different() { + let _ = if true { + //~^ NOTE `if` and `else` have incompatible types + async_dummy(); //~ NOTE expected because of this + //~^ HELP consider removing this semicolon + } else { + async_dummy2() //~ ERROR `if` and `else` have incompatible types + //~^ NOTE expected `()`, found opaque type + //~| NOTE expected unit type `()` + //~| HELP consider `await`ing on the `Future` + }; +} + +async fn async_different_futures() { + let _ = if true { + //~^ NOTE `if` and `else` have incompatible types + async_dummy() //~ NOTE expected because of this + //~| HELP consider `await`ing on both `Future`s + } else { + async_dummy2() //~ ERROR `if` and `else` have incompatible types + //~^ NOTE expected opaque type, found a different opaque type + //~| NOTE expected opaque type `impl Future` + //~| NOTE distinct uses of `impl Trait` result in different opaque types + }; +} + +fn main() {} diff --git a/src/test/ui/suggestions/if-then-neeing-semi.stderr b/src/test/ui/suggestions/if-then-neeing-semi.stderr new file mode 100644 index 00000000000..d7c5818abbd --- /dev/null +++ b/src/test/ui/suggestions/if-then-neeing-semi.stderr @@ -0,0 +1,130 @@ +error[E0308]: `if` and `else` have incompatible types + --> $DIR/if-then-neeing-semi.rs:37:9 + | +LL | let _ = if true { + | _____________- +LL | | +LL | | async_dummy(); + | | -------------- expected because of this +LL | | +LL | | } else { +LL | | async_dummy() + | | ^^^^^^^^^^^^^ expected `()`, found opaque type +... | +LL | | +LL | | }; + | |_____- `if` and `else` have incompatible types + | +note: while checking the return type of the `async fn` + --> $DIR/if-then-neeing-semi.rs:18:24 + | +LL | async fn async_dummy() {} + | ^ checked the `Output` of this `async fn`, found opaque type + = note: expected unit type `()` + found opaque type `impl Future` +help: consider `await`ing on the `Future` + | +LL | async_dummy().await + | ++++++ +help: consider removing this semicolon + | +LL - async_dummy(); +LL + async_dummy() + | + +error[E0308]: `if` and `else` have incompatible types + --> $DIR/if-then-neeing-semi.rs:50:9 + | +LL | let _ = if true { + | _____________- +LL | | +LL | | async_dummy(); + | | -------------- expected because of this +LL | | +LL | | } else { +LL | | async_dummy2() + | | ^^^^^^^^^^^^^^ expected `()`, found opaque type +... | +LL | | +LL | | }; + | |_____- `if` and `else` have incompatible types + | +note: while checking the return type of the `async fn` + --> $DIR/if-then-neeing-semi.rs:24:25 + | +LL | async fn async_dummy2() {} + | ^ checked the `Output` of this `async fn`, found opaque type + = note: expected unit type `()` + found opaque type `impl Future` +help: consider `await`ing on the `Future` + | +LL | async_dummy2().await + | ++++++ +help: consider removing this semicolon and boxing the expressions + | +LL ~ Box::new(async_dummy()) +LL | +LL | } else { +LL ~ Box::new(async_dummy2()) + | + +error[E0308]: `if` and `else` have incompatible types + --> $DIR/if-then-neeing-semi.rs:63:9 + | +LL | let _ = if true { + | _____________- +LL | | +LL | | async_dummy() + | | ------------- expected because of this +LL | | +LL | | } else { +LL | | async_dummy2() + | | ^^^^^^^^^^^^^^ expected opaque type, found a different opaque type +... | +LL | | +LL | | }; + | |_____- `if` and `else` have incompatible types + | +note: while checking the return type of the `async fn` + --> $DIR/if-then-neeing-semi.rs:18:24 + | +LL | async fn async_dummy() {} + | ^ checked the `Output` of this `async fn`, expected opaque type +note: while checking the return type of the `async fn` + --> $DIR/if-then-neeing-semi.rs:24:25 + | +LL | async fn async_dummy2() {} + | ^ checked the `Output` of this `async fn`, found opaque type + = note: expected opaque type `impl Future` (opaque type at <$DIR/if-then-neeing-semi.rs:18:24>) + found opaque type `impl Future` (opaque type at <$DIR/if-then-neeing-semi.rs:24:25>) + = note: distinct uses of `impl Trait` result in different opaque types +help: consider `await`ing on both `Future`s + | +LL ~ async_dummy().await +LL | +LL | } else { +LL ~ async_dummy2().await + | + +error[E0308]: `if` and `else` have incompatible types + --> $DIR/if-then-neeing-semi.rs:13:9 + | +LL | let _ = if true { + | _____________- +LL | | +LL | | dummy(); + | | -------- + | | | | + | | | help: consider removing this semicolon + | | expected because of this +LL | | +LL | | } else { +LL | | dummy() + | | ^^^^^^^ expected `()`, found `i32` +LL | | +LL | | }; + | |_____- `if` and `else` have incompatible types + +error: aborting due to 4 previous errors + +For more information about this error, try `rustc --explain E0308`.