From a2b6734ea40dcd53e1bdad6d8dc3e31c0fec929a Mon Sep 17 00:00:00 2001 From: Sean McArthur Date: Wed, 13 Mar 2019 14:24:42 -0700 Subject: [PATCH 01/12] resolve: collect trait aliases along with traits --- src/librustc_resolve/lib.rs | 15 ++++++++++-- .../run-pass/traits/trait-alias-import.rs | 23 +++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) create mode 100644 src/test/run-pass/traits/trait-alias-import.rs diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index ac149be4b2a89..e50cda602c2bf 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1195,6 +1195,13 @@ impl<'a> ModuleData<'a> { } } + fn is_trait_alias(&self) -> bool { + match self.kind { + ModuleKind::Def(Def::TraitAlias(_), _) => true, + _ => false, + } + } + fn nearest_item_scope(&'a self) -> Module<'a> { if self.is_trait() { self.parent.unwrap() } else { self } } @@ -4369,8 +4376,10 @@ impl<'a> Resolver<'a> { let mut collected_traits = Vec::new(); module.for_each_child(|name, ns, binding| { if ns != TypeNS { return } - if let Def::Trait(_) = binding.def() { - collected_traits.push((name, binding)); + match binding.def() { + Def::Trait(_) | + Def::TraitAlias(_) => collected_traits.push((name, binding)), + _ => (), } }); *traits = Some(collected_traits.into_boxed_slice()); @@ -4834,6 +4843,7 @@ impl<'a> Resolver<'a> { let container = match parent.kind { ModuleKind::Def(Def::Mod(_), _) => "module", ModuleKind::Def(Def::Trait(_), _) => "trait", + ModuleKind::Def(Def::TraitAlias(_), _) => "trait alias", ModuleKind::Block(..) => "block", _ => "enum", }; @@ -4862,6 +4872,7 @@ impl<'a> Resolver<'a> { (TypeNS, _) if old_binding.is_extern_crate() => "extern crate", (TypeNS, Some(module)) if module.is_normal() => "module", (TypeNS, Some(module)) if module.is_trait() => "trait", + (TypeNS, Some(module)) if module.is_trait_alias() => "trait alias", (TypeNS, _) => "type", }; diff --git a/src/test/run-pass/traits/trait-alias-import.rs b/src/test/run-pass/traits/trait-alias-import.rs new file mode 100644 index 0000000000000..9def1f0d4804c --- /dev/null +++ b/src/test/run-pass/traits/trait-alias-import.rs @@ -0,0 +1,23 @@ +#![feature(trait_alias)] + +mod inner { + pub trait Foo { + fn foo(&self); + } + + pub struct Qux; + + impl Foo for Qux { + fn foo(&self) {} + } + + pub trait Bar = Foo; +} + +// Import only the alias, not the `Foo` trait. +use inner::{Bar, Qux}; + +fn main() { + let q = Qux; + q.foo(); +} From 8fe108796d9674bb828023539089b6e0636d9ea1 Mon Sep 17 00:00:00 2001 From: Christian Date: Fri, 29 Mar 2019 16:22:26 +0100 Subject: [PATCH 02/12] Added documentation on the remainder (Rem) operator for floating points. --- src/libcore/ops/arith.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libcore/ops/arith.rs b/src/libcore/ops/arith.rs index 0252edee23125..007a8db6c47e7 100644 --- a/src/libcore/ops/arith.rs +++ b/src/libcore/ops/arith.rs @@ -537,6 +537,10 @@ rem_impl_integer! { usize u8 u16 u32 u64 u128 isize i8 i16 i32 i64 i128 } macro_rules! rem_impl_float { ($($t:ty)*) => ($( + + /// The remainder from the division of two floats. + /// + /// The remainder has the same sign as the dividend. For example: `-5.0 % 2.0 = -1.0`. #[stable(feature = "rust1", since = "1.0.0")] impl Rem for $t { type Output = $t; From ea369cbc2f733568779215fd2aa68a54d5b75327 Mon Sep 17 00:00:00 2001 From: Christian Date: Sat, 30 Mar 2019 10:03:47 +0100 Subject: [PATCH 03/12] Added an example that shows how the remainder function on floating point values is computed internally. --- src/libcore/ops/arith.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/libcore/ops/arith.rs b/src/libcore/ops/arith.rs index 007a8db6c47e7..484461687e4eb 100644 --- a/src/libcore/ops/arith.rs +++ b/src/libcore/ops/arith.rs @@ -540,7 +540,17 @@ macro_rules! rem_impl_float { /// The remainder from the division of two floats. /// - /// The remainder has the same sign as the dividend. For example: `-5.0 % 2.0 = -1.0`. + /// The remainder has the same sign as the dividend and is computed as: + /// `x - (x / y).trunc() * y`. + /// + /// # Examples + /// ``` + /// let x: f32 = 4.0; + /// let y: f32 = 2.5; + /// let remainder = x - (x / y).trunc() * y; + /// + /// assert_eq!(x % y, remainder); + /// ``` #[stable(feature = "rust1", since = "1.0.0")] impl Rem for $t { type Output = $t; From 5bcc365a0f0842955457c5edc25f464925e51b66 Mon Sep 17 00:00:00 2001 From: O01eg Date: Sun, 31 Mar 2019 22:28:12 +0300 Subject: [PATCH 04/12] Fix custom relative libdir. Uses relative libdir to place libraries on all stages. Adds verbose installation output. --- src/bootstrap/builder.rs | 23 ++++++++++++++++++++++- src/bootstrap/compile.rs | 8 ++++---- src/bootstrap/dist.rs | 15 +++++++++------ src/bootstrap/lib.rs | 1 + 4 files changed, 36 insertions(+), 11 deletions(-) diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index 8d3c8fc435c8d..522466314d660 100644 --- a/src/bootstrap/builder.rs +++ b/src/bootstrap/builder.rs @@ -634,7 +634,28 @@ impl<'a> Builder<'a> { if compiler.is_snapshot(self) { self.rustc_snapshot_libdir() } else { - self.sysroot(compiler).join(libdir(&compiler.host)) + match self.config.libdir_relative() { + Some(relative_libdir) if compiler.stage >= 1 + => self.sysroot(compiler).join(relative_libdir), + _ => self.sysroot(compiler).join(libdir(&compiler.host)) + } + } + } + + /// Returns the compiler's relative libdir where it stores the dynamic libraries that + /// it itself links against. + /// + /// For example this returns `lib` on Unix and `bin` on + /// Windows. + pub fn libdir_relative(&self, compiler: Compiler) -> &Path { + if compiler.is_snapshot(self) { + libdir(&self.config.build).as_ref() + } else { + match self.config.libdir_relative() { + Some(relative_libdir) if compiler.stage >= 1 + => relative_libdir, + _ => libdir(&compiler.host).as_ref() + } } } diff --git a/src/bootstrap/compile.rs b/src/bootstrap/compile.rs index 237f5c0ea2f15..08316b71ea85b 100644 --- a/src/bootstrap/compile.rs +++ b/src/bootstrap/compile.rs @@ -20,7 +20,7 @@ use filetime::FileTime; use serde_json; use crate::dist; -use crate::util::{exe, libdir, is_dylib}; +use crate::util::{exe, is_dylib}; use crate::{Compiler, Mode, GitRepo}; use crate::native; @@ -1005,13 +1005,13 @@ impl Step for Assemble { // Link in all dylibs to the libdir let sysroot = builder.sysroot(target_compiler); - let sysroot_libdir = sysroot.join(libdir(&*host)); - t!(fs::create_dir_all(&sysroot_libdir)); + let rustc_libdir = builder.rustc_libdir(target_compiler); + t!(fs::create_dir_all(&rustc_libdir)); let src_libdir = builder.sysroot_libdir(build_compiler, host); for f in builder.read_dir(&src_libdir) { let filename = f.file_name().into_string().unwrap(); if is_dylib(&filename) { - builder.copy(&f.path(), &sysroot_libdir.join(&filename)); + builder.copy(&f.path(), &rustc_libdir.join(&filename)); } } diff --git a/src/bootstrap/dist.rs b/src/bootstrap/dist.rs index d982330a61640..a4d924d64ee78 100644 --- a/src/bootstrap/dist.rs +++ b/src/bootstrap/dist.rs @@ -18,7 +18,7 @@ use build_helper::output; use crate::{Compiler, Mode, LLVM_TOOLS}; use crate::channel; -use crate::util::{libdir, is_dylib, exe}; +use crate::util::{is_dylib, exe}; use crate::builder::{Builder, RunConfig, ShouldRun, Step}; use crate::compile; use crate::tool::{self, Tool}; @@ -473,7 +473,7 @@ impl Step for Rustc { fn prepare_image(builder: &Builder<'_>, compiler: Compiler, image: &Path) { let host = compiler.host; let src = builder.sysroot(compiler); - let libdir = libdir(&host); + let libdir = builder.rustc_libdir(compiler); // Copy rustc/rustdoc binaries t!(fs::create_dir_all(image.join("bin"))); @@ -481,13 +481,15 @@ impl Step for Rustc { builder.install(&builder.rustdoc(compiler), &image.join("bin"), 0o755); + let libdir_relative = builder.libdir_relative(compiler); + // Copy runtime DLLs needed by the compiler - if libdir != "bin" { - for entry in builder.read_dir(&src.join(libdir)) { + if libdir_relative.to_str() != Some("bin") { + for entry in builder.read_dir(&libdir) { let name = entry.file_name(); if let Some(s) = name.to_str() { if is_dylib(s) { - builder.install(&entry.path(), &image.join(libdir), 0o644); + builder.install(&entry.path(), &image.join(&libdir_relative), 0o644); } } } @@ -516,7 +518,8 @@ impl Step for Rustc { .join("bin") .join(&exe); // for the rationale about this rename check `compile::copy_lld_to_sysroot` - let dst = image.join("lib/rustlib") + let dst = image.join(libdir_relative) + .join("rustlib") .join(&*host) .join("bin") .join(&exe); diff --git a/src/bootstrap/lib.rs b/src/bootstrap/lib.rs index 2394ae7fb7913..47ac04baf6d6d 100644 --- a/src/bootstrap/lib.rs +++ b/src/bootstrap/lib.rs @@ -1275,6 +1275,7 @@ impl Build { fn install(&self, src: &Path, dstdir: &Path, perms: u32) { if self.config.dry_run { return; } let dst = dstdir.join(src.file_name().unwrap()); + self.verbose_than(1, &format!("Install {:?} to {:?}", src, dst)); t!(fs::create_dir_all(dstdir)); drop(fs::remove_file(&dst)); { From 512069f8e51c975aac6bab662d9fccbf019d2a27 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Tue, 26 Mar 2019 15:30:41 -0400 Subject: [PATCH 05/12] Fix stack overflow when generating debuginfo for 'recursive' type By using 'impl trait', it's possible to create a self-referential type as follows: fn foo() -> impl Copy { foo } This is a function which returns itself. Normally, the signature of this function would be impossible to write - it would look like 'fn foo() -> fn() -> fn() ...' e.g. a function which returns a function, which returns a function... Using 'impl trait' allows us to avoid writing this infinitely long type. While it's useless for practical purposes, it does compile and run However, issues arise when we try to generate llvm debuginfo for such a type. All 'impl trait' types (e.g. ty::Opaque) are resolved when we generate debuginfo, which can lead to us recursing back to the original 'fn' type when we try to process its return type. To resolve this, I've modified debuginfo generation to account for these kinds of weird types. Unfortunately, there's no 'correct' debuginfo that we can generate - 'impl trait' does not exist in debuginfo, and this kind of recursive type is impossible to directly represent. To ensure that we emit *something*, this commit emits dummy debuginfo/type names whenever it encounters a self-reference. In practice, this should never happen - it's just to ensure that we can emit some kind of debuginfo, even if it's not particularly meaningful Fixes #58463 --- .../debuginfo/metadata.rs | 68 +++++++++++++++++-- .../debuginfo/type_names.rs | 49 +++++++++---- src/test/run-pass/issues/issue-58463.rs | 8 +++ 3 files changed, 108 insertions(+), 17 deletions(-) create mode 100644 src/test/run-pass/issues/issue-58463.rs diff --git a/src/librustc_codegen_llvm/debuginfo/metadata.rs b/src/librustc_codegen_llvm/debuginfo/metadata.rs index e549b120da979..3d742811ff049 100644 --- a/src/librustc_codegen_llvm/debuginfo/metadata.rs +++ b/src/librustc_codegen_llvm/debuginfo/metadata.rs @@ -117,6 +117,32 @@ impl TypeMap<'ll, 'tcx> { } } + // Removes a Ty to metadata mapping + // This is useful when computing the metadata for a potentially + // recursive type (e.g. a function ptr of the form: + // + // fn foo() -> impl Copy { foo } + // + // This kind of type cannot be properly represented + // via LLVM debuginfo. As a workaround, + // we register a temporary Ty to metadata mapping + // for the function before we compute its actual metadat.a + // If the metadata computation ends up recursing back to the + // original function, it will use the temporary mapping + // for the inner self-reference, preventing us from + // recursing forever. + // + // This function is used to remove the temporary metadata + // mapping after we've computed the actual metadat + fn remove_type( + &mut self, + type_: Ty<'tcx>, + ) { + if self.type_to_metadata.remove(type_).is_none() { + bug!("Type metadata Ty '{}' is not in the TypeMap!", type_); + } + } + // Adds a UniqueTypeId to metadata mapping to the TypeMap. The method will // fail if the mapping already exists. fn register_unique_id_with_metadata( @@ -608,10 +634,7 @@ pub fn type_metadata( } } ty::FnDef(..) | ty::FnPtr(_) => { - let fn_metadata = subroutine_type_metadata(cx, - unique_type_id, - t.fn_sig(cx.tcx), - usage_site_span).metadata; + if let Some(metadata) = debug_context(cx).type_map .borrow() .find_metadata_for_unique_id(unique_type_id) @@ -619,6 +642,43 @@ pub fn type_metadata( return metadata; } + // It's possible to create a self-referential + // type in Rust by using 'impl trait': + // + // fn foo() -> impl Copy { foo } + // + // See TypeMap::remove_type for more detals + // about the workaround + + let temp_type = { + unsafe { + // The choice of type here is pretty arbitrary - + // anything reading the debuginfo for a recursive + // type is going to see *somthing* weird - the only + // question is what exactly it will see + let (size, align) = cx.size_and_align_of(t); + llvm::LLVMRustDIBuilderCreateBasicType( + DIB(cx), + SmallCStr::new("").as_ptr(), + size.bits(), + align.bits() as u32, + DW_ATE_unsigned) + + + } + }; + + let type_map = &debug_context(cx).type_map; + type_map.borrow_mut().register_type_with_metadata(t, temp_type); + + let fn_metadata = subroutine_type_metadata(cx, + unique_type_id, + t.fn_sig(cx.tcx), + usage_site_span).metadata; + + type_map.borrow_mut().remove_type(t); + + // This is actually a function pointer, so wrap it in pointer DI MetadataCreationResult::new(pointer_type_metadata(cx, t, fn_metadata), false) diff --git a/src/librustc_codegen_llvm/debuginfo/type_names.rs b/src/librustc_codegen_llvm/debuginfo/type_names.rs index 8b218ab39d99b..024840a3fa8ad 100644 --- a/src/librustc_codegen_llvm/debuginfo/type_names.rs +++ b/src/librustc_codegen_llvm/debuginfo/type_names.rs @@ -5,6 +5,7 @@ use rustc::hir::def_id::DefId; use rustc::ty::subst::SubstsRef; use rustc::ty::{self, Ty}; use rustc_codegen_ssa::traits::*; +use rustc_data_structures::fx::FxHashSet; use rustc::hir; @@ -17,7 +18,8 @@ pub fn compute_debuginfo_type_name<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>, qualified: bool) -> String { let mut result = String::with_capacity(64); - push_debuginfo_type_name(cx, t, qualified, &mut result); + let mut visited = FxHashSet::default(); + push_debuginfo_type_name(cx, t, qualified, &mut result, &mut visited); result } @@ -26,7 +28,27 @@ pub fn compute_debuginfo_type_name<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>, pub fn push_debuginfo_type_name<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>, t: Ty<'tcx>, qualified: bool, - output: &mut String) { + output: &mut String, + visited: &mut FxHashSet>) { + + // We've encountered a weird 'recursive type' + // Currently, the only way to generate such a type + // is by using 'impl trait': + // + // fn foo() -> impl Copy { foo } + // + // There's not really a sensible name we can generate, + // since we don't include 'impl trait' types (e.g. ty::Opaque) + // in the output + // + // Since we need to generate *something*, we just + // use a dummy string that should make it clear + // that something unusual is going on + if visited.insert(t) { + output.push_str(""); + return; + } + // When targeting MSVC, emit C++ style type names for compatibility with // .natvis visualizers (and perhaps other existing native debuggers?) let cpp_like_names = cx.sess().target.target.options.is_like_msvc; @@ -42,12 +64,12 @@ pub fn push_debuginfo_type_name<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>, ty::Foreign(def_id) => push_item_name(cx, def_id, qualified, output), ty::Adt(def, substs) => { push_item_name(cx, def.did, qualified, output); - push_type_params(cx, substs, output); + push_type_params(cx, substs, output, visited); }, ty::Tuple(component_types) => { output.push('('); for &component_type in component_types { - push_debuginfo_type_name(cx, component_type, true, output); + push_debuginfo_type_name(cx, component_type, true, output, visited); output.push_str(", "); } if !component_types.is_empty() { @@ -65,7 +87,7 @@ pub fn push_debuginfo_type_name<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>, hir::MutMutable => output.push_str("mut "), } - push_debuginfo_type_name(cx, inner_type, true, output); + push_debuginfo_type_name(cx, inner_type, true, output, visited); if cpp_like_names { output.push('*'); @@ -79,7 +101,7 @@ pub fn push_debuginfo_type_name<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>, output.push_str("mut "); } - push_debuginfo_type_name(cx, inner_type, true, output); + push_debuginfo_type_name(cx, inner_type, true, output, visited); if cpp_like_names { output.push('*'); @@ -87,7 +109,7 @@ pub fn push_debuginfo_type_name<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>, }, ty::Array(inner_type, len) => { output.push('['); - push_debuginfo_type_name(cx, inner_type, true, output); + push_debuginfo_type_name(cx, inner_type, true, output, visited); output.push_str(&format!("; {}", len.unwrap_usize(cx.tcx))); output.push(']'); }, @@ -98,7 +120,7 @@ pub fn push_debuginfo_type_name<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>, output.push('['); } - push_debuginfo_type_name(cx, inner_type, true, output); + push_debuginfo_type_name(cx, inner_type, true, output, visited); if cpp_like_names { output.push('>'); @@ -113,7 +135,7 @@ pub fn push_debuginfo_type_name<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>, &principal, ); push_item_name(cx, principal.def_id, false, output); - push_type_params(cx, principal.substs, output); + push_type_params(cx, principal.substs, output, visited); } else { output.push_str("dyn '_"); } @@ -136,7 +158,7 @@ pub fn push_debuginfo_type_name<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>, let sig = cx.tcx.normalize_erasing_late_bound_regions(ty::ParamEnv::reveal_all(), &sig); if !sig.inputs().is_empty() { for ¶meter_type in sig.inputs() { - push_debuginfo_type_name(cx, parameter_type, true, output); + push_debuginfo_type_name(cx, parameter_type, true, output, visited); output.push_str(", "); } output.pop(); @@ -155,7 +177,7 @@ pub fn push_debuginfo_type_name<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>, if !sig.output().is_unit() { output.push_str(" -> "); - push_debuginfo_type_name(cx, sig.output(), true, output); + push_debuginfo_type_name(cx, sig.output(), true, output, visited); } }, ty::Closure(..) => { @@ -200,7 +222,8 @@ pub fn push_debuginfo_type_name<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>, // common denominator - otherwise we would run into conflicts. fn push_type_params<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>, substs: SubstsRef<'tcx>, - output: &mut String) { + output: &mut String, + visited: &mut FxHashSet>) { if substs.types().next().is_none() { return; } @@ -208,7 +231,7 @@ pub fn push_debuginfo_type_name<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>, output.push('<'); for type_parameter in substs.types() { - push_debuginfo_type_name(cx, type_parameter, true, output); + push_debuginfo_type_name(cx, type_parameter, true, output, visited); output.push_str(", "); } diff --git a/src/test/run-pass/issues/issue-58463.rs b/src/test/run-pass/issues/issue-58463.rs new file mode 100644 index 0000000000000..8ab845366b7b4 --- /dev/null +++ b/src/test/run-pass/issues/issue-58463.rs @@ -0,0 +1,8 @@ +// run-pass +// compile-flags:-C debuginfo=2 +fn foo() -> impl Copy { + foo +} +fn main() { + foo(); +} From e1837a0d1aa9fa9484bdc7f85c785c9fba6d7300 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Tue, 26 Mar 2019 19:49:14 -0400 Subject: [PATCH 06/12] Fix inverted panic check --- src/librustc_codegen_llvm/debuginfo/type_names.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_codegen_llvm/debuginfo/type_names.rs b/src/librustc_codegen_llvm/debuginfo/type_names.rs index 024840a3fa8ad..5854610256110 100644 --- a/src/librustc_codegen_llvm/debuginfo/type_names.rs +++ b/src/librustc_codegen_llvm/debuginfo/type_names.rs @@ -44,7 +44,7 @@ pub fn push_debuginfo_type_name<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>, // Since we need to generate *something*, we just // use a dummy string that should make it clear // that something unusual is going on - if visited.insert(t) { + if !visited.insert(t) { output.push_str(""); return; } From aed7ec42b3615d0b469029a73a8638fcd57281cd Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Tue, 26 Mar 2019 22:38:55 -0400 Subject: [PATCH 07/12] Add codegen test --- src/test/codegen/fn-impl-trait-self.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 src/test/codegen/fn-impl-trait-self.rs diff --git a/src/test/codegen/fn-impl-trait-self.rs b/src/test/codegen/fn-impl-trait-self.rs new file mode 100644 index 0000000000000..f9113d50197c1 --- /dev/null +++ b/src/test/codegen/fn-impl-trait-self.rs @@ -0,0 +1,15 @@ +// compile-flags: -g +// +// CHECK-LABEL: @main +// CHECK: {{.*}}DIDerivedType(tag: DW_TAG_pointer_type, name: "fn() -> ",{{.*}} +// +// CHECK: {{.*}}DISubroutineType{{.*}} +// CHECK: {{.*}}DIBasicType(name: "", encoding: DW_ATE_unsigned) + +pub fn foo() -> impl Copy { + foo +} + +fn main() { + let my_res = foo(); +} From c4556a5a656b70a8be726847f540c2184dec4ef4 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Thu, 28 Mar 2019 12:22:08 -0400 Subject: [PATCH 08/12] Fix typos --- src/librustc_codegen_llvm/debuginfo/metadata.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/librustc_codegen_llvm/debuginfo/metadata.rs b/src/librustc_codegen_llvm/debuginfo/metadata.rs index 3d742811ff049..94d520ec78c71 100644 --- a/src/librustc_codegen_llvm/debuginfo/metadata.rs +++ b/src/librustc_codegen_llvm/debuginfo/metadata.rs @@ -126,14 +126,14 @@ impl TypeMap<'ll, 'tcx> { // This kind of type cannot be properly represented // via LLVM debuginfo. As a workaround, // we register a temporary Ty to metadata mapping - // for the function before we compute its actual metadat.a + // for the function before we compute its actual metadata. // If the metadata computation ends up recursing back to the // original function, it will use the temporary mapping // for the inner self-reference, preventing us from // recursing forever. // // This function is used to remove the temporary metadata - // mapping after we've computed the actual metadat + // mapping after we've computed the actual metadata fn remove_type( &mut self, type_: Ty<'tcx>, @@ -663,8 +663,6 @@ pub fn type_metadata( size.bits(), align.bits() as u32, DW_ATE_unsigned) - - } }; From d2584e3b6aa8ed60f291d71a86f849a7f667c2fd Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sun, 31 Mar 2019 22:53:27 -0400 Subject: [PATCH 09/12] Only track 'visited' state for function types --- .../debuginfo/type_names.rs | 49 ++++++++++++------- 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/src/librustc_codegen_llvm/debuginfo/type_names.rs b/src/librustc_codegen_llvm/debuginfo/type_names.rs index 5854610256110..05c6c9877acd7 100644 --- a/src/librustc_codegen_llvm/debuginfo/type_names.rs +++ b/src/librustc_codegen_llvm/debuginfo/type_names.rs @@ -31,24 +31,6 @@ pub fn push_debuginfo_type_name<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>, output: &mut String, visited: &mut FxHashSet>) { - // We've encountered a weird 'recursive type' - // Currently, the only way to generate such a type - // is by using 'impl trait': - // - // fn foo() -> impl Copy { foo } - // - // There's not really a sensible name we can generate, - // since we don't include 'impl trait' types (e.g. ty::Opaque) - // in the output - // - // Since we need to generate *something*, we just - // use a dummy string that should make it clear - // that something unusual is going on - if !visited.insert(t) { - output.push_str(""); - return; - } - // When targeting MSVC, emit C++ style type names for compatibility with // .natvis visualizers (and perhaps other existing native debuggers?) let cpp_like_names = cx.sess().target.target.options.is_like_msvc; @@ -141,6 +123,25 @@ pub fn push_debuginfo_type_name<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>, } }, ty::FnDef(..) | ty::FnPtr(_) => { + // We've encountered a weird 'recursive type' + // Currently, the only way to generate such a type + // is by using 'impl trait': + // + // fn foo() -> impl Copy { foo } + // + // There's not really a sensible name we can generate, + // since we don't include 'impl trait' types (e.g. ty::Opaque) + // in the output + // + // Since we need to generate *something*, we just + // use a dummy string that should make it clear + // that something unusual is going on + if !visited.insert(t) { + output.push_str(""); + return; + } + + let sig = t.fn_sig(cx.tcx); if sig.unsafety() == hir::Unsafety::Unsafe { output.push_str("unsafe "); @@ -179,6 +180,18 @@ pub fn push_debuginfo_type_name<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>, output.push_str(" -> "); push_debuginfo_type_name(cx, sig.output(), true, output, visited); } + + + // We only keep the type in 'visited' + // for the duration of the body of this method. + // It's fine for a particular function type + // to show up multiple times in one overall type + // (e.g. MyType u8, fn() -> u8> + // + // We only care about avoiding recursing + // directly back to the type we're currentlu + // processing + visited.remove(t); }, ty::Closure(..) => { output.push_str("closure"); From c13daeb036273d3fd0544c3cc58592f2a86d1631 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Mon, 1 Apr 2019 13:41:41 -0400 Subject: [PATCH 10/12] Fix typo --- src/librustc_codegen_llvm/debuginfo/type_names.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_codegen_llvm/debuginfo/type_names.rs b/src/librustc_codegen_llvm/debuginfo/type_names.rs index 05c6c9877acd7..eff7cd1bc8a48 100644 --- a/src/librustc_codegen_llvm/debuginfo/type_names.rs +++ b/src/librustc_codegen_llvm/debuginfo/type_names.rs @@ -189,7 +189,7 @@ pub fn push_debuginfo_type_name<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>, // (e.g. MyType u8, fn() -> u8> // // We only care about avoiding recursing - // directly back to the type we're currentlu + // directly back to the type we're currently // processing visited.remove(t); }, From 3ccd35cd70eed139de411f9533d406c498ed4d62 Mon Sep 17 00:00:00 2001 From: Sean McArthur Date: Fri, 15 Mar 2019 14:55:04 -0700 Subject: [PATCH 11/12] resolve all in scope trait aliases, then elaborate their bounds --- src/librustc_resolve/lib.rs | 62 +++++++++++-------- src/librustc_resolve/resolve_imports.rs | 2 +- src/librustc_typeck/check/method/probe.rs | 46 +++++++++----- src/librustc_typeck/check/method/suggest.rs | 13 ++-- .../run-pass/traits/auxiliary/trait_alias.rs | 13 ++++ .../traits/trait-alias-import-cross-crate.rs | 14 +++++ .../run-pass/traits/trait-alias-import.rs | 17 ++++- src/test/ui/traits/trait-alias-ambiguous.rs | 24 +++++++ .../ui/traits/trait-alias-ambiguous.stderr | 20 ++++++ 9 files changed, 166 insertions(+), 45 deletions(-) create mode 100644 src/test/run-pass/traits/auxiliary/trait_alias.rs create mode 100644 src/test/run-pass/traits/trait-alias-import-cross-crate.rs create mode 100644 src/test/ui/traits/trait-alias-ambiguous.rs create mode 100644 src/test/ui/traits/trait-alias-ambiguous.stderr diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index e50cda602c2bf..fdfb21a605f7f 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1195,13 +1195,6 @@ impl<'a> ModuleData<'a> { } } - fn is_trait_alias(&self) -> bool { - match self.kind { - ModuleKind::Def(Def::TraitAlias(_), _) => true, - _ => false, - } - } - fn nearest_item_scope(&'a self) -> Module<'a> { if self.is_trait() { self.parent.unwrap() } else { self } } @@ -1359,7 +1352,7 @@ impl<'a> NameBinding<'a> { } } - // We sometimes need to treat variants as `pub` for backwards compatibility + // We sometimes need to treat variants as `pub` for backwards compatibility. fn pseudo_vis(&self) -> ty::Visibility { if self.is_variant() && self.def().def_id().is_local() { ty::Visibility::Public @@ -2717,7 +2710,7 @@ impl<'a> Resolver<'a> { { let mut self_type_rib = Rib::new(NormalRibKind); - // plain insert (no renaming, types are not currently hygienic....) + // Plain insert (no renaming, since types are not currently hygienic) self_type_rib.bindings.insert(keywords::SelfUpper.ident(), self_def); self.ribs[TypeNS].push(self_type_rib); f(self); @@ -4386,18 +4379,37 @@ impl<'a> Resolver<'a> { } for &(trait_name, binding) in traits.as_ref().unwrap().iter() { - let module = binding.module().unwrap(); - let mut ident = ident; - if ident.span.glob_adjust(module.expansion, binding.span.ctxt().modern()).is_none() { - continue - } - if self.resolve_ident_in_module_unadjusted( - ModuleOrUniformRoot::Module(module), - ident, - ns, - false, - module.span, - ).is_ok() { + // Traits have pseudo-modules that can be used to search for the given ident. + if let Some(module) = binding.module() { + let mut ident = ident; + if ident.span.glob_adjust( + module.expansion, + binding.span.ctxt().modern(), + ).is_none() { + continue + } + if self.resolve_ident_in_module_unadjusted( + ModuleOrUniformRoot::Module(module), + ident, + ns, + false, + module.span, + ).is_ok() { + let import_id = match binding.kind { + NameBindingKind::Import { directive, .. } => { + self.maybe_unused_trait_imports.insert(directive.id); + self.add_to_glob_map(&directive, trait_name); + Some(directive.id) + } + _ => None, + }; + let trait_def_id = module.def_id().unwrap(); + found_traits.push(TraitCandidate { def_id: trait_def_id, import_id }); + } + } else if let Def::TraitAlias(_) = binding.def() { + // For now, just treat all trait aliases as possible candidates, since we don't + // know if the ident is somewhere in the transitive bounds. + let import_id = match binding.kind { NameBindingKind::Import { directive, .. } => { self.maybe_unused_trait_imports.insert(directive.id); @@ -4406,8 +4418,10 @@ impl<'a> Resolver<'a> { } _ => None, }; - let trait_def_id = module.def_id().unwrap(); - found_traits.push(TraitCandidate { def_id: trait_def_id, import_id: import_id }); + let trait_def_id = binding.def().def_id(); + found_traits.push(TraitCandidate { def_id: trait_def_id, import_id }); + } else { + bug!("candidate is not trait or trait alias?") } } } @@ -4843,7 +4857,6 @@ impl<'a> Resolver<'a> { let container = match parent.kind { ModuleKind::Def(Def::Mod(_), _) => "module", ModuleKind::Def(Def::Trait(_), _) => "trait", - ModuleKind::Def(Def::TraitAlias(_), _) => "trait alias", ModuleKind::Block(..) => "block", _ => "enum", }; @@ -4872,7 +4885,6 @@ impl<'a> Resolver<'a> { (TypeNS, _) if old_binding.is_extern_crate() => "extern crate", (TypeNS, Some(module)) if module.is_normal() => "module", (TypeNS, Some(module)) if module.is_trait() => "trait", - (TypeNS, Some(module)) if module.is_trait_alias() => "trait alias", (TypeNS, _) => "type", }; diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 9daffd522bf2c..05597de2735ef 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -1245,7 +1245,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { self.populate_module_if_necessary(module); - if let Some(Def::Trait(_)) = module.def() { + if module.is_trait() { self.session.span_err(directive.span, "items in traits are not importable."); return; } else if module.def_id() == directive.parent_scope.module.def_id() { diff --git a/src/librustc_typeck/check/method/probe.rs b/src/librustc_typeck/check/method/probe.rs index 1f0ab3abb2836..48fa26e5882fe 100644 --- a/src/librustc_typeck/check/method/probe.rs +++ b/src/librustc_typeck/check/method/probe.rs @@ -895,20 +895,36 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { let trait_substs = self.fresh_item_substs(trait_def_id); let trait_ref = ty::TraitRef::new(trait_def_id, trait_substs); - for item in self.impl_or_trait_item(trait_def_id) { - // Check whether `trait_def_id` defines a method with suitable name: - if !self.has_applicable_self(&item) { - debug!("method has inapplicable self"); - self.record_static_candidate(TraitSource(trait_def_id)); - continue; - } + if self.tcx.is_trait_alias(trait_def_id) { + // For trait aliases, assume all super-traits are relevant. + let bounds = iter::once(trait_ref.to_poly_trait_ref()); + self.elaborate_bounds(bounds, |this, new_trait_ref, item| { + let new_trait_ref = this.erase_late_bound_regions(&new_trait_ref); + + let (xform_self_ty, xform_ret_ty) = + this.xform_self_ty(&item, new_trait_ref.self_ty(), new_trait_ref.substs); + this.push_candidate(Candidate { + xform_self_ty, xform_ret_ty, item, import_id, + kind: TraitCandidate(new_trait_ref), + }, true); + }); + } else { + debug_assert!(self.tcx.is_trait(trait_def_id)); + for item in self.impl_or_trait_item(trait_def_id) { + // Check whether `trait_def_id` defines a method with suitable name. + if !self.has_applicable_self(&item) { + debug!("method has inapplicable self"); + self.record_static_candidate(TraitSource(trait_def_id)); + continue; + } - let (xform_self_ty, xform_ret_ty) = - self.xform_self_ty(&item, trait_ref.self_ty(), trait_substs); - self.push_candidate(Candidate { - xform_self_ty, xform_ret_ty, item, import_id, - kind: TraitCandidate(trait_ref), - }, false); + let (xform_self_ty, xform_ret_ty) = + self.xform_self_ty(&item, trait_ref.self_ty(), trait_substs); + self.push_candidate(Candidate { + xform_self_ty, xform_ret_ty, item, import_id, + kind: TraitCandidate(trait_ref), + }, false); + } } Ok(()) } @@ -929,7 +945,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { .filter(|&name| set.insert(name)) .collect(); - // sort them by the name so we have a stable result + // Sort them by the name so we have a stable result. names.sort_by_cached_key(|n| n.as_str()); names } @@ -944,6 +960,8 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { return r; } + debug!("pick: actual search failed, assemble diagnotics"); + let static_candidates = mem::replace(&mut self.static_candidates, vec![]); let private_candidate = self.private_candidate.take(); let unsatisfied_predicates = mem::replace(&mut self.unsatisfied_predicates, vec![]); diff --git a/src/librustc_typeck/check/method/suggest.rs b/src/librustc_typeck/check/method/suggest.rs index f933e61b8c63e..b49614eedb15f 100644 --- a/src/librustc_typeck/check/method/suggest.rs +++ b/src/librustc_typeck/check/method/suggest.rs @@ -748,9 +748,13 @@ fn compute_all_traits<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>) -> Vec impl<'v, 'a, 'tcx> itemlikevisit::ItemLikeVisitor<'v> for Visitor<'a, 'tcx> { fn visit_item(&mut self, i: &'v hir::Item) { - if let hir::ItemKind::Trait(..) = i.node { - let def_id = self.map.local_def_id_from_hir_id(i.hir_id); - self.traits.push(def_id); + match i.node { + hir::ItemKind::Trait(..) | + hir::ItemKind::TraitAlias(..) => { + let def_id = self.map.local_def_id_from_hir_id(i.hir_id); + self.traits.push(def_id); + } + _ => () } } @@ -772,7 +776,8 @@ fn compute_all_traits<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>) -> Vec external_mods: &mut FxHashSet, def: Def) { match def { - Def::Trait(def_id) => { + Def::Trait(def_id) | + Def::TraitAlias(def_id) => { traits.push(def_id); } Def::Mod(def_id) => { diff --git a/src/test/run-pass/traits/auxiliary/trait_alias.rs b/src/test/run-pass/traits/auxiliary/trait_alias.rs new file mode 100644 index 0000000000000..9e412215512c9 --- /dev/null +++ b/src/test/run-pass/traits/auxiliary/trait_alias.rs @@ -0,0 +1,13 @@ +#![feature(trait_alias)] + +pub trait Hello { + fn hello(&self); +} + +pub struct Hi; + +impl Hello for Hi { + fn hello(&self) {} +} + +pub trait Greet = Hello; diff --git a/src/test/run-pass/traits/trait-alias-import-cross-crate.rs b/src/test/run-pass/traits/trait-alias-import-cross-crate.rs new file mode 100644 index 0000000000000..975542ab49b59 --- /dev/null +++ b/src/test/run-pass/traits/trait-alias-import-cross-crate.rs @@ -0,0 +1,14 @@ +// run-pass +// aux-build:trait_alias.rs + +#![feature(trait_alias)] + +extern crate trait_alias; + +// Import only the alias, not the real trait. +use trait_alias::{Greet, Hi}; + +fn main() { + let hi = Hi; + hi.hello(); // From `Hello`, via `Greet` alias. +} diff --git a/src/test/run-pass/traits/trait-alias-import.rs b/src/test/run-pass/traits/trait-alias-import.rs index 9def1f0d4804c..7d63320b9aad4 100644 --- a/src/test/run-pass/traits/trait-alias-import.rs +++ b/src/test/run-pass/traits/trait-alias-import.rs @@ -14,10 +14,25 @@ mod inner { pub trait Bar = Foo; } +mod two { + pub trait A { + fn foo(); + } + + impl A for u8 { + fn foo() {} + } +} + // Import only the alias, not the `Foo` trait. use inner::{Bar, Qux}; +// Declaring an alias also brings in aliased methods. +trait Two = two::A; + fn main() { let q = Qux; - q.foo(); + q.foo(); // From Bar. + + u8::foo(); // From A. } diff --git a/src/test/ui/traits/trait-alias-ambiguous.rs b/src/test/ui/traits/trait-alias-ambiguous.rs new file mode 100644 index 0000000000000..28409e0c66277 --- /dev/null +++ b/src/test/ui/traits/trait-alias-ambiguous.rs @@ -0,0 +1,24 @@ +#![feature(trait_alias)] + +mod inner { + pub trait A { fn foo(&self); } + pub trait B { fn foo(&self); } + + impl A for u8 { + fn foo(&self) {} + } + impl B for u8 { + fn foo(&self) {} + } + + pub trait C = A + B; +} + +use inner::C; + +fn main() { + let t = 1u8; + t.foo(); //~ ERROR E0034 + + inner::A::foo(&t); // ok +} diff --git a/src/test/ui/traits/trait-alias-ambiguous.stderr b/src/test/ui/traits/trait-alias-ambiguous.stderr new file mode 100644 index 0000000000000..b7443269b882d --- /dev/null +++ b/src/test/ui/traits/trait-alias-ambiguous.stderr @@ -0,0 +1,20 @@ +error[E0034]: multiple applicable items in scope + --> $DIR/trait-alias-ambiguous.rs:21:7 + | +LL | t.foo(); + | ^^^ multiple `foo` found + | +note: candidate #1 is defined in an impl of the trait `inner::A` for the type `u8` + --> $DIR/trait-alias-ambiguous.rs:8:9 + | +LL | fn foo(&self) {} + | ^^^^^^^^^^^^^ +note: candidate #2 is defined in an impl of the trait `inner::B` for the type `u8` + --> $DIR/trait-alias-ambiguous.rs:11:9 + | +LL | fn foo(&self) {} + | ^^^^^^^^^^^^^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0034`. From a1c79056e5df1236a82bdc6b315660f93ed5b11e Mon Sep 17 00:00:00 2001 From: Christian Date: Mon, 1 Apr 2019 22:49:14 +0200 Subject: [PATCH 12/12] Improved the example with numbers that can be exactly represented as floats and added a comment with the solution. --- src/libcore/ops/arith.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libcore/ops/arith.rs b/src/libcore/ops/arith.rs index 484461687e4eb..77c2dfa51bbbf 100644 --- a/src/libcore/ops/arith.rs +++ b/src/libcore/ops/arith.rs @@ -545,10 +545,11 @@ macro_rules! rem_impl_float { /// /// # Examples /// ``` - /// let x: f32 = 4.0; - /// let y: f32 = 2.5; + /// let x: f32 = 50.50; + /// let y: f32 = 8.125; /// let remainder = x - (x / y).trunc() * y; /// + /// // The answer to both operations is 1.75 /// assert_eq!(x % y, remainder); /// ``` #[stable(feature = "rust1", since = "1.0.0")]