From af7c540b2f403cc65a8ac0f376416a5883b7eac0 Mon Sep 17 00:00:00 2001 From: Berrysoft Date: Sun, 4 Apr 2021 22:50:01 +0800 Subject: [PATCH 1/9] Add empty __CxxFrameHandler3 impl in panic_abort --- library/panic_abort/src/lib.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/library/panic_abort/src/lib.rs b/library/panic_abort/src/lib.rs index eb2277d8baacd..0347df2d63717 100644 --- a/library/panic_abort/src/lib.rs +++ b/library/panic_abort/src/lib.rs @@ -116,6 +116,19 @@ pub mod personalities { )))] pub extern "C" fn rust_eh_personality() {} + // On *-pc-windows-msvc we need such a symbol to make linker happy. + #[allow(non_snake_case)] + #[no_mangle] + #[cfg(all(target_os = "windows", target_env = "msvc"))] + pub extern "C" fn __CxxFrameHandler3( + _record: usize, + _frame: usize, + _context: usize, + _dispatcher: usize, + ) -> u32 { + 1 + } + // On x86_64-pc-windows-gnu we use our own personality function that needs // to return `ExceptionContinueSearch` as we're passing on all our frames. #[rustc_std_internal_symbol] From 9cdaea332b407e96476410445329349f5fed1feb Mon Sep 17 00:00:00 2001 From: Berrysoft Date: Sun, 4 Apr 2021 22:50:39 +0800 Subject: [PATCH 2/9] Add tests for abort link to unwind. --- ...nk-to-unwind-msvc-no-std-link-to-libcmt.rs | 32 +++++++++++ ...nk-to-unwind-msvc-no-std-link-to-msvcrt.rs | 32 +++++++++++ .../abort-link-to-unwind-msvc-no-std.rs | 54 +++++++++++++++++++ .../exit-success-if-unwind-msvc-no-std.rs | 24 +++++++++ 4 files changed, 142 insertions(+) create mode 100644 src/test/ui/panic-runtime/abort-link-to-unwind-msvc-no-std-link-to-libcmt.rs create mode 100644 src/test/ui/panic-runtime/abort-link-to-unwind-msvc-no-std-link-to-msvcrt.rs create mode 100644 src/test/ui/panic-runtime/abort-link-to-unwind-msvc-no-std.rs create mode 100644 src/test/ui/panic-runtime/auxiliary/exit-success-if-unwind-msvc-no-std.rs diff --git a/src/test/ui/panic-runtime/abort-link-to-unwind-msvc-no-std-link-to-libcmt.rs b/src/test/ui/panic-runtime/abort-link-to-unwind-msvc-no-std-link-to-libcmt.rs new file mode 100644 index 0000000000000..34347ab80cce0 --- /dev/null +++ b/src/test/ui/panic-runtime/abort-link-to-unwind-msvc-no-std-link-to-libcmt.rs @@ -0,0 +1,32 @@ +// build-pass +// compile-flags: -C panic=abort -C target-feature=+crt-static +// aux-build:exit-success-if-unwind-msvc-no-std.rs +// only-msvc +// Test that `no_std` with `panic=abort` under MSVC toolchain +// doesn't cause error when linking to libcmt. +// We don't run this executable because it will hang in `rust_begin_unwind` + +#![no_std] +#![no_main] + +extern crate exit_success_if_unwind_msvc_no_std; + +use core::panic::PanicInfo; + +#[panic_handler] +fn handle_panic(_: &PanicInfo) -> ! { + loop {} +} + +#[link(name = "libcmt")] +extern "C" {} + +#[no_mangle] +pub extern "C" fn main() -> i32 { + exit_success_if_unwind_msvc_no_std::bar(do_panic); + 0 +} + +fn do_panic() { + panic!(); +} diff --git a/src/test/ui/panic-runtime/abort-link-to-unwind-msvc-no-std-link-to-msvcrt.rs b/src/test/ui/panic-runtime/abort-link-to-unwind-msvc-no-std-link-to-msvcrt.rs new file mode 100644 index 0000000000000..ef071b565fe3e --- /dev/null +++ b/src/test/ui/panic-runtime/abort-link-to-unwind-msvc-no-std-link-to-msvcrt.rs @@ -0,0 +1,32 @@ +// build-pass +// compile-flags: -C panic=abort +// aux-build:exit-success-if-unwind-msvc-no-std.rs +// only-msvc +// Test that `no_std` with `panic=abort` under MSVC toolchain +// doesn't cause error when linking to msvcrt. +// We don't run this executable because it will hang in `rust_begin_unwind` + +#![no_std] +#![no_main] + +extern crate exit_success_if_unwind_msvc_no_std; + +use core::panic::PanicInfo; + +#[panic_handler] +fn handle_panic(_: &PanicInfo) -> ! { + loop {} +} + +#[link(name = "msvcrt")] +extern "C" {} + +#[no_mangle] +pub extern "C" fn main() -> i32 { + exit_success_if_unwind_msvc_no_std::bar(do_panic); + 0 +} + +fn do_panic() { + panic!(); +} diff --git a/src/test/ui/panic-runtime/abort-link-to-unwind-msvc-no-std.rs b/src/test/ui/panic-runtime/abort-link-to-unwind-msvc-no-std.rs new file mode 100644 index 0000000000000..5c90285695db7 --- /dev/null +++ b/src/test/ui/panic-runtime/abort-link-to-unwind-msvc-no-std.rs @@ -0,0 +1,54 @@ +// build-pass +// compile-flags:-C panic=abort +// aux-build:exit-success-if-unwind-msvc-no-std.rs +// no-prefer-dynamic +// only-msvc +// We don't run this executable because it will hang in `rust_begin_unwind` + +#![no_std] +#![no_main] +#![windows_subsystem = "console"] +#![feature(panic_abort)] + +extern crate exit_success_if_unwind_msvc_no_std; +extern crate panic_abort; + +use core::panic::PanicInfo; + +#[panic_handler] +fn handle_panic(_: &PanicInfo) -> ! { + loop {} +} + +#[no_mangle] +pub unsafe extern "C" fn memcpy(dest: *mut u8, _src: *const u8, _n: usize) -> *mut u8 { + dest +} + +#[no_mangle] +pub unsafe extern "C" fn memmove(dest: *mut u8, _src: *const u8, _n: usize) -> *mut u8 { + dest +} + +#[no_mangle] +pub unsafe extern "C" fn memset(mem: *mut u8, _val: i32, _n: usize) -> *mut u8 { + mem +} + +#[no_mangle] +pub unsafe extern "C" fn memcmp(_mem1: *const u8, _mem2: *const u8, _n: usize) -> i32 { + 0 +} + +#[no_mangle] +#[used] +static _fltused: i32 = 0; + +#[no_mangle] +pub extern "C" fn mainCRTStartup() { + exit_success_if_unwind_msvc_no_std::bar(main); +} + +fn main() { + panic!(); +} diff --git a/src/test/ui/panic-runtime/auxiliary/exit-success-if-unwind-msvc-no-std.rs b/src/test/ui/panic-runtime/auxiliary/exit-success-if-unwind-msvc-no-std.rs new file mode 100644 index 0000000000000..7afe4f17e6d7f --- /dev/null +++ b/src/test/ui/panic-runtime/auxiliary/exit-success-if-unwind-msvc-no-std.rs @@ -0,0 +1,24 @@ +// compile-flags:-C panic=unwind +// no-prefer-dynamic + +#![no_std] +#![crate_type = "rlib"] + +struct Bomb; + +impl Drop for Bomb { + fn drop(&mut self) { + #[link(name = "kernel32")] + extern "C" { + fn ExitProcess(code: u32) -> !; + } + unsafe { + ExitProcess(0); + } + } +} + +pub fn bar(f: fn()) { + let _bomb = Bomb; + f(); +} From 48acdb719731ba98a1cc5635d26d08cbe8809e4b Mon Sep 17 00:00:00 2001 From: Berrysoft Date: Sun, 4 Apr 2021 22:52:34 +0800 Subject: [PATCH 3/9] Add tests for unwind no_std. --- .../unwind-msvc-no-std-link-to-libcmt.rs | 47 +++++++++++++++++++ .../unwind-msvc-no-std-link-to-msvcrt.rs | 47 +++++++++++++++++++ 2 files changed, 94 insertions(+) create mode 100644 src/test/ui/panic-runtime/unwind-msvc-no-std-link-to-libcmt.rs create mode 100644 src/test/ui/panic-runtime/unwind-msvc-no-std-link-to-msvcrt.rs diff --git a/src/test/ui/panic-runtime/unwind-msvc-no-std-link-to-libcmt.rs b/src/test/ui/panic-runtime/unwind-msvc-no-std-link-to-libcmt.rs new file mode 100644 index 0000000000000..b27b95ee050f9 --- /dev/null +++ b/src/test/ui/panic-runtime/unwind-msvc-no-std-link-to-libcmt.rs @@ -0,0 +1,47 @@ +// build-pass +// compile-flags: -C panic=unwind -C target-feature=+crt-static +// only-msvc +// Test that `no_std` with `panic=unwind` under MSVC toolchain +// doesn't cause error when linking to libcmt. + +#![no_std] +#![no_main] +#![feature(alloc_error_handler)] +#![feature(panic_unwind)] + +use core::alloc::{GlobalAlloc, Layout}; + +struct DummyAllocator; + +unsafe impl GlobalAlloc for DummyAllocator { + unsafe fn alloc(&self, _layout: Layout) -> *mut u8 { + core::ptr::null_mut() + } + + unsafe fn dealloc(&self, _ptr: *mut u8, _layout: Layout) {} +} + +#[global_allocator] +static ALLOC: DummyAllocator = DummyAllocator; + +#[alloc_error_handler] +fn rust_oom(_layout: Layout) -> ! { + panic!() +} + +extern crate panic_unwind; + +use core::panic::PanicInfo; + +#[panic_handler] +fn handle_panic(_: &PanicInfo) -> ! { + loop {} +} + +#[link(name = "libcmt")] +extern "C" {} + +#[no_mangle] +pub extern "C" fn main() -> i32 { + panic!(); +} diff --git a/src/test/ui/panic-runtime/unwind-msvc-no-std-link-to-msvcrt.rs b/src/test/ui/panic-runtime/unwind-msvc-no-std-link-to-msvcrt.rs new file mode 100644 index 0000000000000..95036e5774cee --- /dev/null +++ b/src/test/ui/panic-runtime/unwind-msvc-no-std-link-to-msvcrt.rs @@ -0,0 +1,47 @@ +// build-pass +// compile-flags: -C panic=unwind +// only-msvc +// Test that `no_std` with `panic=unwind` under MSVC toolchain +// doesn't cause error when linking to msvcrt. + +#![no_std] +#![no_main] +#![feature(alloc_error_handler)] +#![feature(panic_unwind)] + +use core::alloc::{GlobalAlloc, Layout}; + +struct DummyAllocator; + +unsafe impl GlobalAlloc for DummyAllocator { + unsafe fn alloc(&self, _layout: Layout) -> *mut u8 { + core::ptr::null_mut() + } + + unsafe fn dealloc(&self, _ptr: *mut u8, _layout: Layout) {} +} + +#[global_allocator] +static ALLOC: DummyAllocator = DummyAllocator; + +#[alloc_error_handler] +fn rust_oom(_layout: Layout) -> ! { + panic!() +} + +extern crate panic_unwind; + +use core::panic::PanicInfo; + +#[panic_handler] +fn handle_panic(_: &PanicInfo) -> ! { + loop {} +} + +#[link(name = "msvcrt")] +extern "C" {} + +#[no_mangle] +pub extern "C" fn main() -> i32 { + panic!(); +} From 6ce9a028a6a5a2e41851ed30b1a0c29e2e087cdc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lauren=C8=9Biu=20Nicola?= Date: Mon, 5 Apr 2021 14:40:58 +0300 Subject: [PATCH 4/9] :arrow_up: rust-analyzer --- src/tools/rust-analyzer | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/rust-analyzer b/src/tools/rust-analyzer index bb1d925dab363..19e09a4a54c75 160000 --- a/src/tools/rust-analyzer +++ b/src/tools/rust-analyzer @@ -1 +1 @@ -Subproject commit bb1d925dab36372c6bd1fb5671bb68ce938ff009 +Subproject commit 19e09a4a54c75312aeaac04577f2d0e067463ab6 From d63b3f9bbb66b6ec4c7eea42078fca23730761c1 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 4 Apr 2021 09:21:22 -0400 Subject: [PATCH 5/9] Remove duplicate unwrap_or_else --- .../passes/collect_intra_doc_links.rs | 90 ++++++++++--------- 1 file changed, 47 insertions(+), 43 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 437f42b26dd11..295f1087d63ae 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -377,7 +377,6 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { ns: Namespace, module_id: DefId, item_name: Symbol, - item_str: &'path str, ) -> Result<(Res, Option), ErrorKind<'path>> { let tcx = self.cx.tcx; @@ -399,7 +398,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { .map(|out| { ( Res::Primitive(prim_ty), - Some(format!("{}#{}.{}", prim_ty.as_str(), out, item_str)), + Some(format!("{}#{}.{}", prim_ty.as_str(), out, item_name)), ) }) }) @@ -413,7 +412,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { ResolutionFailure::NotResolved { module_id, partial_res: Some(Res::Primitive(prim_ty)), - unresolved: item_str.into(), + unresolved: item_name.to_string().into(), } .into() }) @@ -490,8 +489,6 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { module_id: DefId, extra_fragment: &Option, ) -> Result<(Res, Option), ErrorKind<'path>> { - let tcx = self.cx.tcx; - if let Some(res) = self.resolve_path(path_str, ns, module_id) { match res { // FIXME(#76467): make this fallthrough to lookup the associated @@ -534,29 +531,44 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } })?; - // FIXME: are these both necessary? - let ty_res = if let Some(ty_res) = resolve_primitive(&path_root, TypeNS) + // FIXME(#83862): this arbitrarily gives precedence to primitives over modules to support + // links to primitives when `#[doc(primitive)]` is present. It should give an ambiguity + // error instead and special case *only* modules with `#[doc(primitive)]`, not all + // primitives. + resolve_primitive(&path_root, TypeNS) .or_else(|| self.resolve_path(&path_root, TypeNS, module_id)) - { - ty_res - } else { - // FIXME: this is duplicated on the end of this function. - return if ns == Namespace::ValueNS { - self.variant_field(path_str, module_id) - } else { - Err(ResolutionFailure::NotResolved { - module_id, - partial_res: None, - unresolved: path_root.into(), + .and_then(|ty_res| { + self.resolve_associated_item(ty_res, item_name, ns, module_id, extra_fragment) + }) + .unwrap_or_else(|| { + if ns == Namespace::ValueNS { + self.variant_field(path_str, module_id) + } else { + Err(ResolutionFailure::NotResolved { + module_id, + partial_res: None, + unresolved: path_root.into(), + } + .into()) } - .into()) - }; - }; + }) + } + + fn resolve_associated_item( + &mut self, + root_res: Res, + item_name: Symbol, + ns: Namespace, + module_id: DefId, + extra_fragment: &Option, + // lol this is so bad + ) -> Option), ErrorKind<'static>>> { + let tcx = self.cx.tcx; - let res = match ty_res { - Res::Primitive(prim) => Some( - self.resolve_primitive_associated_item(prim, ns, module_id, item_name, item_str), - ), + match root_res { + Res::Primitive(prim) => { + Some(self.resolve_primitive_associated_item(prim, ns, module_id, item_name)) + } Res::Def( DefKind::Struct | DefKind::Union @@ -600,13 +612,15 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { ty::AssocKind::Type => "associatedtype", }; Some(if extra_fragment.is_some() { - Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict(ty_res))) + Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict( + root_res, + ))) } else { // HACK(jynelson): `clean` expects the type, not the associated item // but the disambiguator logic expects the associated item. // Store the kind in a side channel so that only the disambiguator logic looks at it. self.kind_side_channel.set(Some((kind.as_def_kind(), id))); - Ok((ty_res, Some(format!("{}.{}", out, item_str)))) + Ok((root_res, Some(format!("{}.{}", out, item_name)))) }) } else if ns == Namespace::ValueNS { debug!("looking for variants or fields named {} for {:?}", item_name, did); @@ -637,7 +651,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { )) } else { Ok(( - ty_res, + root_res, Some(format!( "{}.{}", if def.is_enum() { "variant" } else { "structfield" }, @@ -670,26 +684,16 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { }; if extra_fragment.is_some() { - Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict(ty_res))) + Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict( + root_res, + ))) } else { let res = Res::Def(item.kind.as_def_kind(), item.def_id); - Ok((res, Some(format!("{}.{}", kind, item_str)))) + Ok((res, Some(format!("{}.{}", kind, item_name)))) } }), _ => None, - }; - res.unwrap_or_else(|| { - if ns == Namespace::ValueNS { - self.variant_field(path_str, module_id) - } else { - Err(ResolutionFailure::NotResolved { - module_id, - partial_res: Some(ty_res), - unresolved: item_str.into(), - } - .into()) - } - }) + } } /// Used for reporting better errors. From ac04dbd056a94d59699be3983e5404856a9add13 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 4 Apr 2021 09:31:10 -0400 Subject: [PATCH 6/9] Reduce indentation in `resolve_associated_item` --- .../passes/collect_intra_doc_links.rs | 80 ++++++++----------- 1 file changed, 35 insertions(+), 45 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 295f1087d63ae..54d5f1cabfce7 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -611,7 +611,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { ty::AssocKind::Const => "associatedconstant", ty::AssocKind::Type => "associatedtype", }; - Some(if extra_fragment.is_some() { + return Some(if extra_fragment.is_some() { Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict( root_res, ))) @@ -621,51 +621,41 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { // Store the kind in a side channel so that only the disambiguator logic looks at it. self.kind_side_channel.set(Some((kind.as_def_kind(), id))); Ok((root_res, Some(format!("{}.{}", out, item_name)))) - }) - } else if ns == Namespace::ValueNS { - debug!("looking for variants or fields named {} for {:?}", item_name, did); - // FIXME(jynelson): why is this different from - // `variant_field`? - match tcx.type_of(did).kind() { - ty::Adt(def, _) => { - let field = if def.is_enum() { - def.all_fields().find(|item| item.ident.name == item_name) - } else { - def.non_enum_variant() - .fields - .iter() - .find(|item| item.ident.name == item_name) - }; - field.map(|item| { - if extra_fragment.is_some() { - let res = Res::Def( - if def.is_enum() { - DefKind::Variant - } else { - DefKind::Field - }, - item.did, - ); - Err(ErrorKind::AnchorFailure( - AnchorFailure::RustdocAnchorConflict(res), - )) - } else { - Ok(( - root_res, - Some(format!( - "{}.{}", - if def.is_enum() { "variant" } else { "structfield" }, - item.ident - )), - )) - } - }) - } - _ => None, - } - } else { - None + }); + } + + if ns != Namespace::ValueNS { + return None; } + debug!("looking for variants or fields named {} for {:?}", item_name, did); + // FIXME: this doesn't really belong in `associated_item` (maybe `variant_field` is better?) + // NOTE: it's different from variant_field because it resolves fields and variants, + // not variant fields (2 path segments, not 3). + let def = match tcx.type_of(did).kind() { + ty::Adt(def, _) => def, + _ => return None, + }; + let field = if def.is_enum() { + def.all_fields().find(|item| item.ident.name == item_name) + } else { + def.non_enum_variant().fields.iter().find(|item| item.ident.name == item_name) + }?; + Some(if extra_fragment.is_some() { + let res = Res::Def( + if def.is_enum() { DefKind::Variant } else { DefKind::Field }, + field.did, + ); + Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict(res))) + } else { + Ok(( + root_res, + Some(format!( + "{}.{}", + if def.is_enum() { "variant" } else { "structfield" }, + field.ident + )), + )) + }) } Res::Def(DefKind::Trait, did) => tcx .associated_items(did) From 3b7e654fad80e91064b26416a4334cd3e984a6ce Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 4 Apr 2021 16:23:08 -0400 Subject: [PATCH 7/9] Use more appropriate return type for `resolve_associated_item` Previously, the types looked like this: - None means this is not an associated item (but may be a variant field) - Some(Err) means this is known to be an error. I think the only way that can happen is if it resolved and but you had your own anchor. - Some(Ok(_, None)) was impossible. Now, this returns a nested Option and does the error handling and fiddling with the side channel in the caller. As a side-effect, it also removes duplicate error handling. This has one small change in behavior, which is that `resolve_primitive_associated_item` now goes through `variant_field` if it fails to resolve something. This is not ideal, but since it will be quickly rejected anyway, I think the performance hit is worth the cleanup. This also fixes a bug where struct fields would forget to set the side channel, adds a test for the bug, and ignores `private_intra_doc_links` in rustc_resolve (since it's always documented with --document-private-items). --- compiler/rustc_resolve/src/lib.rs | 1 + .../passes/collect_intra_doc_links.rs | 139 +++++++----------- .../intra-doc/private.private.stderr | 18 ++- .../intra-doc/private.public.stderr | 18 ++- src/test/rustdoc-ui/intra-doc/private.rs | 10 +- src/test/rustdoc/intra-doc/private.rs | 15 +- 6 files changed, 103 insertions(+), 98 deletions(-) diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index d474e99021104..1c5f8996e1b45 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -18,6 +18,7 @@ #![feature(nll)] #![cfg_attr(bootstrap, feature(or_patterns))] #![recursion_limit = "256"] +#![allow(rustdoc::private_intra_doc_links)] pub use rustc_hir::def::{Namespace, PerNS}; diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 54d5f1cabfce7..3501b7d86a46f 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -368,54 +368,28 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } /// Given a primitive type, try to resolve an associated item. - /// - /// HACK(jynelson): `item_str` is passed in instead of derived from `item_name` so the - /// lifetimes on `&'path` will work. fn resolve_primitive_associated_item( &self, prim_ty: PrimitiveType, ns: Namespace, - module_id: DefId, item_name: Symbol, - ) -> Result<(Res, Option), ErrorKind<'path>> { + ) -> Option<(Res, String, Option<(DefKind, DefId)>)> { let tcx = self.cx.tcx; - prim_ty - .impls(tcx) - .into_iter() - .find_map(|&impl_| { - tcx.associated_items(impl_) - .find_by_name_and_namespace(tcx, Ident::with_dummy_span(item_name), ns, impl_) - .map(|item| { - let kind = item.kind; - self.kind_side_channel.set(Some((kind.as_def_kind(), item.def_id))); - match kind { - ty::AssocKind::Fn => "method", - ty::AssocKind::Const => "associatedconstant", - ty::AssocKind::Type => "associatedtype", - } - }) - .map(|out| { - ( - Res::Primitive(prim_ty), - Some(format!("{}#{}.{}", prim_ty.as_str(), out, item_name)), - ) - }) - }) - .ok_or_else(|| { - debug!( - "returning primitive error for {}::{} in {} namespace", - prim_ty.as_str(), - item_name, - ns.descr() - ); - ResolutionFailure::NotResolved { - module_id, - partial_res: Some(Res::Primitive(prim_ty)), - unresolved: item_name.to_string().into(), - } - .into() - }) + prim_ty.impls(tcx).into_iter().find_map(|&impl_| { + tcx.associated_items(impl_) + .find_by_name_and_namespace(tcx, Ident::with_dummy_span(item_name), ns, impl_) + .map(|item| { + let kind = item.kind; + let out = match kind { + ty::AssocKind::Fn => "method", + ty::AssocKind::Const => "associatedconstant", + ty::AssocKind::Type => "associatedtype", + }; + let fragment = format!("{}#{}.{}", prim_ty.as_str(), out, item_name); + (Res::Primitive(prim_ty), fragment, Some((kind.as_def_kind(), item.def_id))) + }) + }) } /// Resolves a string as a macro. @@ -538,7 +512,21 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { resolve_primitive(&path_root, TypeNS) .or_else(|| self.resolve_path(&path_root, TypeNS, module_id)) .and_then(|ty_res| { - self.resolve_associated_item(ty_res, item_name, ns, module_id, extra_fragment) + let (res, fragment, side_channel) = + self.resolve_associated_item(ty_res, item_name, ns, module_id)?; + let result = if extra_fragment.is_some() { + let diag_res = side_channel.map_or(res, |(k, r)| Res::Def(k, r)); + Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict(diag_res))) + } else { + // HACK(jynelson): `clean` expects the type, not the associated item + // but the disambiguator logic expects the associated item. + // Store the kind in a side channel so that only the disambiguator logic looks at it. + if let Some((kind, id)) = side_channel { + self.kind_side_channel.set(Some((kind, id))); + } + Ok((res, Some(fragment))) + }; + Some(result) }) .unwrap_or_else(|| { if ns == Namespace::ValueNS { @@ -554,21 +542,21 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { }) } + /// Returns: + /// - None if no associated item was found + /// - Some((_, _, Some(_))) if an item was found and should go through a side channel + /// - Some((_, _, None)) otherwise fn resolve_associated_item( &mut self, root_res: Res, item_name: Symbol, ns: Namespace, module_id: DefId, - extra_fragment: &Option, - // lol this is so bad - ) -> Option), ErrorKind<'static>>> { + ) -> Option<(Res, String, Option<(DefKind, DefId)>)> { let tcx = self.cx.tcx; match root_res { - Res::Primitive(prim) => { - Some(self.resolve_primitive_associated_item(prim, ns, module_id, item_name)) - } + Res::Primitive(prim) => self.resolve_primitive_associated_item(prim, ns, item_name), Res::Def( DefKind::Struct | DefKind::Union @@ -611,17 +599,14 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { ty::AssocKind::Const => "associatedconstant", ty::AssocKind::Type => "associatedtype", }; - return Some(if extra_fragment.is_some() { - Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict( - root_res, - ))) - } else { - // HACK(jynelson): `clean` expects the type, not the associated item - // but the disambiguator logic expects the associated item. - // Store the kind in a side channel so that only the disambiguator logic looks at it. - self.kind_side_channel.set(Some((kind.as_def_kind(), id))); - Ok((root_res, Some(format!("{}.{}", out, item_name)))) - }); + // HACK(jynelson): `clean` expects the type, not the associated item + // but the disambiguator logic expects the associated item. + // Store the kind in a side channel so that only the disambiguator logic looks at it. + return Some(( + root_res, + format!("{}.{}", out, item_name), + Some((kind.as_def_kind(), id)), + )); } if ns != Namespace::ValueNS { @@ -640,22 +625,16 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } else { def.non_enum_variant().fields.iter().find(|item| item.ident.name == item_name) }?; - Some(if extra_fragment.is_some() { - let res = Res::Def( - if def.is_enum() { DefKind::Variant } else { DefKind::Field }, - field.did, - ); - Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict(res))) - } else { - Ok(( - root_res, - Some(format!( - "{}.{}", - if def.is_enum() { "variant" } else { "structfield" }, - field.ident - )), - )) - }) + let kind = if def.is_enum() { DefKind::Variant } else { DefKind::Field }; + Some(( + root_res, + format!( + "{}.{}", + if def.is_enum() { "variant" } else { "structfield" }, + field.ident + ), + Some((kind, field.did)), + )) } Res::Def(DefKind::Trait, did) => tcx .associated_items(did) @@ -673,14 +652,8 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } }; - if extra_fragment.is_some() { - Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict( - root_res, - ))) - } else { - let res = Res::Def(item.kind.as_def_kind(), item.def_id); - Ok((res, Some(format!("{}.{}", kind, item_name)))) - } + let res = Res::Def(item.kind.as_def_kind(), item.def_id); + (res, format!("{}.{}", kind, item_name), None) }), _ => None, } diff --git a/src/test/rustdoc-ui/intra-doc/private.private.stderr b/src/test/rustdoc-ui/intra-doc/private.private.stderr index cae5b1f20e6c3..392321f9c60db 100644 --- a/src/test/rustdoc-ui/intra-doc/private.private.stderr +++ b/src/test/rustdoc-ui/intra-doc/private.private.stderr @@ -1,19 +1,27 @@ warning: public documentation for `DocMe` links to private item `DontDocMe` - --> $DIR/private.rs:5:11 + --> $DIR/private.rs:7:11 | -LL | /// docs [DontDocMe] [DontDocMe::f] +LL | /// docs [DontDocMe] [DontDocMe::f] [DontDocMe::x] | ^^^^^^^^^ this item is private | = note: `#[warn(rustdoc::private_intra_doc_links)]` on by default = note: this link resolves only because you passed `--document-private-items`, but will break without warning: public documentation for `DocMe` links to private item `DontDocMe::f` - --> $DIR/private.rs:5:23 + --> $DIR/private.rs:7:23 | -LL | /// docs [DontDocMe] [DontDocMe::f] +LL | /// docs [DontDocMe] [DontDocMe::f] [DontDocMe::x] | ^^^^^^^^^^^^ this item is private | = note: this link resolves only because you passed `--document-private-items`, but will break without -warning: 2 warnings emitted +warning: public documentation for `DocMe` links to private item `DontDocMe::x` + --> $DIR/private.rs:7:38 + | +LL | /// docs [DontDocMe] [DontDocMe::f] [DontDocMe::x] + | ^^^^^^^^^^^^ this item is private + | + = note: this link resolves only because you passed `--document-private-items`, but will break without + +warning: 3 warnings emitted diff --git a/src/test/rustdoc-ui/intra-doc/private.public.stderr b/src/test/rustdoc-ui/intra-doc/private.public.stderr index 05b202e37fbcb..5d1c34b9168d9 100644 --- a/src/test/rustdoc-ui/intra-doc/private.public.stderr +++ b/src/test/rustdoc-ui/intra-doc/private.public.stderr @@ -1,19 +1,27 @@ warning: public documentation for `DocMe` links to private item `DontDocMe` - --> $DIR/private.rs:5:11 + --> $DIR/private.rs:7:11 | -LL | /// docs [DontDocMe] [DontDocMe::f] +LL | /// docs [DontDocMe] [DontDocMe::f] [DontDocMe::x] | ^^^^^^^^^ this item is private | = note: `#[warn(rustdoc::private_intra_doc_links)]` on by default = note: this link will resolve properly if you pass `--document-private-items` warning: public documentation for `DocMe` links to private item `DontDocMe::f` - --> $DIR/private.rs:5:23 + --> $DIR/private.rs:7:23 | -LL | /// docs [DontDocMe] [DontDocMe::f] +LL | /// docs [DontDocMe] [DontDocMe::f] [DontDocMe::x] | ^^^^^^^^^^^^ this item is private | = note: this link will resolve properly if you pass `--document-private-items` -warning: 2 warnings emitted +warning: public documentation for `DocMe` links to private item `DontDocMe::x` + --> $DIR/private.rs:7:38 + | +LL | /// docs [DontDocMe] [DontDocMe::f] [DontDocMe::x] + | ^^^^^^^^^^^^ this item is private + | + = note: this link will resolve properly if you pass `--document-private-items` + +warning: 3 warnings emitted diff --git a/src/test/rustdoc-ui/intra-doc/private.rs b/src/test/rustdoc-ui/intra-doc/private.rs index 3782864305f1f..525332ddaac3b 100644 --- a/src/test/rustdoc-ui/intra-doc/private.rs +++ b/src/test/rustdoc-ui/intra-doc/private.rs @@ -2,12 +2,16 @@ // revisions: public private // [private]compile-flags: --document-private-items -/// docs [DontDocMe] [DontDocMe::f] +// make sure to update `rustdoc/intra-doc/private.rs` if you update this file + +/// docs [DontDocMe] [DontDocMe::f] [DontDocMe::x] //~^ WARNING public documentation for `DocMe` links to private item `DontDocMe` +//~| WARNING public documentation for `DocMe` links to private item `DontDocMe::x` //~| WARNING public documentation for `DocMe` links to private item `DontDocMe::f` -// FIXME: for [private] we should also make sure the link was actually generated pub struct DocMe; -struct DontDocMe; +struct DontDocMe { + x: usize, +} impl DontDocMe { fn f() {} diff --git a/src/test/rustdoc/intra-doc/private.rs b/src/test/rustdoc/intra-doc/private.rs index f86ca44403d93..337102d6ab3fa 100644 --- a/src/test/rustdoc/intra-doc/private.rs +++ b/src/test/rustdoc/intra-doc/private.rs @@ -1,6 +1,17 @@ #![crate_name = "private"] // compile-flags: --document-private-items -/// docs [DontDocMe] + +// make sure to update `rustdoc-ui/intra-doc/private.rs` if you update this file + +/// docs [DontDocMe] [DontDocMe::f] [DontDocMe::x] // @has private/struct.DocMe.html '//*a[@href="../private/struct.DontDocMe.html"]' 'DontDocMe' +// @has private/struct.DocMe.html '//*a[@href="../private/struct.DontDocMe.html#method.f"]' 'DontDocMe::f' +// @has private/struct.DocMe.html '//*a[@href="../private/struct.DontDocMe.html#structfield.x"]' 'DontDocMe::x' pub struct DocMe; -struct DontDocMe; +struct DontDocMe { + x: usize, +} + +impl DontDocMe { + fn f() {} +} From 0a351abf83f1d146e2d259d404fb16a158721391 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Mon, 5 Apr 2021 08:38:09 -0400 Subject: [PATCH 8/9] Document compiler/ with -Aprivate-intra-doc-links Since compiler/ always passes --document-private-items, it's ok to link to items that are private. --- compiler/rustc_errors/src/diagnostic_builder.rs | 9 --------- src/bootstrap/doc.rs | 2 ++ 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_errors/src/diagnostic_builder.rs b/compiler/rustc_errors/src/diagnostic_builder.rs index 3fc63b4e50c14..282877d5dd109 100644 --- a/compiler/rustc_errors/src/diagnostic_builder.rs +++ b/compiler/rustc_errors/src/diagnostic_builder.rs @@ -45,9 +45,6 @@ macro_rules! forward { pub fn $n:ident(&self, $($name:ident: $ty:ty),* $(,)?) -> &Self ) => { $(#[$attrs])* - // we always document with --document-private-items - #[cfg_attr(not(bootstrap), allow(rustdoc::private_intra_doc_links))] - #[cfg_attr(bootstrap, allow(private_intra_doc_links))] #[doc = concat!("See [`Diagnostic::", stringify!($n), "()`].")] pub fn $n(&self, $($name: $ty),*) -> &Self { self.diagnostic.$n($($name),*); @@ -62,9 +59,6 @@ macro_rules! forward { ) => { $(#[$attrs])* #[doc = concat!("See [`Diagnostic::", stringify!($n), "()`].")] - // we always document with --document-private-items - #[cfg_attr(not(bootstrap), allow(rustdoc::private_intra_doc_links))] - #[cfg_attr(bootstrap, allow(private_intra_doc_links))] pub fn $n(&mut self, $($name: $ty),*) -> &mut Self { self.0.diagnostic.$n($($name),*); self @@ -82,9 +76,6 @@ macro_rules! forward { ) => { $(#[$attrs])* #[doc = concat!("See [`Diagnostic::", stringify!($n), "()`].")] - // we always document with --document-private-items - #[cfg_attr(not(bootstrap), allow(rustdoc::private_intra_doc_links))] - #[cfg_attr(bootstrap, allow(private_intra_doc_links))] pub fn $n<$($generic: $bound),*>(&mut self, $($name: $ty),*) -> &mut Self { self.0.diagnostic.$n($($name),*); self diff --git a/src/bootstrap/doc.rs b/src/bootstrap/doc.rs index fc79fc10fb4c5..f499f1a684d1d 100644 --- a/src/bootstrap/doc.rs +++ b/src/bootstrap/doc.rs @@ -549,6 +549,8 @@ impl Step for Rustc { // Build cargo command. let mut cargo = builder.cargo(compiler, Mode::Rustc, SourceType::InTree, target, "doc"); cargo.rustdocflag("--document-private-items"); + // Since we always pass --document-private-items, there's no need to warn about linking to private items. + cargo.rustdocflag("-Arustdoc::private-intra-doc-links"); cargo.rustdocflag("--enable-index-page"); cargo.rustdocflag("-Zunstable-options"); cargo.rustdocflag("-Znormalize-docs"); From f8653c9aca23317836277ef38decb2b220d9843d Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 4 Dec 2020 12:31:13 -0500 Subject: [PATCH 9/9] Add config file for tools enabling stage1 downloads by default Otherwise no one will be able to find the setting. --- src/bootstrap/defaults/config.compiler.toml | 3 +-- src/bootstrap/defaults/config.tools.toml | 16 ++++++++++++++++ src/bootstrap/setup.rs | 20 +++++++++++++++++--- 3 files changed, 34 insertions(+), 5 deletions(-) create mode 100644 src/bootstrap/defaults/config.tools.toml diff --git a/src/bootstrap/defaults/config.compiler.toml b/src/bootstrap/defaults/config.compiler.toml index 0ca928843d589..883bfead64e4a 100644 --- a/src/bootstrap/defaults/config.compiler.toml +++ b/src/bootstrap/defaults/config.compiler.toml @@ -8,6 +8,5 @@ debug-logging = true incremental = true [llvm] -# Will download LLVM from CI if available on your platform (Linux only for now) -# https://github.com/rust-lang/rust/issues/77084 tracks support for more platforms +# Will download LLVM from CI if available on your platform. download-ci-llvm = "if-available" diff --git a/src/bootstrap/defaults/config.tools.toml b/src/bootstrap/defaults/config.tools.toml new file mode 100644 index 0000000000000..182fb0fb0675c --- /dev/null +++ b/src/bootstrap/defaults/config.tools.toml @@ -0,0 +1,16 @@ +# These defaults are meant for contributors to tools which build on the +# compiler, but do not modify it directly. +[rust] +# This enables `RUSTC_LOG=debug`, avoiding confusing situations +# where adding `debug!()` appears to do nothing. +# However, it makes running the compiler slightly slower. +debug-logging = true +# This greatly increases the speed of rebuilds, especially when there are only minor changes. However, it makes the initial build slightly slower. +incremental = true +# Download rustc from CI instead of building it from source. +# This cuts compile times by almost 60x, but means you can't modify the compiler. +download-rustc = "if-unchanged" + +[llvm] +# Will download LLVM from CI if available on your platform. +download-ci-llvm = "if-available" diff --git a/src/bootstrap/setup.rs b/src/bootstrap/setup.rs index 725147767dbd1..a5829dfa9d879 100644 --- a/src/bootstrap/setup.rs +++ b/src/bootstrap/setup.rs @@ -13,6 +13,7 @@ pub enum Profile { Compiler, Codegen, Library, + Tools, User, } @@ -24,15 +25,16 @@ impl Profile { pub fn all() -> impl Iterator { use Profile::*; // N.B. these are ordered by how they are displayed, not alphabetically - [Library, Compiler, Codegen, User].iter().copied() + [Library, Compiler, Codegen, Tools, User].iter().copied() } pub fn purpose(&self) -> String { use Profile::*; match self { Library => "Contribute to the standard library", - Compiler => "Contribute to the compiler or rustdoc", + Compiler => "Contribute to the compiler itself", Codegen => "Contribute to the compiler, and also modify LLVM or codegen", + Tools => "Contribute to tools which depend on the compiler, but do not modify it directly (e.g. rustdoc, clippy, miri)", User => "Install Rust from source", } .to_string() @@ -53,9 +55,12 @@ impl FromStr for Profile { fn from_str(s: &str) -> Result { match s { "lib" | "library" => Ok(Profile::Library), - "compiler" | "rustdoc" => Ok(Profile::Compiler), + "compiler" => Ok(Profile::Compiler), "llvm" | "codegen" => Ok(Profile::Codegen), "maintainer" | "user" => Ok(Profile::User), + "tools" | "tool" | "rustdoc" | "clippy" | "miri" | "rustfmt" | "rls" => { + Ok(Profile::Tools) + } _ => Err(format!("unknown profile: '{}'", s)), } } @@ -68,6 +73,7 @@ impl fmt::Display for Profile { Profile::Codegen => write!(f, "codegen"), Profile::Library => write!(f, "library"), Profile::User => write!(f, "user"), + Profile::Tools => write!(f, "tools"), } } } @@ -103,6 +109,14 @@ pub fn setup(src_path: &Path, profile: Profile) { let suggestions = match profile { Profile::Codegen | Profile::Compiler => &["check", "build", "test"][..], + Profile::Tools => &[ + "check", + "build", + "test src/test/rustdoc*", + "test src/tools/clippy", + "test src/tools/miri", + "test src/tools/rustfmt", + ], Profile::Library => &["check", "build", "test library/std", "doc"], Profile::User => &["dist", "build"], };