From 1316c786a08344c965a97b1b67c76a300a479eec Mon Sep 17 00:00:00 2001 From: The8472 <git@infinite-source.de> Date: Wed, 12 Aug 2020 01:15:08 +0200 Subject: [PATCH 01/37] Workaround for copy_file_range spuriously returning EOPNOTSUPP when attemted on a NFS mount under RHEL/CentOS 7. The syscall is supposed to return ENOSYS in most cases but when calling it on NFS it may leak through EOPNOTSUPP even though that's supposed to be handled by the kernel and not returned to userspace. Since it returns ENOSYS in some cases anyway this will trip the HAS_COPY_FILE_RANGE detection anyway, so treat EOPNOTSUPP as if it were a ENOSYS. https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/7/html/7.8_release_notes/deprecated_functionality#the_literal_copy_file_range_literal_call_has_been_disabled_on_local_file_systems_and_in_nfs https://bugzilla.redhat.com/show_bug.cgi?id=1783554 --- library/std/src/sys/unix/fs.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index acb18e6d064e6..bcdc36a516e3a 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -1162,7 +1162,7 @@ pub fn copy(from: &Path, to: &Path) -> io::Result<u64> { }; if let Err(ref copy_err) = copy_result { match copy_err.raw_os_error() { - Some(libc::ENOSYS) | Some(libc::EPERM) => { + Some(libc::ENOSYS) | Some(libc::EPERM) | Some(libc::EOPNOTSUPP) => { HAS_COPY_FILE_RANGE.store(false, Ordering::Relaxed); } _ => {} @@ -1180,11 +1180,13 @@ pub fn copy(from: &Path, to: &Path) -> io::Result<u64> { if os_err == libc::ENOSYS || os_err == libc::EXDEV || os_err == libc::EINVAL - || os_err == libc::EPERM => + || os_err == libc::EPERM + || os_err == libc::EOPNOTSUPP => { // Try fallback io::copy if either: // - Kernel version is < 4.5 (ENOSYS) // - Files are mounted on different fs (EXDEV) + // - copy_file_range is broken in various ways on RHEL/CentOS 7 (EOPNOTSUPP) // - copy_file_range is disallowed, for example by seccomp (EPERM) // - copy_file_range cannot be used with pipes or device nodes (EINVAL) assert_eq!(written, 0); From f0783632d315db90c0ca34d79d56207d132f3764 Mon Sep 17 00:00:00 2001 From: The8472 <git@infinite-source.de> Date: Wed, 12 Aug 2020 20:09:55 +0200 Subject: [PATCH 02/37] more concise error matching --- library/std/src/sys/unix/fs.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index bcdc36a516e3a..63af25e309230 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -1162,7 +1162,7 @@ pub fn copy(from: &Path, to: &Path) -> io::Result<u64> { }; if let Err(ref copy_err) = copy_result { match copy_err.raw_os_error() { - Some(libc::ENOSYS) | Some(libc::EPERM) | Some(libc::EOPNOTSUPP) => { + Some(libc::ENOSYS | libc::EPERM | libc::EOPNOTSUPP) => { HAS_COPY_FILE_RANGE.store(false, Ordering::Relaxed); } _ => {} @@ -1176,13 +1176,9 @@ pub fn copy(from: &Path, to: &Path) -> io::Result<u64> { Ok(ret) => written += ret as u64, Err(err) => { match err.raw_os_error() { - Some(os_err) - if os_err == libc::ENOSYS - || os_err == libc::EXDEV - || os_err == libc::EINVAL - || os_err == libc::EPERM - || os_err == libc::EOPNOTSUPP => - { + Some( + libc::ENOSYS | libc::EXDEV | libc::EINVAL | libc::EPERM | libc::EOPNOTSUPP, + ) => { // Try fallback io::copy if either: // - Kernel version is < 4.5 (ENOSYS) // - Files are mounted on different fs (EXDEV) From 4ddedd521418d67e845ecb43dc02c09b0af53022 Mon Sep 17 00:00:00 2001 From: The8472 <git@infinite-source.de> Date: Fri, 14 Aug 2020 22:39:04 +0200 Subject: [PATCH 03/37] perform copy_file_range until EOF is reached instead of basing things on file size This solves several problems - race conditions where a file is truncated while copying from it. if we blindly trusted the file size this would lead to an infinite loop - proc files appearing empty to copy_file_range but not to read/write https://github.com/coreutils/coreutils/commit/4b04a0c - copy_file_range returning 0 for some filesystems (overlay? bind mounts?) inside docker, again leading to an infinite loop --- library/std/src/sys/unix/fs.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index 63af25e309230..566ac0920dc8f 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -1140,14 +1140,14 @@ pub fn copy(from: &Path, to: &Path) -> io::Result<u64> { } let (mut reader, reader_metadata) = open_from(from)?; - let len = reader_metadata.len(); + let max_len = u64::MAX; let (mut writer, _) = open_to_and_set_permissions(to, reader_metadata)?; let has_copy_file_range = HAS_COPY_FILE_RANGE.load(Ordering::Relaxed); let mut written = 0u64; - while written < len { + while written < max_len { let copy_result = if has_copy_file_range { - let bytes_to_copy = cmp::min(len - written, usize::MAX as u64) as usize; + let bytes_to_copy = cmp::min(max_len - written, usize::MAX as u64) as usize; let copy_result = unsafe { // We actually don't have to adjust the offsets, // because copy_file_range adjusts the file offset automatically @@ -1173,6 +1173,15 @@ pub fn copy(from: &Path, to: &Path) -> io::Result<u64> { Err(io::Error::from_raw_os_error(libc::ENOSYS)) }; match copy_result { + Ok(0) if written == 0 => { + // fallback to work around several kernel bugs where copy_file_range will fail to + // copy any bytes and return 0 instead of an error if + // - reading virtual files from the proc filesystem which appear to have 0 size + // but are not empty. noted in coreutils to affect kernels at least up to 5.6.19. + // - copying from an overlay filesystem in docker. reported to occur on fedora 32. + return io::copy(&mut reader, &mut writer); + } + Ok(0) => return Ok(written), // reached EOF Ok(ret) => written += ret as u64, Err(err) => { match err.raw_os_error() { From 18ad5a594ec4ee039897eb03d90dfeb117b4c6d6 Mon Sep 17 00:00:00 2001 From: Yuki Okushi <huyuumi.dev@gmail.com> Date: Wed, 19 Aug 2020 17:19:21 +0900 Subject: [PATCH 04/37] Add a regression test for issue-72793 --- .../ui/type-alias-impl-trait/issue-72793.rs | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 src/test/ui/type-alias-impl-trait/issue-72793.rs diff --git a/src/test/ui/type-alias-impl-trait/issue-72793.rs b/src/test/ui/type-alias-impl-trait/issue-72793.rs new file mode 100644 index 0000000000000..e643a8cab5b02 --- /dev/null +++ b/src/test/ui/type-alias-impl-trait/issue-72793.rs @@ -0,0 +1,27 @@ +// build-pass + +// Regression test for #72793. +// FIXME: This still shows ICE with `-Zmir-opt-level=2`. + +#![feature(type_alias_impl_trait)] + +trait T { type Item; } + +type Alias<'a> = impl T<Item = &'a ()>; + +struct S; +impl<'a> T for &'a S { + type Item = &'a (); +} + +fn filter_positive<'a>() -> Alias<'a> { + &S +} + +fn with_positive(fun: impl Fn(Alias<'_>)) { + fun(filter_positive()); +} + +fn main() { + with_positive(|_| ()); +} From aa40c028fcb20a47fb214fea2899ff9a8ae88840 Mon Sep 17 00:00:00 2001 From: Arkadiusz Piekarz <piekarzarkadiusz@gmail.com> Date: Wed, 26 Aug 2020 22:03:29 +0200 Subject: [PATCH 05/37] Unstable Book: add links to tracking issues for FFI features --- src/doc/unstable-book/src/language-features/ffi-const.md | 5 +++++ src/doc/unstable-book/src/language-features/ffi-pure.md | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/src/doc/unstable-book/src/language-features/ffi-const.md b/src/doc/unstable-book/src/language-features/ffi-const.md index 9a1ced4033b22..24a304437542d 100644 --- a/src/doc/unstable-book/src/language-features/ffi-const.md +++ b/src/doc/unstable-book/src/language-features/ffi-const.md @@ -1,5 +1,9 @@ # `ffi_const` +The tracking issue for this feature is: [#58328] + +------ + The `#[ffi_const]` attribute applies clang's `const` attribute to foreign functions declarations. @@ -42,6 +46,7 @@ implemented in this way on all of them. It is therefore also worth verifying that the semantics of the C toolchain used to compile the binary being linked against are compatible with those of the `#[ffi_const]`. +[#58328]: https://github.com/rust-lang/rust/issues/58328 [ARM C/C++ compiler]: http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0491c/Cacgigch.html [GCC]: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-const-function-attribute [IBM ILE C/C++]: https://www.ibm.com/support/knowledgecenter/fr/ssw_ibm_i_71/rzarg/fn_attrib_const.htm diff --git a/src/doc/unstable-book/src/language-features/ffi-pure.md b/src/doc/unstable-book/src/language-features/ffi-pure.md index 7bfd7a378f00b..4aef4eeab5532 100644 --- a/src/doc/unstable-book/src/language-features/ffi-pure.md +++ b/src/doc/unstable-book/src/language-features/ffi-pure.md @@ -1,5 +1,9 @@ # `ffi_pure` +The tracking issue for this feature is: [#58329] + +------ + The `#[ffi_pure]` attribute applies clang's `pure` attribute to foreign functions declarations. @@ -46,6 +50,7 @@ that the semantics of the C toolchain used to compile the binary being linked against are compatible with those of the `#[ffi_pure]`. +[#58329]: https://github.com/rust-lang/rust/issues/58329 [ARM C/C++ compiler]: http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0491c/Cacigdac.html [GCC]: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-pure-function-attribute [IBM ILE C/C++]: https://www.ibm.com/support/knowledgecenter/fr/ssw_ibm_i_71/rzarg/fn_attrib_pure.htm From f03d0b38d6a33a64307d83f8ddd3df8ef57ca537 Mon Sep 17 00:00:00 2001 From: mental32 <m3nta1@yahoo.com> Date: Thu, 27 Aug 2020 19:19:29 +0100 Subject: [PATCH 06/37] `impl Rc::new_cyclic` --- library/alloc/src/rc.rs | 34 ++++++++++++++++++ library/alloc/src/rc/tests.rs | 66 +++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+) diff --git a/library/alloc/src/rc.rs b/library/alloc/src/rc.rs index 1046397f4be60..76266d77bb0d2 100644 --- a/library/alloc/src/rc.rs +++ b/library/alloc/src/rc.rs @@ -325,6 +325,40 @@ impl<T> Rc<T> { ) } + /// Constructs a new `Rc<T>` using a weak reference to itself. Attempting + /// to upgrade the weak reference before this function retuns will result + /// in a `None` value. However, the weak reference may be cloned freely and + /// stored for use at a later time. + #[inline] + #[unstable(feature = "arc_new_cyclic", issue = "75861")] + pub fn new_cyclic(data_fn: impl FnOnce(&Weak<T>) -> T) -> Rc<T> { + let uninit_ptr: NonNull<_> = Box::leak(box RcBox { + strong: Cell::new(0), + weak: Cell::new(1), + value: mem::MaybeUninit::<T>::uninit(), + }) + .into(); + + let init_ptr: NonNull<RcBox<T>> = uninit_ptr.cast(); + + let weak = Weak { ptr: init_ptr }; + + let data = data_fn(&weak); + + unsafe { + let inner = init_ptr.as_ptr(); + ptr::write(&raw mut (*inner).value, data); + + let prev_value = (*inner).strong.get(); + debug_assert_eq!(prev_value, 0, "No prior strong references should exist"); + (*inner).strong.set(1); + } + + let strong = Rc::from_inner(init_ptr); + mem::forget(weak); + strong + } + /// Constructs a new `Rc` with uninitialized contents. /// /// # Examples diff --git a/library/alloc/src/rc/tests.rs b/library/alloc/src/rc/tests.rs index e88385faf4fd4..fed48a59f809e 100644 --- a/library/alloc/src/rc/tests.rs +++ b/library/alloc/src/rc/tests.rs @@ -434,3 +434,69 @@ fn test_array_from_slice() { let a: Result<Rc<[u32; 2]>, _> = r.clone().try_into(); assert!(a.is_err()); } + +#[test] +fn test_rc_cyclic_with_zero_refs() { + struct ZeroRefs { + inner: Weak<ZeroRefs>, + } + + let zero_refs = Rc::new_cyclic(|inner| { + assert_eq!(inner.strong_count(), 0); + assert!(inner.upgrade().is_none()); + ZeroRefs { inner: Weak::new() } + }); + + assert_eq!(Rc::strong_count(&zero_refs), 1); + assert_eq!(Rc::weak_count(&zero_refs), 0); + assert_eq!(zero_refs.inner.strong_count(), 0); + assert_eq!(zero_refs.inner.weak_count(), 0); +} + +#[test] +fn test_rc_cyclic_with_one_ref() { + struct OneRef { + inner: Weak<OneRef>, + } + + let one_ref = Rc::new_cyclic(|inner| { + assert_eq!(inner.strong_count(), 0); + assert!(inner.upgrade().is_none()); + OneRef { inner: inner.clone() } + }); + + assert_eq!(Rc::strong_count(&one_ref), 1); + assert_eq!(Rc::weak_count(&one_ref), 1); + + let one_ref2 = Weak::upgrade(&one_ref.inner).unwrap(); + assert!(Rc::ptr_eq(&one_ref, &one_ref2)); + + assert_eq!(one_ref.inner.strong_count(), 2); + assert_eq!(one_ref.inner.weak_count(), 1); +} + +#[test] +fn test_rc_cyclic_with_two_ref() { + struct TwoRefs { + inner: Weak<TwoRefs>, + inner1: Weak<TwoRefs>, + } + + let two_refs = Rc::new_cyclic(|inner| { + assert_eq!(inner.strong_count(), 0); + assert!(inner.upgrade().is_none()); + TwoRefs { inner: inner.clone(), inner1: inner.clone() } + }); + + assert_eq!(Rc::strong_count(&two_refs), 1); + assert_eq!(Rc::weak_count(&two_refs), 2); + + let two_ref3 = Weak::upgrade(&two_refs.inner).unwrap(); + assert!(Rc::ptr_eq(&two_refs, &two_ref3)); + + let two_ref2 = Weak::upgrade(&two_refs.inner1).unwrap(); + assert!(Rc::ptr_eq(&two_refs, &two_ref2)); + + assert_eq!(Rc::strong_count(&two_refs), 3); + assert_eq!(Rc::weak_count(&two_refs), 2); +} From 42fb27001e07e832cb40604c7daeaa6aada07675 Mon Sep 17 00:00:00 2001 From: mental <m3nta1@yahoo.com> Date: Sat, 29 Aug 2020 07:39:03 +0100 Subject: [PATCH 07/37] typo Co-authored-by: Andrew Hickman <andrew.hickman1@sky.com> --- library/alloc/src/rc.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/alloc/src/rc.rs b/library/alloc/src/rc.rs index 76266d77bb0d2..7dbdc8f6017fe 100644 --- a/library/alloc/src/rc.rs +++ b/library/alloc/src/rc.rs @@ -326,7 +326,7 @@ impl<T> Rc<T> { } /// Constructs a new `Rc<T>` using a weak reference to itself. Attempting - /// to upgrade the weak reference before this function retuns will result + /// to upgrade the weak reference before this function returns will result /// in a `None` value. However, the weak reference may be cloned freely and /// stored for use at a later time. #[inline] From bb5e79cbd11dbb8aa91840108b67085e5d5d04f8 Mon Sep 17 00:00:00 2001 From: Ivan Tham <pickfire@riseup.net> Date: Sat, 29 Aug 2020 18:47:11 +0800 Subject: [PATCH 08/37] Link vec doc to & reference It is not always obvious that people could see the docs for `&` especially for beginners, it also helps learnability. --- library/alloc/src/vec.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index b4ad238680f79..5e6493c27ae0f 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -159,7 +159,7 @@ use crate::raw_vec::RawVec; /// # Slicing /// /// A `Vec` can be mutable. Slices, on the other hand, are read-only objects. -/// To get a slice, use `&`. Example: +/// To get a slice, use [`&`][prim@reference]. Example: /// /// ``` /// fn read_slice(slice: &[usize]) { From 1b1935452f26593d07fa17ccbe3308901dda891b Mon Sep 17 00:00:00 2001 From: Joshua Nelson <jyn514@gmail.com> Date: Sat, 29 Aug 2020 16:32:17 -0400 Subject: [PATCH 09/37] [WIP] Fix intra-doc links on pub re-exports This removes the incorrect error, but doesn't show the documentation anywhere. --- src/librustdoc/passes/collect_intra_doc_links.rs | 4 ++++ src/test/rustdoc/intra-link-pub-use.rs | 8 ++++++++ 2 files changed, 12 insertions(+) create mode 100644 src/test/rustdoc/intra-link-pub-use.rs diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 65d116b9c670c..be6a7e25c881d 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -578,6 +578,9 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { let parent_node = if item.is_fake() { // FIXME: is this correct? None + // If we're documenting the crate root itself, it has no parent. Use the root instead. + } else if item.def_id.is_top_level_module() { + Some(item.def_id) } else { let mut current = item.def_id; // The immediate parent might not always be a module. @@ -589,6 +592,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { } current = parent; } else { + debug!("{:?} has no parent (kind={:?}, original was {:?})", current, self.cx.tcx.def_kind(current), item.def_id); break None; } } diff --git a/src/test/rustdoc/intra-link-pub-use.rs b/src/test/rustdoc/intra-link-pub-use.rs new file mode 100644 index 0000000000000..a5ceeafc80abc --- /dev/null +++ b/src/test/rustdoc/intra-link-pub-use.rs @@ -0,0 +1,8 @@ +#![deny(intra_doc_link_resolution_failure)] + +/// [std::env] [g] +// @has intra_link_pub_use/index.html '//a[@href="https://doc.rust-lang.org/nightly/std/env/fn.var.html"]' "std::env" +// @has - '//a[@href="../intra_link_pub_use/fn.f.html"]' "g" +pub use f as g; + +pub fn f() {} From 20a68666d89a85e14021f656233a086e46ff8a34 Mon Sep 17 00:00:00 2001 From: Ivan Tham <pickfire@riseup.net> Date: Sun, 30 Aug 2020 09:17:22 +0800 Subject: [PATCH 10/37] Try removing [prim@reference] --- library/alloc/src/vec.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index 5e6493c27ae0f..35d64d85010c3 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -159,7 +159,7 @@ use crate::raw_vec::RawVec; /// # Slicing /// /// A `Vec` can be mutable. Slices, on the other hand, are read-only objects. -/// To get a slice, use [`&`][prim@reference]. Example: +/// To get a slice, use [`&`]. Example: /// /// ``` /// fn read_slice(slice: &[usize]) { From e885f00f24aab657b3a9835818fc96e638e7fb21 Mon Sep 17 00:00:00 2001 From: Joshua Nelson <jyn514@gmail.com> Date: Sun, 30 Aug 2020 08:44:20 -0400 Subject: [PATCH 11/37] Comment out test for generated docs until rustdoc changes its behavior around documenting re-exports --- src/librustdoc/passes/collect_intra_doc_links.rs | 7 ++++++- src/test/rustdoc/intra-link-pub-use.rs | 15 ++++++++++++--- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index be6a7e25c881d..55d7974a9ef6c 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -592,7 +592,12 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { } current = parent; } else { - debug!("{:?} has no parent (kind={:?}, original was {:?})", current, self.cx.tcx.def_kind(current), item.def_id); + debug!( + "{:?} has no parent (kind={:?}, original was {:?})", + current, + self.cx.tcx.def_kind(current), + item.def_id + ); break None; } } diff --git a/src/test/rustdoc/intra-link-pub-use.rs b/src/test/rustdoc/intra-link-pub-use.rs index a5ceeafc80abc..f49edea410d73 100644 --- a/src/test/rustdoc/intra-link-pub-use.rs +++ b/src/test/rustdoc/intra-link-pub-use.rs @@ -1,8 +1,17 @@ -#![deny(intra_doc_link_resolution_failure)] +#![deny(broken_intra_doc_links)] /// [std::env] [g] -// @has intra_link_pub_use/index.html '//a[@href="https://doc.rust-lang.org/nightly/std/env/fn.var.html"]' "std::env" -// @has - '//a[@href="../intra_link_pub_use/fn.f.html"]' "g" +// FIXME: This can't be tested because rustdoc doesn't show documentation on pub re-exports. +// Until then, comment out the `htmldocck` test. +// This test still does something; namely check that no incorrect errors are emitted when +// documenting the re-export. + +// @has intra_link_pub_use/index.html +// @ has - '//a[@href="https://doc.rust-lang.org/nightly/std/env/fn.var.html"]' "std::env" +// @ has - '//a[@href="../intra_link_pub_use/fn.f.html"]' "g" pub use f as g; +/// [std::env] +extern crate self as _; + pub fn f() {} From d7150154fa5c35c0b570570f156ba3a5cc6dfb1d Mon Sep 17 00:00:00 2001 From: Joshua Nelson <jyn514@gmail.com> Date: Sun, 30 Aug 2020 12:06:40 -0400 Subject: [PATCH 12/37] Improve tests Now this actually tests the links are generated correctly --- .../rustdoc/auxiliary/intra-link-pub-use.rs | 4 ++++ src/test/rustdoc/intra-link-pub-use.rs | 18 ++++++++++++++---- 2 files changed, 18 insertions(+), 4 deletions(-) create mode 100644 src/test/rustdoc/auxiliary/intra-link-pub-use.rs diff --git a/src/test/rustdoc/auxiliary/intra-link-pub-use.rs b/src/test/rustdoc/auxiliary/intra-link-pub-use.rs new file mode 100644 index 0000000000000..a4db2ffc445f8 --- /dev/null +++ b/src/test/rustdoc/auxiliary/intra-link-pub-use.rs @@ -0,0 +1,4 @@ +#![crate_name = "inner"] + +/// Documentation, including a link to [std::ptr] +pub fn f() {} diff --git a/src/test/rustdoc/intra-link-pub-use.rs b/src/test/rustdoc/intra-link-pub-use.rs index f49edea410d73..dd52249abc6d0 100644 --- a/src/test/rustdoc/intra-link-pub-use.rs +++ b/src/test/rustdoc/intra-link-pub-use.rs @@ -1,17 +1,27 @@ +// aux-build: intra-link-pub-use.rs #![deny(broken_intra_doc_links)] +#![crate_name = "outer"] + +extern crate inner; + +/// [mod@std::env] [g] -/// [std::env] [g] // FIXME: This can't be tested because rustdoc doesn't show documentation on pub re-exports. // Until then, comment out the `htmldocck` test. // This test still does something; namely check that no incorrect errors are emitted when // documenting the re-export. -// @has intra_link_pub_use/index.html +// @has outer/index.html // @ has - '//a[@href="https://doc.rust-lang.org/nightly/std/env/fn.var.html"]' "std::env" -// @ has - '//a[@href="../intra_link_pub_use/fn.f.html"]' "g" +// @ has - '//a[@href="../outer/fn.f.html"]' "g" pub use f as g; +// FIXME: same as above /// [std::env] extern crate self as _; -pub fn f() {} +// Make sure the documentation is actually correct by documenting an inlined re-export +/// [mod@std::env] +// @has outer/fn.f.html +// @has - '//a[@href="https://doc.rust-lang.org/nightly/std/env/index.html"]' "std::env" +pub use inner::f; From 81e85ce76d6630a8f87f81df2470e76fbe73de7d Mon Sep 17 00:00:00 2001 From: Alexis Bourget <alexis.bourget@gmail.com> Date: Sun, 30 Aug 2020 21:59:43 +0200 Subject: [PATCH 13/37] Move to Arc::clone(&x) over x.clone() in library/std --- library/std/src/collections/hash/map.rs | 4 ++-- library/std/src/sync/barrier.rs | 4 ++-- library/std/src/sync/condvar.rs | 18 +++++++++--------- library/std/src/sync/mutex.rs | 8 ++++---- library/std/src/sync/rwlock.rs | 4 ++-- library/std/src/sys_common/poison.rs | 4 ++-- 6 files changed, 21 insertions(+), 21 deletions(-) diff --git a/library/std/src/collections/hash/map.rs b/library/std/src/collections/hash/map.rs index 70f7214e2f1d7..56f63002b7f11 100644 --- a/library/std/src/collections/hash/map.rs +++ b/library/std/src/collections/hash/map.rs @@ -2381,7 +2381,7 @@ impl<'a, K, V> OccupiedEntry<'a, K, V> { /// use std::rc::Rc; /// /// let mut map: HashMap<Rc<String>, u32> = HashMap::new(); - /// let mut known_strings: Vec<Rc<String>> = Vec::new(); + /// let known_strings: Vec<Rc<String>> = Vec::new(); /// /// // Initialise known strings, run program, etc. /// @@ -2389,7 +2389,7 @@ impl<'a, K, V> OccupiedEntry<'a, K, V> { /// /// fn reclaim_memory(map: &mut HashMap<Rc<String>, u32>, known_strings: &[Rc<String>] ) { /// for s in known_strings { - /// if let Entry::Occupied(entry) = map.entry(s.clone()) { + /// if let Entry::Occupied(entry) = map.entry(Rc::clone(s)) { /// // Replaces the entry's key with our version of it in `known_strings`. /// entry.replace_key(); /// } diff --git a/library/std/src/sync/barrier.rs b/library/std/src/sync/barrier.rs index 23c989fd2fdfb..61f097ccf250c 100644 --- a/library/std/src/sync/barrier.rs +++ b/library/std/src/sync/barrier.rs @@ -13,7 +13,7 @@ use crate::sync::{Condvar, Mutex}; /// let mut handles = Vec::with_capacity(10); /// let barrier = Arc::new(Barrier::new(10)); /// for _ in 0..10 { -/// let c = barrier.clone(); +/// let c = Arc::clone(&barrier); /// // The same messages will be printed together. /// // You will NOT see any interleaving. /// handles.push(thread::spawn(move|| { @@ -110,7 +110,7 @@ impl Barrier { /// let mut handles = Vec::with_capacity(10); /// let barrier = Arc::new(Barrier::new(10)); /// for _ in 0..10 { - /// let c = barrier.clone(); + /// let c = Arc::clone(&barrier); /// // The same messages will be printed together. /// // You will NOT see any interleaving. /// handles.push(thread::spawn(move|| { diff --git a/library/std/src/sync/condvar.rs b/library/std/src/sync/condvar.rs index 4efd86aa3ede8..1fbcb0ac08e0f 100644 --- a/library/std/src/sync/condvar.rs +++ b/library/std/src/sync/condvar.rs @@ -33,7 +33,7 @@ impl WaitTimeoutResult { /// use std::time::Duration; /// /// let pair = Arc::new((Mutex::new(false), Condvar::new())); - /// let pair2 = pair.clone(); + /// let pair2 = Arc::clone(&pair); /// /// thread::spawn(move || { /// let (lock, cvar) = &*pair2; @@ -90,7 +90,7 @@ impl WaitTimeoutResult { /// use std::thread; /// /// let pair = Arc::new((Mutex::new(false), Condvar::new())); -/// let pair2 = pair.clone(); +/// let pair2 = Arc::clone(&pair); /// /// // Inside of our lock, spawn a new thread, and then wait for it to start. /// thread::spawn(move|| { @@ -173,7 +173,7 @@ impl Condvar { /// use std::thread; /// /// let pair = Arc::new((Mutex::new(false), Condvar::new())); - /// let pair2 = pair.clone(); + /// let pair2 = Arc::clone(&pair); /// /// thread::spawn(move|| { /// let (lock, cvar) = &*pair2; @@ -229,7 +229,7 @@ impl Condvar { /// use std::thread; /// /// let pair = Arc::new((Mutex::new(true), Condvar::new())); - /// let pair2 = pair.clone(); + /// let pair2 = Arc::clone(&pair); /// /// thread::spawn(move|| { /// let (lock, cvar) = &*pair2; @@ -288,7 +288,7 @@ impl Condvar { /// use std::thread; /// /// let pair = Arc::new((Mutex::new(false), Condvar::new())); - /// let pair2 = pair.clone(); + /// let pair2 = Arc::clone(&pair); /// /// thread::spawn(move|| { /// let (lock, cvar) = &*pair2; @@ -360,7 +360,7 @@ impl Condvar { /// use std::time::Duration; /// /// let pair = Arc::new((Mutex::new(false), Condvar::new())); - /// let pair2 = pair.clone(); + /// let pair2 = Arc::clone(&pair); /// /// thread::spawn(move|| { /// let (lock, cvar) = &*pair2; @@ -429,7 +429,7 @@ impl Condvar { /// use std::time::Duration; /// /// let pair = Arc::new((Mutex::new(true), Condvar::new())); - /// let pair2 = pair.clone(); + /// let pair2 = Arc::clone(&pair); /// /// thread::spawn(move|| { /// let (lock, cvar) = &*pair2; @@ -493,7 +493,7 @@ impl Condvar { /// use std::thread; /// /// let pair = Arc::new((Mutex::new(false), Condvar::new())); - /// let pair2 = pair.clone(); + /// let pair2 = Arc::clone(&pair); /// /// thread::spawn(move|| { /// let (lock, cvar) = &*pair2; @@ -533,7 +533,7 @@ impl Condvar { /// use std::thread; /// /// let pair = Arc::new((Mutex::new(false), Condvar::new())); - /// let pair2 = pair.clone(); + /// let pair2 = Arc::clone(&pair); /// /// thread::spawn(move|| { /// let (lock, cvar) = &*pair2; diff --git a/library/std/src/sync/mutex.rs b/library/std/src/sync/mutex.rs index d7a4f00305c71..39a5a455fe4d5 100644 --- a/library/std/src/sync/mutex.rs +++ b/library/std/src/sync/mutex.rs @@ -85,7 +85,7 @@ use crate::sys_common::poison::{self, LockResult, TryLockError, TryLockResult}; /// use std::thread; /// /// let lock = Arc::new(Mutex::new(0_u32)); -/// let lock2 = lock.clone(); +/// let lock2 = Arc::clone(&lock); /// /// let _ = thread::spawn(move || -> () { /// // This thread will acquire the mutex first, unwrapping the result of @@ -256,7 +256,7 @@ impl<T: ?Sized> Mutex<T> { /// use std::thread; /// /// let mutex = Arc::new(Mutex::new(0)); - /// let c_mutex = mutex.clone(); + /// let c_mutex = Arc::clone(&mutex); /// /// thread::spawn(move || { /// *c_mutex.lock().unwrap() = 10; @@ -292,7 +292,7 @@ impl<T: ?Sized> Mutex<T> { /// use std::thread; /// /// let mutex = Arc::new(Mutex::new(0)); - /// let c_mutex = mutex.clone(); + /// let c_mutex = Arc::clone(&mutex); /// /// thread::spawn(move || { /// let mut lock = c_mutex.try_lock(); @@ -328,7 +328,7 @@ impl<T: ?Sized> Mutex<T> { /// use std::thread; /// /// let mutex = Arc::new(Mutex::new(0)); - /// let c_mutex = mutex.clone(); + /// let c_mutex = Arc::clone(&mutex); /// /// let _ = thread::spawn(move || { /// let _lock = c_mutex.lock().unwrap(); diff --git a/library/std/src/sync/rwlock.rs b/library/std/src/sync/rwlock.rs index 586093c916dea..b7f135f5c8af4 100644 --- a/library/std/src/sync/rwlock.rs +++ b/library/std/src/sync/rwlock.rs @@ -162,7 +162,7 @@ impl<T: ?Sized> RwLock<T> { /// use std::thread; /// /// let lock = Arc::new(RwLock::new(1)); - /// let c_lock = lock.clone(); + /// let c_lock = Arc::clone(&lock); /// /// let n = lock.read().unwrap(); /// assert_eq!(*n, 1); @@ -318,7 +318,7 @@ impl<T: ?Sized> RwLock<T> { /// use std::thread; /// /// let lock = Arc::new(RwLock::new(0)); - /// let c_lock = lock.clone(); + /// let c_lock = Arc::clone(&lock); /// /// let _ = thread::spawn(move || { /// let _lock = c_lock.write().unwrap(); diff --git a/library/std/src/sys_common/poison.rs b/library/std/src/sys_common/poison.rs index 3f079da9098c7..2ab2c700a1bf1 100644 --- a/library/std/src/sys_common/poison.rs +++ b/library/std/src/sys_common/poison.rs @@ -65,7 +65,7 @@ pub struct Guard { /// let mutex = Arc::new(Mutex::new(1)); /// /// // poison the mutex -/// let c_mutex = mutex.clone(); +/// let c_mutex = Arc::clone(&mutex); /// let _ = thread::spawn(move || { /// let mut data = c_mutex.lock().unwrap(); /// *data = 2; @@ -168,7 +168,7 @@ impl<T> PoisonError<T> { /// let mutex = Arc::new(Mutex::new(HashSet::new())); /// /// // poison the mutex - /// let c_mutex = mutex.clone(); + /// let c_mutex = Arc::clone(&mutex); /// let _ = thread::spawn(move || { /// let mut data = c_mutex.lock().unwrap(); /// data.insert(10); From 6b75e3d11b89671df0a3f284252584414bfe17d9 Mon Sep 17 00:00:00 2001 From: Alexis Bourget <alexis.bourget@gmail.com> Date: Sun, 30 Aug 2020 22:14:17 +0200 Subject: [PATCH 14/37] Move to Arc::clone(&x) over x.clone() in library/core --- library/core/src/pin.rs | 2 +- library/core/src/sync/atomic.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/library/core/src/pin.rs b/library/core/src/pin.rs index 8f60c4787d459..fc17f64879a50 100644 --- a/library/core/src/pin.rs +++ b/library/core/src/pin.rs @@ -541,7 +541,7 @@ impl<P: Deref> Pin<P> { /// use std::pin::Pin; /// /// fn move_pinned_rc<T>(mut x: Rc<T>) { - /// let pinned = unsafe { Pin::new_unchecked(x.clone()) }; + /// let pinned = unsafe { Pin::new_unchecked(Rc::clone(&x)) }; /// { /// let p: Pin<&T> = pinned.as_ref(); /// // This should mean the pointee can never move again. diff --git a/library/core/src/sync/atomic.rs b/library/core/src/sync/atomic.rs index 1aec7e1b5f871..38eabaaa396e0 100644 --- a/library/core/src/sync/atomic.rs +++ b/library/core/src/sync/atomic.rs @@ -76,7 +76,7 @@ //! fn main() { //! let spinlock = Arc::new(AtomicUsize::new(1)); //! -//! let spinlock_clone = spinlock.clone(); +//! let spinlock_clone = Arc::clone(&spinlock); //! let thread = thread::spawn(move|| { //! spinlock_clone.store(0, Ordering::SeqCst); //! }); From 0f301e8bb40aaf7cbfefb8c16ce3b0a112c6d5c1 Mon Sep 17 00:00:00 2001 From: mental <m3nta1@yahoo.com> Date: Tue, 1 Sep 2020 09:46:48 +0100 Subject: [PATCH 15/37] Removed [inline] and copied over comments from Arc::new_cyclic --- library/alloc/src/rc.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/library/alloc/src/rc.rs b/library/alloc/src/rc.rs index 7dbdc8f6017fe..1a2dfd2888261 100644 --- a/library/alloc/src/rc.rs +++ b/library/alloc/src/rc.rs @@ -329,9 +329,10 @@ impl<T> Rc<T> { /// to upgrade the weak reference before this function returns will result /// in a `None` value. However, the weak reference may be cloned freely and /// stored for use at a later time. - #[inline] #[unstable(feature = "arc_new_cyclic", issue = "75861")] pub fn new_cyclic(data_fn: impl FnOnce(&Weak<T>) -> T) -> Rc<T> { + // Construct the inner in the "uninitialized" state with a single + // weak reference. let uninit_ptr: NonNull<_> = Box::leak(box RcBox { strong: Cell::new(0), weak: Cell::new(1), @@ -343,6 +344,12 @@ impl<T> Rc<T> { let weak = Weak { ptr: init_ptr }; + // It's important we don't give up ownership of the weak pointer, or + // else the memory might be freed by the time `data_fn` returns. If + // we really wanted to pass ownership, we could create an additional + // weak pointer for ourselves, but this would result in additional + // updates to the weak reference count which might not be necessary + // otherwise. let data = data_fn(&weak); unsafe { @@ -355,6 +362,9 @@ impl<T> Rc<T> { } let strong = Rc::from_inner(init_ptr); + + // Strong references should collectively own a shared weak reference, + // so don't run the destructor for our old weak reference. mem::forget(weak); strong } From af19262ab447b358ae6da516751fd9ad79695996 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= <tomasz.miasko@gmail.com> Date: Wed, 2 Sep 2020 00:00:00 +0000 Subject: [PATCH 16/37] Fold length constant in Rvalue::Repeat --- .../rustc_middle/src/mir/type_foldable.rs | 2 +- src/test/ui/mir/issue-76248.rs | 29 +++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/mir/issue-76248.rs diff --git a/compiler/rustc_middle/src/mir/type_foldable.rs b/compiler/rustc_middle/src/mir/type_foldable.rs index 6bb6abe028910..ad2eae0298cec 100644 --- a/compiler/rustc_middle/src/mir/type_foldable.rs +++ b/compiler/rustc_middle/src/mir/type_foldable.rs @@ -175,7 +175,7 @@ impl<'tcx> TypeFoldable<'tcx> for Rvalue<'tcx> { use crate::mir::Rvalue::*; match *self { Use(ref op) => Use(op.fold_with(folder)), - Repeat(ref op, len) => Repeat(op.fold_with(folder), len), + Repeat(ref op, len) => Repeat(op.fold_with(folder), len.fold_with(folder)), ThreadLocalRef(did) => ThreadLocalRef(did.fold_with(folder)), Ref(region, bk, ref place) => { Ref(region.fold_with(folder), bk, place.fold_with(folder)) diff --git a/src/test/ui/mir/issue-76248.rs b/src/test/ui/mir/issue-76248.rs new file mode 100644 index 0000000000000..b01a9727852cd --- /dev/null +++ b/src/test/ui/mir/issue-76248.rs @@ -0,0 +1,29 @@ +// This used to ICE during codegen after MIR inlining of g into f. +// The root cause was a missing fold of length constant in Rvalue::Repeat. +// Regression test for #76248. +// +// build-pass +// compile-flags: -Zmir-opt-level=2 + +const N: usize = 1; + +pub struct Elem<M> { + pub x: [usize; N], + pub m: M, +} + +pub fn f() -> Elem<()> { + g(()) +} + +#[inline] +pub fn g<M>(m: M) -> Elem<M> { + Elem { + x: [0; N], + m, + } +} + +pub fn main() { + f(); +} From 22769917b0c55786540f2445c6e04fa95fe82c2f Mon Sep 17 00:00:00 2001 From: Mark Rousskov <mark.simulacrum@gmail.com> Date: Wed, 2 Sep 2020 16:00:47 -0400 Subject: [PATCH 17/37] Avoid spurious print outs during dryrun --- src/bootstrap/check.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bootstrap/check.rs b/src/bootstrap/check.rs index 9f34bb4e6ccd7..1e26e7754aa21 100644 --- a/src/bootstrap/check.rs +++ b/src/bootstrap/check.rs @@ -160,12 +160,12 @@ macro_rules! tool_check_step { &[], ); - println!( + builder.info(&format!( "Checking {} artifacts ({} -> {})", stringify!($name).to_lowercase(), &compiler.host.triple, target.triple - ); + )); run_cargo( builder, cargo, From 8783c62f0c8641ef56094fae13e2fed193b60d1f Mon Sep 17 00:00:00 2001 From: Camelid <37223377+camelid@users.noreply.github.com> Date: Tue, 1 Sep 2020 17:48:15 -0700 Subject: [PATCH 18/37] Add missing link in README --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index a7e23d8ac2caa..6cf9577d3330e 100644 --- a/README.md +++ b/README.md @@ -243,6 +243,8 @@ The Rust community congregates in a few places: If you are interested in contributing to the Rust project, please take a look at the [Getting Started][gettingstarted] guide in the [rustc-dev-guide]. +[rustc-dev-guide]: https://rustc-dev-guide.rust-lang.org + ## License Rust is primarily distributed under the terms of both the MIT license From 89ae59a360b18e34b4cdbec8e53c9a4e05b7db14 Mon Sep 17 00:00:00 2001 From: Joshua Nelson <jyn514@gmail.com> Date: Sun, 5 Jul 2020 22:10:18 -0400 Subject: [PATCH 19/37] Remove needless .to_owned() for link --- src/librustdoc/passes/collect_intra_doc_links.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index f497f341e112d..b6707a55959c7 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -695,9 +695,9 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { // This is an anchor to an element of the current page, nothing to do in here! continue; } - (parts[0].to_owned(), Some(parts[1].to_owned())) + (parts[0], Some(parts[1].to_owned())) } else { - (parts[0].to_owned(), None) + (parts[0], None) }; let resolved_self; let mut path_str; From d5495e215559e503e7ad6d3321bb3786184fc22b Mon Sep 17 00:00:00 2001 From: Joshua Nelson <jyn514@gmail.com> Date: Sun, 5 Jul 2020 22:45:44 -0400 Subject: [PATCH 20/37] Refactor `ItemLink` into its own struct --- src/librustdoc/clean/types.rs | 18 +++++++++++++++--- .../passes/collect_intra_doc_links.rs | 4 ++-- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index a458cdab30303..949790cfe26e1 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -425,10 +425,22 @@ pub struct Attributes { pub cfg: Option<Arc<Cfg>>, pub span: Option<rustc_span::Span>, /// map from Rust paths to resolved defs and potential URL fragments - pub links: Vec<(String, Option<DefId>, Option<String>)>, + pub links: Vec<ItemLink>, pub inner_docs: bool, } +#[derive(Clone, Debug, Default, PartialEq, Eq, Hash)] +/// A link that has not yet been rendered. +/// +/// This link will be turned into a rendered link by [`Attributes::links`] +pub struct ItemLink { + /// The original link written in the markdown + pub(crate) link: String, + pub(crate) did: Option<DefId>, + /// The url fragment to append to the link + pub(crate) fragment: Option<String>, +} + impl Attributes { /// Extracts the content from an attribute `#[doc(cfg(content))]`. pub fn extract_cfg(mi: &ast::MetaItem) -> Option<&ast::MetaItem> { @@ -611,8 +623,8 @@ impl Attributes { self.links .iter() - .filter_map(|&(ref s, did, ref fragment)| { - match did { + .filter_map(|ItemLink { link: s, did, fragment }| { + match *did { Some(did) => { if let Some((mut href, ..)) = href(did) { if let Some(ref fragment) = *fragment { diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index b6707a55959c7..a0cffc92ce195 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -904,7 +904,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { if let Res::PrimTy(_) = res { match disambiguator { Some(Disambiguator::Primitive | Disambiguator::Namespace(_)) | None => { - item.attrs.links.push((ori_link, None, fragment)) + item.attrs.links.push(ItemLink { link: ori_link, did: None, fragment }); } Some(other) => { report_mismatch(other, Disambiguator::Primitive); @@ -955,7 +955,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { } } let id = register_res(cx, res); - item.attrs.links.push((ori_link, Some(id), fragment)); + item.attrs.links.push(ItemLink { link: ori_link, did: Some(id), fragment }); } } From 31a7b6e8323b6e9880f4691e624cf6aa6289f6a7 Mon Sep 17 00:00:00 2001 From: Joshua Nelson <jyn514@gmail.com> Date: Sun, 5 Jul 2020 23:05:18 -0400 Subject: [PATCH 21/37] Refactor RenderedLink into its own type --- src/librustdoc/clean/types.rs | 21 ++++++++++++++------- src/librustdoc/html/markdown.rs | 21 +++++++++++---------- src/librustdoc/html/render/mod.rs | 4 ++-- 3 files changed, 27 insertions(+), 19 deletions(-) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 949790cfe26e1..d79553a8be8de 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -118,7 +118,7 @@ impl Item { self.attrs.collapsed_doc_value() } - pub fn links(&self) -> Vec<(String, String)> { + pub fn links(&self) -> Vec<RenderedLink> { self.attrs.links(&self.def_id.krate) } @@ -441,6 +441,13 @@ pub struct ItemLink { pub(crate) fragment: Option<String>, } +pub struct RenderedLink { + /// The text the link was original written as + pub(crate) original_text: String, + /// The URL to put in the `href` + pub(crate) href: String, +} + impl Attributes { /// Extracts the content from an attribute `#[doc(cfg(content))]`. pub fn extract_cfg(mi: &ast::MetaItem) -> Option<&ast::MetaItem> { @@ -617,7 +624,7 @@ impl Attributes { /// Gets links as a vector /// /// Cache must be populated before call - pub fn links(&self, krate: &CrateNum) -> Vec<(String, String)> { + pub fn links(&self, krate: &CrateNum) -> Vec<RenderedLink> { use crate::html::format::href; use crate::html::render::CURRENT_DEPTH; @@ -631,7 +638,7 @@ impl Attributes { href.push_str("#"); href.push_str(fragment); } - Some((s.clone(), href)) + Some(RenderedLink { original_text: s.clone(), href }) } else { None } @@ -651,16 +658,16 @@ impl Attributes { }; // This is a primitive so the url is done "by hand". let tail = fragment.find('#').unwrap_or_else(|| fragment.len()); - Some(( - s.clone(), - format!( + Some(RenderedLink { + original_text: s.clone(), + href: format!( "{}{}std/primitive.{}.html{}", url, if !url.ends_with('/') { "/" } else { "" }, &fragment[..tail], &fragment[tail..] ), - )) + }) } else { panic!("This isn't a primitive?!"); } diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index d54b8ea747899..e6484cd1c2c05 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -34,6 +34,7 @@ use std::fmt::Write; use std::ops::Range; use std::str; +use crate::clean::RenderedLink; use crate::doctest; use crate::html::highlight; use crate::html::toc::TocBuilder; @@ -52,7 +53,7 @@ fn opts() -> Options { pub struct Markdown<'a>( pub &'a str, /// A list of link replacements. - pub &'a [(String, String)], + pub &'a [RenderedLink], /// The current list of used header IDs. pub &'a mut IdMap, /// Whether to allow the use of explicit error codes in doctest lang strings. @@ -78,7 +79,7 @@ pub struct MarkdownHtml<'a>( pub &'a Option<Playground>, ); /// A tuple struct like `Markdown` that renders only the first paragraph. -pub struct MarkdownSummaryLine<'a>(pub &'a str, pub &'a [(String, String)]); +pub struct MarkdownSummaryLine<'a>(pub &'a str, pub &'a [RenderedLink]); #[derive(Copy, Clone, PartialEq, Debug)] pub enum ErrorCodes { @@ -339,11 +340,11 @@ impl<'a, I: Iterator<Item = Event<'a>>> Iterator for CodeBlocks<'_, 'a, I> { /// Make headings links with anchor IDs and build up TOC. struct LinkReplacer<'a, 'b, I: Iterator<Item = Event<'a>>> { inner: I, - links: &'b [(String, String)], + links: &'b [RenderedLink], } impl<'a, 'b, I: Iterator<Item = Event<'a>>> LinkReplacer<'a, 'b, I> { - fn new(iter: I, links: &'b [(String, String)]) -> Self { + fn new(iter: I, links: &'b [RenderedLink]) -> Self { LinkReplacer { inner: iter, links } } } @@ -354,8 +355,8 @@ impl<'a, 'b, I: Iterator<Item = Event<'a>>> Iterator for LinkReplacer<'a, 'b, I> fn next(&mut self) -> Option<Self::Item> { let event = self.inner.next(); if let Some(Event::Start(Tag::Link(kind, dest, text))) = event { - if let Some(&(_, ref replace)) = self.links.iter().find(|link| link.0 == *dest) { - Some(Event::Start(Tag::Link(kind, replace.to_owned().into(), text))) + if let Some(link) = self.links.iter().find(|link| link.original_text == *dest) { + Some(Event::Start(Tag::Link(kind, link.href.clone().into(), text))) } else { Some(Event::Start(Tag::Link(kind, dest, text))) } @@ -855,8 +856,8 @@ impl Markdown<'_> { return String::new(); } let replacer = |_: &str, s: &str| { - if let Some(&(_, ref replace)) = links.iter().find(|link| &*link.0 == s) { - Some((replace.clone(), s.to_owned())) + if let Some(link) = links.iter().find(|link| &*link.original_text == s) { + Some((link.original_text.clone(), link.href.clone())) } else { None } @@ -933,8 +934,8 @@ impl MarkdownSummaryLine<'_> { } let replacer = |_: &str, s: &str| { - if let Some(&(_, ref replace)) = links.iter().find(|link| &*link.0 == s) { - Some((replace.clone(), s.to_owned())) + if let Some(rendered_link) = links.iter().find(|link| &*link.original_text == s) { + Some((rendered_link.original_text.clone(), rendered_link.href.clone())) } else { None } diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index e4aba8963c762..9dc85881482f5 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -63,7 +63,7 @@ use rustc_span::symbol::{sym, Symbol}; use serde::ser::SerializeSeq; use serde::{Serialize, Serializer}; -use crate::clean::{self, AttributesExt, Deprecation, GetDefId, SelfTy, TypeKind}; +use crate::clean::{self, AttributesExt, Deprecation, GetDefId, RenderedLink, SelfTy, TypeKind}; use crate::config::RenderInfo; use crate::config::RenderOptions; use crate::docfs::{DocFS, PathError}; @@ -1780,7 +1780,7 @@ fn render_markdown( w: &mut Buffer, cx: &Context, md_text: &str, - links: Vec<(String, String)>, + links: Vec<RenderedLink>, prefix: &str, is_hidden: bool, ) { From 9815010d8f6a46621ee96cb824f9939b9db0ae47 Mon Sep 17 00:00:00 2001 From: Joshua Nelson <jyn514@gmail.com> Date: Sun, 5 Jul 2020 23:38:31 -0400 Subject: [PATCH 22/37] Remove disambiguators from link text Related to https://github.com/rust-lang/rust/issues/65354 - Pass through the replacement text to `markdown.rs` - Add some tests - Add a state machine that actually replaces the text when parsing Markdown --- src/librustdoc/clean/types.rs | 16 +++- src/librustdoc/html/markdown.rs | 89 +++++++++++++++---- src/librustdoc/html/render/mod.rs | 3 +- .../passes/collect_intra_doc_links.rs | 22 ++++- src/test/rustdoc/disambiguator_removed.rs | 33 +++++++ 5 files changed, 141 insertions(+), 22 deletions(-) create mode 100644 src/test/rustdoc/disambiguator_removed.rs diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index d79553a8be8de..05c90a7c4034f 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -436,6 +436,11 @@ pub struct Attributes { pub struct ItemLink { /// The original link written in the markdown pub(crate) link: String, + /// The link text displayed in the HTML. + /// + /// This may not be the same as `link` if there was a disambiguator + /// in an intra-doc link (e.g. [`fn@f`]) + pub(crate) link_text: String, pub(crate) did: Option<DefId>, /// The url fragment to append to the link pub(crate) fragment: Option<String>, @@ -444,6 +449,8 @@ pub struct ItemLink { pub struct RenderedLink { /// The text the link was original written as pub(crate) original_text: String, + /// The text to display in the HTML + pub(crate) new_text: String, /// The URL to put in the `href` pub(crate) href: String, } @@ -630,7 +637,7 @@ impl Attributes { self.links .iter() - .filter_map(|ItemLink { link: s, did, fragment }| { + .filter_map(|ItemLink { link: s, link_text, did, fragment }| { match *did { Some(did) => { if let Some((mut href, ..)) = href(did) { @@ -638,7 +645,11 @@ impl Attributes { href.push_str("#"); href.push_str(fragment); } - Some(RenderedLink { original_text: s.clone(), href }) + Some(RenderedLink { + original_text: s.clone(), + new_text: link_text.clone(), + href, + }) } else { None } @@ -660,6 +671,7 @@ impl Attributes { let tail = fragment.find('#').unwrap_or_else(|| fragment.len()); Some(RenderedLink { original_text: s.clone(), + new_text: link_text.clone(), href: format!( "{}{}std/primitive.{}.html{}", url, diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index e6484cd1c2c05..6d634bac7622d 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -340,29 +340,86 @@ impl<'a, I: Iterator<Item = Event<'a>>> Iterator for CodeBlocks<'_, 'a, I> { /// Make headings links with anchor IDs and build up TOC. struct LinkReplacer<'a, 'b, I: Iterator<Item = Event<'a>>> { inner: I, - links: &'b [RenderedLink], + links: &'a [RenderedLink], + shortcut_link: Option<&'b RenderedLink>, } -impl<'a, 'b, I: Iterator<Item = Event<'a>>> LinkReplacer<'a, 'b, I> { - fn new(iter: I, links: &'b [RenderedLink]) -> Self { - LinkReplacer { inner: iter, links } +impl<'a, I: Iterator<Item = Event<'a>>> LinkReplacer<'a, '_, I> { + fn new(iter: I, links: &'a [RenderedLink]) -> Self { + LinkReplacer { inner: iter, links, shortcut_link: None } } } -impl<'a, 'b, I: Iterator<Item = Event<'a>>> Iterator for LinkReplacer<'a, 'b, I> { +impl<'a: 'b, 'b, I: Iterator<Item = Event<'a>>> Iterator for LinkReplacer<'a, 'b, I> { type Item = Event<'a>; fn next(&mut self) -> Option<Self::Item> { - let event = self.inner.next(); - if let Some(Event::Start(Tag::Link(kind, dest, text))) = event { - if let Some(link) = self.links.iter().find(|link| link.original_text == *dest) { - Some(Event::Start(Tag::Link(kind, link.href.clone().into(), text))) - } else { - Some(Event::Start(Tag::Link(kind, dest, text))) + let mut event = self.inner.next(); + + // Remove disambiguators from shortcut links (`[fn@f]`) + match &mut event { + Some(Event::Start(Tag::Link( + pulldown_cmark::LinkType::ShortcutUnknown, + dest, + title, + ))) => { + debug!("saw start of shortcut link to {} with title {}", dest, title); + let link = if let Some(link) = + self.links.iter().find(|&link| *link.original_text == **dest) + { + // Not sure why this is necessary - maybe the broken_link_callback doesn't always work? + *dest = CowStr::Borrowed(link.href.as_ref()); + Some(link) + } else { + self.links.iter().find(|&link| *link.href == **dest) + }; + if let Some(link) = link { + trace!("it matched"); + assert!(self.shortcut_link.is_none(), "shortcut links cannot be nested"); + self.shortcut_link = Some(link); + } } - } else { - event + Some(Event::End(Tag::Link(pulldown_cmark::LinkType::ShortcutUnknown, dest, _))) => { + debug!("saw end of shortcut link to {}", dest); + if let Some(_link) = self.links.iter().find(|&link| *link.href == **dest) { + assert!(self.shortcut_link.is_some(), "saw closing link without opening tag"); + self.shortcut_link = None; + } + } + // Handle backticks in inline code blocks + Some(Event::Code(text)) => { + trace!("saw code {}", text); + if let Some(link) = self.shortcut_link { + trace!("original text was {}", link.original_text); + if **text == link.original_text[1..link.original_text.len() - 1] { + debug!("replacing {} with {}", text, link.new_text); + *text = link.new_text.clone().into(); + } + } + } + // Replace plain text in links + Some(Event::Text(text)) => { + trace!("saw text {}", text); + if let Some(link) = self.shortcut_link { + trace!("original text was {}", link.original_text); + if **text == *link.original_text { + debug!("replacing {} with {}", text, link.new_text); + *text = link.new_text.clone().into(); + } + } + } + Some(Event::Start(Tag::Link(_, dest, _))) => { + if let Some(link) = self.links.iter().find(|&link| *link.original_text == **dest) { + // Not sure why this is necessary - maybe the broken_link_callback doesn't always work? + *dest = CowStr::Borrowed(link.href.as_ref()); + } + } + // Anything else couldn't have been a valid Rust path + _ => {} } + + // Yield the modified event + event } } @@ -857,7 +914,7 @@ impl Markdown<'_> { } let replacer = |_: &str, s: &str| { if let Some(link) = links.iter().find(|link| &*link.original_text == s) { - Some((link.original_text.clone(), link.href.clone())) + Some((link.href.clone(), link.new_text.clone())) } else { None } @@ -934,8 +991,8 @@ impl MarkdownSummaryLine<'_> { } let replacer = |_: &str, s: &str| { - if let Some(rendered_link) = links.iter().find(|link| &*link.original_text == s) { - Some((rendered_link.original_text.clone(), rendered_link.href.clone())) + if let Some(link) = links.iter().find(|link| &*link.original_text == s) { + Some((link.href.clone(), link.new_text.clone())) } else { None } diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index 9dc85881482f5..318b10e2af2b3 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -64,8 +64,7 @@ use serde::ser::SerializeSeq; use serde::{Serialize, Serializer}; use crate::clean::{self, AttributesExt, Deprecation, GetDefId, RenderedLink, SelfTy, TypeKind}; -use crate::config::RenderInfo; -use crate::config::RenderOptions; +use crate::config::{RenderInfo, RenderOptions}; use crate::docfs::{DocFS, PathError}; use crate::doctree; use crate::error::Error; diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index a0cffc92ce195..ff3c19da3cf75 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -685,6 +685,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { continue; } + //let had_backticks = ori_link.contains("`"); let link = ori_link.replace("`", ""); let parts = link.split('#').collect::<Vec<_>>(); let (link, extra_fragment) = if parts.len() > 2 { @@ -700,6 +701,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { (parts[0], None) }; let resolved_self; + let link_text; let mut path_str; let disambiguator; let (mut res, mut fragment) = { @@ -716,6 +718,12 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { continue; } + // We stripped ` characters for `path_str`. + // The original link might have had multiple pairs of backticks, + // but we don't handle this case. + //link_text = if had_backticks { format!("`{}`", path_str) } else { path_str.to_owned() }; + link_text = path_str.to_owned(); + // In order to correctly resolve intra-doc-links we need to // pick a base AST node to work from. If the documentation for // this module came from an inner comment (//!) then we anchor @@ -904,7 +912,12 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { if let Res::PrimTy(_) = res { match disambiguator { Some(Disambiguator::Primitive | Disambiguator::Namespace(_)) | None => { - item.attrs.links.push(ItemLink { link: ori_link, did: None, fragment }); + item.attrs.links.push(ItemLink { + link: ori_link, + link_text: path_str.to_owned(), + did: None, + fragment, + }); } Some(other) => { report_mismatch(other, Disambiguator::Primitive); @@ -955,7 +968,12 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { } } let id = register_res(cx, res); - item.attrs.links.push(ItemLink { link: ori_link, did: Some(id), fragment }); + item.attrs.links.push(ItemLink { + link: ori_link, + link_text, + did: Some(id), + fragment, + }); } } diff --git a/src/test/rustdoc/disambiguator_removed.rs b/src/test/rustdoc/disambiguator_removed.rs new file mode 100644 index 0000000000000..74411870e9f8a --- /dev/null +++ b/src/test/rustdoc/disambiguator_removed.rs @@ -0,0 +1,33 @@ +#![deny(intra_doc_link_resolution_failure)] +// first try backticks +/// Trait: [`trait@Name`], fn: [`fn@Name`], [`Name`][`macro@Name`] +// @has disambiguator_removed/struct.AtDisambiguator.html +// @has - '//a[@href="../disambiguator_removed/trait.Name.html"][code]' "Name" +// @has - '//a[@href="../disambiguator_removed/fn.Name.html"][code]' "Name" +// @has - '//a[@href="../disambiguator_removed/macro.Name.html"][code]' "Name" +pub struct AtDisambiguator; + +/// fn: [`Name()`], macro: [`Name!`] +// @has disambiguator_removed/struct.SymbolDisambiguator.html +// @has - '//a[@href="../disambiguator_removed/fn.Name.html"][code]' "Name()" +// @has - '//a[@href="../disambiguator_removed/macro.Name.html"][code]' "Name!" +pub struct SymbolDisambiguator; + +// Now make sure that backticks aren't added if they weren't already there +/// [fn@Name] +// @has disambiguator_removed/trait.Name.html +// @has - '//a[@href="../disambiguator_removed/fn.Name.html"]' "Name" +// @!has - '//a[@href="../disambiguator_removed/fn.Name.html"][code]' "Name" + +// FIXME: this will turn !() into ! alone +/// [Name!()] +// @has - '//a[@href="../disambiguator_removed/macro.Name.html"]' "Name!" +pub trait Name {} + +#[allow(non_snake_case)] +pub fn Name() {} + +#[macro_export] +macro_rules! Name { + () => () +} From 9d7e797514ed1ea60a761d272c1ba8426bc31739 Mon Sep 17 00:00:00 2001 From: Joshua Nelson <jyn514@gmail.com> Date: Sat, 29 Aug 2020 15:13:28 -0400 Subject: [PATCH 23/37] display_for -> suggestion_for --- src/librustdoc/passes/collect_intra_doc_links.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index ff3c19da3cf75..feadf82a2b57e 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -685,7 +685,6 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { continue; } - //let had_backticks = ori_link.contains("`"); let link = ori_link.replace("`", ""); let parts = link.split('#').collect::<Vec<_>>(); let (link, extra_fragment) = if parts.len() > 2 { @@ -1053,7 +1052,7 @@ impl Disambiguator { } /// Return (description of the change, suggestion) - fn display_for(self, path_str: &str) -> (&'static str, String) { + fn suggestion_for(self, path_str: &str) -> (&'static str, String) { const PREFIX: &str = "prefix with the item kind"; const FUNCTION: &str = "add parentheses"; const MACRO: &str = "add an exclamation mark"; @@ -1308,7 +1307,7 @@ fn suggest_disambiguator( sp: Option<rustc_span::Span>, link_range: &Option<Range<usize>>, ) { - let (action, mut suggestion) = disambiguator.display_for(path_str); + let (action, mut suggestion) = disambiguator.suggestion_for(path_str); let help = format!("to link to the {}, {}", disambiguator.descr(), action); if let Some(sp) = sp { From 4df64905ea74f9732cb1448c14f72b17d3f32abc Mon Sep 17 00:00:00 2001 From: Ivan Tham <pickfire@riseup.net> Date: Thu, 3 Sep 2020 23:02:27 +0800 Subject: [PATCH 24/37] Link & primitive using relative link --- library/alloc/src/vec.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index 35d64d85010c3..43a0b048b47cc 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -287,6 +287,7 @@ use crate::raw_vec::RawVec; /// [`insert`]: Vec::insert /// [`reserve`]: Vec::reserve /// [owned slice]: Box +/// [`&`]: ../../std/primitive.reference.html #[stable(feature = "rust1", since = "1.0.0")] #[cfg_attr(not(test), rustc_diagnostic_item = "vec_type")] pub struct Vec<T> { From af133382f20b2e506b9e14b93e714fb76c7f6902 Mon Sep 17 00:00:00 2001 From: Mark Rousskov <mark.simulacrum@gmail.com> Date: Wed, 2 Sep 2020 16:41:36 -0400 Subject: [PATCH 25/37] Check test/example/benchmark on x.py check Often when modifying compiler code you'll miss that you've changed an API used by unit tests, since x.py check didn't previously catch that. It's also useful to have this for editing with rust-analyzer and similar tooling where editing tests previously didn't notify you of errors in test files. --- src/bootstrap/builder.rs | 2 +- src/bootstrap/check.rs | 61 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index cecc9ef75ea86..4708b207156c9 100644 --- a/src/bootstrap/builder.rs +++ b/src/bootstrap/builder.rs @@ -382,7 +382,7 @@ impl<'a> Builder<'a> { native::Lld ), Kind::Check | Kind::Clippy | Kind::Fix | Kind::Format => { - describe!(check::Std, check::Rustc, check::Rustdoc, check::Clippy) + describe!(check::Std, check::Rustc, check::Rustdoc, check::Clippy, check::Bootstrap) } Kind::Test => describe!( crate::toolstate::ToolStateCheck, diff --git a/src/bootstrap/check.rs b/src/bootstrap/check.rs index 1e26e7754aa21..ead0bd0413b9c 100644 --- a/src/bootstrap/check.rs +++ b/src/bootstrap/check.rs @@ -66,6 +66,43 @@ impl Step for Std { let libdir = builder.sysroot_libdir(compiler, target); let hostdir = builder.sysroot_libdir(compiler, compiler.host); add_to_sysroot(&builder, &libdir, &hostdir, &libstd_stamp(builder, compiler, target)); + + // Then run cargo again, once we've put the rmeta files for the library + // crates into the sysroot. This is needed because e.g., core's tests + // depend on `libtest` -- Cargo presumes it will exist, but it doesn't + // since we initialize with an empty sysroot. + // + // Currently only the "libtest" tree of crates does this. + + let mut cargo = builder.cargo( + compiler, + Mode::Std, + SourceType::InTree, + target, + cargo_subcommand(builder.kind), + ); + std_cargo(builder, target, compiler.stage, &mut cargo); + cargo.arg("--all-targets"); + + // Explicitly pass -p for all dependencies krates -- this will force cargo + // to also check the tests/benches/examples for these crates, rather + // than just the leaf crate. + for krate in builder.in_tree_crates("test") { + cargo.arg("-p").arg(krate.name); + } + + builder.info(&format!( + "Checking std test/bench/example targets ({} -> {})", + &compiler.host, target + )); + run_cargo( + builder, + cargo, + args(builder.kind), + &libstd_test_stamp(builder, compiler, target), + vec![], + true, + ); } } @@ -106,6 +143,14 @@ impl Step for Rustc { cargo_subcommand(builder.kind), ); rustc_cargo(builder, &mut cargo, target); + cargo.arg("--all-targets"); + + // Explicitly pass -p for all compiler krates -- this will force cargo + // to also check the tests/benches/examples for these crates, rather + // than just the leaf crate. + for krate in builder.in_tree_crates("rustc-main") { + cargo.arg("-p").arg(krate.name); + } builder.info(&format!("Checking compiler artifacts ({} -> {})", &compiler.host, target)); run_cargo( @@ -149,7 +194,7 @@ macro_rules! tool_check_step { builder.ensure(Rustc { target }); - let cargo = prepare_tool_cargo( + let mut cargo = prepare_tool_cargo( builder, compiler, Mode::ToolRustc, @@ -160,6 +205,8 @@ macro_rules! tool_check_step { &[], ); + cargo.arg("--all-targets"); + builder.info(&format!( "Checking {} artifacts ({} -> {})", stringify!($name).to_lowercase(), @@ -202,12 +249,24 @@ tool_check_step!(Rustdoc, "src/tools/rustdoc", SourceType::InTree); // rejected. tool_check_step!(Clippy, "src/tools/clippy", SourceType::InTree); +tool_check_step!(Bootstrap, "src/bootstrap", SourceType::InTree); + /// Cargo's output path for the standard library in a given stage, compiled /// by a particular compiler for the specified target. fn libstd_stamp(builder: &Builder<'_>, compiler: Compiler, target: TargetSelection) -> PathBuf { builder.cargo_out(compiler, Mode::Std, target).join(".libstd-check.stamp") } +/// Cargo's output path for the standard library in a given stage, compiled +/// by a particular compiler for the specified target. +fn libstd_test_stamp( + builder: &Builder<'_>, + compiler: Compiler, + target: TargetSelection, +) -> PathBuf { + builder.cargo_out(compiler, Mode::Std, target).join(".libstd-check-test.stamp") +} + /// Cargo's output path for librustc in a given stage, compiled by a particular /// compiler for the specified target. fn librustc_stamp(builder: &Builder<'_>, compiler: Compiler, target: TargetSelection) -> PathBuf { From cbc396fdfb411b4963131889aeae5c675f4512a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= <tomasz.miasko@gmail.com> Date: Thu, 3 Sep 2020 00:00:00 +0000 Subject: [PATCH 26/37] inliner: Check for target features compatibility --- compiler/rustc_mir/src/transform/inline.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_mir/src/transform/inline.rs b/compiler/rustc_mir/src/transform/inline.rs index 428f4e138c7f9..0fe0169e2c88c 100644 --- a/compiler/rustc_mir/src/transform/inline.rs +++ b/compiler/rustc_mir/src/transform/inline.rs @@ -4,7 +4,7 @@ use rustc_attr as attr; use rustc_hir::def_id::DefId; use rustc_index::bit_set::BitSet; use rustc_index::vec::{Idx, IndexVec}; -use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; +use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs}; use rustc_middle::mir::visit::*; use rustc_middle::mir::*; use rustc_middle::ty::subst::{Subst, SubstsRef}; @@ -45,7 +45,8 @@ impl<'tcx> MirPass<'tcx> for Inline { // based function. debug!("function inlining is disabled when compiling with `instrument_coverage`"); } else { - Inliner { tcx, source }.run_pass(body); + Inliner { tcx, source, codegen_fn_attrs: tcx.codegen_fn_attrs(source.def_id()) } + .run_pass(body); } } } @@ -54,6 +55,7 @@ impl<'tcx> MirPass<'tcx> for Inline { struct Inliner<'tcx> { tcx: TyCtxt<'tcx>, source: MirSource<'tcx>, + codegen_fn_attrs: &'tcx CodegenFnAttrs, } impl Inliner<'tcx> { @@ -242,6 +244,13 @@ impl Inliner<'tcx> { return false; } + let self_features = &self.codegen_fn_attrs.target_features; + let callee_features = &codegen_fn_attrs.target_features; + if callee_features.iter().any(|feature| !self_features.contains(feature)) { + debug!("`callee has extra target features - not inlining"); + return false; + } + // Avoid inlining functions marked as no_sanitize if sanitizer is enabled, // since instrumentation might be enabled and performed on the caller. if self.tcx.sess.opts.debugging_opts.sanitizer.intersects(codegen_fn_attrs.no_sanitize) { From 326b772609c4cfbd09026c534be4cbfda36fbcf4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= <tomasz.miasko@gmail.com> Date: Thu, 3 Sep 2020 00:00:00 +0000 Subject: [PATCH 27/37] inliner: Check for no_sanitize attribute compatibility --- compiler/rustc_mir/src/transform/inline.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_mir/src/transform/inline.rs b/compiler/rustc_mir/src/transform/inline.rs index 0fe0169e2c88c..7ca454430ccb9 100644 --- a/compiler/rustc_mir/src/transform/inline.rs +++ b/compiler/rustc_mir/src/transform/inline.rs @@ -251,9 +251,12 @@ impl Inliner<'tcx> { return false; } - // Avoid inlining functions marked as no_sanitize if sanitizer is enabled, - // since instrumentation might be enabled and performed on the caller. - if self.tcx.sess.opts.debugging_opts.sanitizer.intersects(codegen_fn_attrs.no_sanitize) { + let self_no_sanitize = + self.codegen_fn_attrs.no_sanitize & self.tcx.sess.opts.debugging_opts.sanitizer; + let callee_no_sanitize = + codegen_fn_attrs.no_sanitize & self.tcx.sess.opts.debugging_opts.sanitizer; + if self_no_sanitize != callee_no_sanitize { + debug!("`callee has incompatible no_sanitize attribute - not inlining"); return false; } From c23151bdb1b242d96d545cfe4c5f3314b6ef448c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= <tomasz.miasko@gmail.com> Date: Fri, 4 Sep 2020 00:00:00 +0000 Subject: [PATCH 28/37] inliner: Add mir-opt tests for codegen attributes compatibility --- .../mir-opt/inline/inline-compatibility.rs | 39 +++++++++++++++++++ ...patibility.inlined_no_sanitize.Inline.diff | 25 ++++++++++++ ...ibility.inlined_target_feature.Inline.diff | 25 ++++++++++++ ...bility.not_inlined_no_sanitize.Inline.diff | 22 +++++++++++ ...ity.not_inlined_target_feature.Inline.diff | 22 +++++++++++ 5 files changed, 133 insertions(+) create mode 100644 src/test/mir-opt/inline/inline-compatibility.rs create mode 100644 src/test/mir-opt/inline/inline_compatibility.inlined_no_sanitize.Inline.diff create mode 100644 src/test/mir-opt/inline/inline_compatibility.inlined_target_feature.Inline.diff create mode 100644 src/test/mir-opt/inline/inline_compatibility.not_inlined_no_sanitize.Inline.diff create mode 100644 src/test/mir-opt/inline/inline_compatibility.not_inlined_target_feature.Inline.diff diff --git a/src/test/mir-opt/inline/inline-compatibility.rs b/src/test/mir-opt/inline/inline-compatibility.rs new file mode 100644 index 0000000000000..ff9049edb4f2c --- /dev/null +++ b/src/test/mir-opt/inline/inline-compatibility.rs @@ -0,0 +1,39 @@ +// Checks that only functions with compatible attributes are inlined. +// +// only-x86_64 +// needs-sanitizer-address +// compile-flags: -Zsanitizer=address + +#![crate_type = "lib"] +#![feature(no_sanitize)] +#![feature(target_feature_11)] + +// EMIT_MIR inline_compatibility.inlined_target_feature.Inline.diff +#[target_feature(enable = "sse2")] +pub unsafe fn inlined_target_feature() { + target_feature(); +} + +// EMIT_MIR inline_compatibility.not_inlined_target_feature.Inline.diff +pub unsafe fn not_inlined_target_feature() { + target_feature(); +} + +// EMIT_MIR inline_compatibility.inlined_no_sanitize.Inline.diff +#[no_sanitize(address)] +pub unsafe fn inlined_no_sanitize() { + no_sanitize(); +} + +// EMIT_MIR inline_compatibility.not_inlined_no_sanitize.Inline.diff +pub unsafe fn not_inlined_no_sanitize() { + no_sanitize(); +} + +#[inline] +#[target_feature(enable = "sse2")] +pub unsafe fn target_feature() {} + +#[inline] +#[no_sanitize(address, memory)] +pub unsafe fn no_sanitize() {} diff --git a/src/test/mir-opt/inline/inline_compatibility.inlined_no_sanitize.Inline.diff b/src/test/mir-opt/inline/inline_compatibility.inlined_no_sanitize.Inline.diff new file mode 100644 index 0000000000000..7b0ecaffdd7cb --- /dev/null +++ b/src/test/mir-opt/inline/inline_compatibility.inlined_no_sanitize.Inline.diff @@ -0,0 +1,25 @@ +- // MIR for `inlined_no_sanitize` before Inline ++ // MIR for `inlined_no_sanitize` after Inline + + fn inlined_no_sanitize() -> () { + let mut _0: (); // return place in scope 0 at $DIR/inline-compatibility.rs:24:37: 24:37 + let _1: (); // in scope 0 at $DIR/inline-compatibility.rs:25:5: 25:18 ++ scope 1 { ++ } + + bb0: { + StorageLive(_1); // scope 0 at $DIR/inline-compatibility.rs:25:5: 25:18 +- _1 = no_sanitize() -> bb1; // scope 0 at $DIR/inline-compatibility.rs:25:5: 25:18 +- // mir::Constant +- // + span: $DIR/inline-compatibility.rs:25:5: 25:16 +- // + literal: Const { ty: unsafe fn() {no_sanitize}, val: Value(Scalar(<ZST>)) } +- } +- +- bb1: { ++ _1 = const (); // scope 1 at $DIR/inline-compatibility.rs:39:29: 39:31 + StorageDead(_1); // scope 0 at $DIR/inline-compatibility.rs:25:18: 25:19 + _0 = const (); // scope 0 at $DIR/inline-compatibility.rs:24:37: 26:2 + return; // scope 0 at $DIR/inline-compatibility.rs:26:2: 26:2 + } + } + diff --git a/src/test/mir-opt/inline/inline_compatibility.inlined_target_feature.Inline.diff b/src/test/mir-opt/inline/inline_compatibility.inlined_target_feature.Inline.diff new file mode 100644 index 0000000000000..f55eae6c50a5f --- /dev/null +++ b/src/test/mir-opt/inline/inline_compatibility.inlined_target_feature.Inline.diff @@ -0,0 +1,25 @@ +- // MIR for `inlined_target_feature` before Inline ++ // MIR for `inlined_target_feature` after Inline + + fn inlined_target_feature() -> () { + let mut _0: (); // return place in scope 0 at $DIR/inline-compatibility.rs:13:40: 13:40 + let _1: (); // in scope 0 at $DIR/inline-compatibility.rs:14:5: 14:21 ++ scope 1 { ++ } + + bb0: { + StorageLive(_1); // scope 0 at $DIR/inline-compatibility.rs:14:5: 14:21 +- _1 = target_feature() -> bb1; // scope 0 at $DIR/inline-compatibility.rs:14:5: 14:21 +- // mir::Constant +- // + span: $DIR/inline-compatibility.rs:14:5: 14:19 +- // + literal: Const { ty: unsafe fn() {target_feature}, val: Value(Scalar(<ZST>)) } +- } +- +- bb1: { ++ _1 = const (); // scope 1 at $DIR/inline-compatibility.rs:35:32: 35:34 + StorageDead(_1); // scope 0 at $DIR/inline-compatibility.rs:14:21: 14:22 + _0 = const (); // scope 0 at $DIR/inline-compatibility.rs:13:40: 15:2 + return; // scope 0 at $DIR/inline-compatibility.rs:15:2: 15:2 + } + } + diff --git a/src/test/mir-opt/inline/inline_compatibility.not_inlined_no_sanitize.Inline.diff b/src/test/mir-opt/inline/inline_compatibility.not_inlined_no_sanitize.Inline.diff new file mode 100644 index 0000000000000..651eadc1e849c --- /dev/null +++ b/src/test/mir-opt/inline/inline_compatibility.not_inlined_no_sanitize.Inline.diff @@ -0,0 +1,22 @@ +- // MIR for `not_inlined_no_sanitize` before Inline ++ // MIR for `not_inlined_no_sanitize` after Inline + + fn not_inlined_no_sanitize() -> () { + let mut _0: (); // return place in scope 0 at $DIR/inline-compatibility.rs:29:41: 29:41 + let _1: (); // in scope 0 at $DIR/inline-compatibility.rs:30:5: 30:18 + + bb0: { + StorageLive(_1); // scope 0 at $DIR/inline-compatibility.rs:30:5: 30:18 + _1 = no_sanitize() -> bb1; // scope 0 at $DIR/inline-compatibility.rs:30:5: 30:18 + // mir::Constant + // + span: $DIR/inline-compatibility.rs:30:5: 30:16 + // + literal: Const { ty: unsafe fn() {no_sanitize}, val: Value(Scalar(<ZST>)) } + } + + bb1: { + StorageDead(_1); // scope 0 at $DIR/inline-compatibility.rs:30:18: 30:19 + _0 = const (); // scope 0 at $DIR/inline-compatibility.rs:29:41: 31:2 + return; // scope 0 at $DIR/inline-compatibility.rs:31:2: 31:2 + } + } + diff --git a/src/test/mir-opt/inline/inline_compatibility.not_inlined_target_feature.Inline.diff b/src/test/mir-opt/inline/inline_compatibility.not_inlined_target_feature.Inline.diff new file mode 100644 index 0000000000000..55b9edf3adc1f --- /dev/null +++ b/src/test/mir-opt/inline/inline_compatibility.not_inlined_target_feature.Inline.diff @@ -0,0 +1,22 @@ +- // MIR for `not_inlined_target_feature` before Inline ++ // MIR for `not_inlined_target_feature` after Inline + + fn not_inlined_target_feature() -> () { + let mut _0: (); // return place in scope 0 at $DIR/inline-compatibility.rs:18:44: 18:44 + let _1: (); // in scope 0 at $DIR/inline-compatibility.rs:19:5: 19:21 + + bb0: { + StorageLive(_1); // scope 0 at $DIR/inline-compatibility.rs:19:5: 19:21 + _1 = target_feature() -> bb1; // scope 0 at $DIR/inline-compatibility.rs:19:5: 19:21 + // mir::Constant + // + span: $DIR/inline-compatibility.rs:19:5: 19:19 + // + literal: Const { ty: unsafe fn() {target_feature}, val: Value(Scalar(<ZST>)) } + } + + bb1: { + StorageDead(_1); // scope 0 at $DIR/inline-compatibility.rs:19:21: 19:22 + _0 = const (); // scope 0 at $DIR/inline-compatibility.rs:18:44: 20:2 + return; // scope 0 at $DIR/inline-compatibility.rs:20:2: 20:2 + } + } + From 85146b9db7bf5de79684f8595ad49c33e436d1ae Mon Sep 17 00:00:00 2001 From: Ivan Tham <pickfire@riseup.net> Date: Fri, 4 Sep 2020 09:50:50 +0800 Subject: [PATCH 29/37] Add slice primitive link to vec --- library/alloc/src/vec.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index 43a0b048b47cc..a132cf17dff4a 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -159,7 +159,7 @@ use crate::raw_vec::RawVec; /// # Slicing /// /// A `Vec` can be mutable. Slices, on the other hand, are read-only objects. -/// To get a slice, use [`&`]. Example: +/// To get a [slice], use [`&`]. Example: /// /// ``` /// fn read_slice(slice: &[usize]) { @@ -287,6 +287,7 @@ use crate::raw_vec::RawVec; /// [`insert`]: Vec::insert /// [`reserve`]: Vec::reserve /// [owned slice]: Box +/// [slice]: ../../std/primitive.slice.html /// [`&`]: ../../std/primitive.reference.html #[stable(feature = "rust1", since = "1.0.0")] #[cfg_attr(not(test), rustc_diagnostic_item = "vec_type")] From 5d41aff0bc188d34e12ab63769eddf832e1c6075 Mon Sep 17 00:00:00 2001 From: Ralf Jung <post@ralfj.de> Date: Tue, 1 Sep 2020 23:16:51 +0200 Subject: [PATCH 30/37] rename MaybeUninit slice methods first_ptr -> slice_as_ptr first_ptr_mut -> slice_as_mut_ptr slice_get_ref -> slice_assume_init_ref slice_get_mut -> slice_assume_init_mut --- library/alloc/src/collections/btree/node.rs | 22 +++-- library/core/src/array/iter.rs | 4 +- library/core/src/array/mod.rs | 2 +- library/core/src/fmt/num.rs | 11 ++- library/core/src/mem/maybe_uninit.rs | 25 ++++-- library/core/src/num/flt2dec/mod.rs | 88 ++++++++++++------- .../core/src/num/flt2dec/strategy/dragon.rs | 10 +-- .../core/src/num/flt2dec/strategy/grisu.rs | 14 +-- library/core/src/slice/sort.rs | 8 +- 9 files changed, 113 insertions(+), 71 deletions(-) diff --git a/library/alloc/src/collections/btree/node.rs b/library/alloc/src/collections/btree/node.rs index acc2ae73572ba..8a9acca89cff5 100644 --- a/library/alloc/src/collections/btree/node.rs +++ b/library/alloc/src/collections/btree/node.rs @@ -461,11 +461,15 @@ impl<'a, K, V, Type> NodeRef<marker::Mut<'a>, K, V, Type> { impl<'a, K: 'a, V: 'a, Type> NodeRef<marker::Immut<'a>, K, V, Type> { fn into_key_slice(self) -> &'a [K] { - unsafe { slice::from_raw_parts(MaybeUninit::first_ptr(&self.as_leaf().keys), self.len()) } + unsafe { + slice::from_raw_parts(MaybeUninit::slice_as_ptr(&self.as_leaf().keys), self.len()) + } } fn into_val_slice(self) -> &'a [V] { - unsafe { slice::from_raw_parts(MaybeUninit::first_ptr(&self.as_leaf().vals), self.len()) } + unsafe { + slice::from_raw_parts(MaybeUninit::slice_as_ptr(&self.as_leaf().vals), self.len()) + } } } @@ -480,7 +484,7 @@ impl<'a, K: 'a, V: 'a, Type> NodeRef<marker::Mut<'a>, K, V, Type> { // SAFETY: The keys of a node must always be initialized up to length. unsafe { slice::from_raw_parts_mut( - MaybeUninit::first_ptr_mut(&mut (*self.as_leaf_mut()).keys), + MaybeUninit::slice_as_mut_ptr(&mut (*self.as_leaf_mut()).keys), self.len(), ) } @@ -490,7 +494,7 @@ impl<'a, K: 'a, V: 'a, Type> NodeRef<marker::Mut<'a>, K, V, Type> { // SAFETY: The values of a node must always be initialized up to length. unsafe { slice::from_raw_parts_mut( - MaybeUninit::first_ptr_mut(&mut (*self.as_leaf_mut()).vals), + MaybeUninit::slice_as_mut_ptr(&mut (*self.as_leaf_mut()).vals), self.len(), ) } @@ -506,10 +510,10 @@ impl<'a, K: 'a, V: 'a, Type> NodeRef<marker::Mut<'a>, K, V, Type> { let leaf = self.as_leaf_mut(); // SAFETY: The keys and values of a node must always be initialized up to length. let keys = unsafe { - slice::from_raw_parts_mut(MaybeUninit::first_ptr_mut(&mut (*leaf).keys), len) + slice::from_raw_parts_mut(MaybeUninit::slice_as_mut_ptr(&mut (*leaf).keys), len) }; let vals = unsafe { - slice::from_raw_parts_mut(MaybeUninit::first_ptr_mut(&mut (*leaf).vals), len) + slice::from_raw_parts_mut(MaybeUninit::slice_as_mut_ptr(&mut (*leaf).vals), len) }; (keys, vals) } @@ -588,7 +592,7 @@ impl<'a, K, V> NodeRef<marker::Mut<'a>, K, V, marker::Internal> { slice_insert(self.vals_mut(), 0, val); slice_insert( slice::from_raw_parts_mut( - MaybeUninit::first_ptr_mut(&mut self.as_internal_mut().edges), + MaybeUninit::slice_as_mut_ptr(&mut self.as_internal_mut().edges), self.len() + 1, ), 0, @@ -646,7 +650,7 @@ impl<'a, K, V> NodeRef<marker::Mut<'a>, K, V, marker::LeafOrInternal> { ForceResult::Internal(mut internal) => { let edge = slice_remove( slice::from_raw_parts_mut( - MaybeUninit::first_ptr_mut(&mut internal.as_internal_mut().edges), + MaybeUninit::slice_as_mut_ptr(&mut internal.as_internal_mut().edges), old_len + 1, ), 0, @@ -933,7 +937,7 @@ impl<'a, K, V> Handle<NodeRef<marker::Mut<'a>, K, V, marker::Internal>, marker:: slice_insert( slice::from_raw_parts_mut( - MaybeUninit::first_ptr_mut(&mut self.node.as_internal_mut().edges), + MaybeUninit::slice_as_mut_ptr(&mut self.node.as_internal_mut().edges), self.node.len(), ), self.idx + 1, diff --git a/library/core/src/array/iter.rs b/library/core/src/array/iter.rs index 919070aadf972..2e8b6419eea1e 100644 --- a/library/core/src/array/iter.rs +++ b/library/core/src/array/iter.rs @@ -73,7 +73,7 @@ impl<T, const N: usize> IntoIter<T, N> { // SAFETY: We know that all elements within `alive` are properly initialized. unsafe { let slice = self.data.get_unchecked(self.alive.clone()); - MaybeUninit::slice_get_ref(slice) + MaybeUninit::slice_assume_init_ref(slice) } } @@ -82,7 +82,7 @@ impl<T, const N: usize> IntoIter<T, N> { // SAFETY: We know that all elements within `alive` are properly initialized. unsafe { let slice = self.data.get_unchecked_mut(self.alive.clone()); - MaybeUninit::slice_get_mut(slice) + MaybeUninit::slice_assume_init_mut(slice) } } } diff --git a/library/core/src/array/mod.rs b/library/core/src/array/mod.rs index f45c99c285c84..c1d3aca6fdd4f 100644 --- a/library/core/src/array/mod.rs +++ b/library/core/src/array/mod.rs @@ -410,7 +410,7 @@ impl<T, const N: usize> [T; N] { } let mut dst = MaybeUninit::uninit_array::<N>(); let mut guard: Guard<U, N> = - Guard { dst: MaybeUninit::first_ptr_mut(&mut dst), initialized: 0 }; + Guard { dst: MaybeUninit::slice_as_mut_ptr(&mut dst), initialized: 0 }; for (src, dst) in IntoIter::new(self).zip(&mut dst) { dst.write(f(src)); guard.initialized += 1; diff --git a/library/core/src/fmt/num.rs b/library/core/src/fmt/num.rs index 7d77e33d74378..ae3d0ddd46beb 100644 --- a/library/core/src/fmt/num.rs +++ b/library/core/src/fmt/num.rs @@ -85,7 +85,10 @@ trait GenericRadix { // SAFETY: The only chars in `buf` are created by `Self::digit` which are assumed to be // valid UTF-8 let buf = unsafe { - str::from_utf8_unchecked(slice::from_raw_parts(MaybeUninit::first_ptr(buf), buf.len())) + str::from_utf8_unchecked(slice::from_raw_parts( + MaybeUninit::slice_as_ptr(buf), + buf.len(), + )) }; f.pad_integral(is_nonnegative, Self::PREFIX, buf) } @@ -192,7 +195,7 @@ macro_rules! impl_Display { // 2^128 is about 3*10^38, so 39 gives an extra byte of space let mut buf = [MaybeUninit::<u8>::uninit(); 39]; let mut curr = buf.len() as isize; - let buf_ptr = MaybeUninit::first_ptr_mut(&mut buf); + let buf_ptr = MaybeUninit::slice_as_mut_ptr(&mut buf); let lut_ptr = DEC_DIGITS_LUT.as_ptr(); // SAFETY: Since `d1` and `d2` are always less than or equal to `198`, we @@ -322,7 +325,7 @@ macro_rules! impl_Exp { // that `curr >= 0`. let mut buf = [MaybeUninit::<u8>::uninit(); 40]; let mut curr = buf.len() as isize; //index for buf - let buf_ptr = MaybeUninit::first_ptr_mut(&mut buf); + let buf_ptr = MaybeUninit::slice_as_mut_ptr(&mut buf); let lut_ptr = DEC_DIGITS_LUT.as_ptr(); // decode 2 chars at a time @@ -370,7 +373,7 @@ macro_rules! impl_Exp { // stores 'e' (or 'E') and the up to 2-digit exponent let mut exp_buf = [MaybeUninit::<u8>::uninit(); 3]; - let exp_ptr = MaybeUninit::first_ptr_mut(&mut exp_buf); + let exp_ptr = MaybeUninit::slice_as_mut_ptr(&mut exp_buf); // SAFETY: In either case, `exp_buf` is written within bounds and `exp_ptr[..len]` // is contained within `exp_buf` since `len <= 3`. let exp_slice = unsafe { diff --git a/library/core/src/mem/maybe_uninit.rs b/library/core/src/mem/maybe_uninit.rs index a79d9e25f34fc..f770df61d9c54 100644 --- a/library/core/src/mem/maybe_uninit.rs +++ b/library/core/src/mem/maybe_uninit.rs @@ -281,7 +281,7 @@ impl<T> MaybeUninit<T> { /// # Examples /// /// ```no_run - /// #![feature(maybe_uninit_uninit_array, maybe_uninit_extra, maybe_uninit_slice_assume_init)] + /// #![feature(maybe_uninit_uninit_array, maybe_uninit_extra, maybe_uninit_slice)] /// /// use std::mem::MaybeUninit; /// @@ -293,7 +293,7 @@ impl<T> MaybeUninit<T> { /// fn read(buf: &mut [MaybeUninit<u8>]) -> &[u8] { /// unsafe { /// let len = read_into_buffer(buf.as_mut_ptr() as *mut u8, buf.len()); - /// MaybeUninit::slice_get_ref(&buf[..len]) + /// MaybeUninit::slice_assume_init_ref(&buf[..len]) /// } /// } /// @@ -303,6 +303,7 @@ impl<T> MaybeUninit<T> { #[unstable(feature = "maybe_uninit_uninit_array", issue = "none")] #[inline(always)] pub fn uninit_array<const LEN: usize>() -> [Self; LEN] { + // SAFETY: An uninitialized `[MaybeUninit<_>; LEN]` is valid. unsafe { MaybeUninit::<[MaybeUninit<T>; LEN]>::uninit().assume_init() } } @@ -769,9 +770,13 @@ impl<T> MaybeUninit<T> { /// It is up to the caller to guarantee that the `MaybeUninit<T>` elements /// really are in an initialized state. /// Calling this when the content is not yet fully initialized causes undefined behavior. - #[unstable(feature = "maybe_uninit_slice_assume_init", issue = "none")] + /// + /// See [`assume_init_ref`] for more details and examples. + /// + /// [`assume_init_ref`]: MaybeUninit::assume_init_ref + #[unstable(feature = "maybe_uninit_slice", issue = "63569")] #[inline(always)] - pub unsafe fn slice_get_ref(slice: &[Self]) -> &[T] { + pub unsafe fn slice_assume_init_ref(slice: &[Self]) -> &[T] { // SAFETY: casting slice to a `*const [T]` is safe since the caller guarantees that // `slice` is initialized, and`MaybeUninit` is guaranteed to have the same layout as `T`. // The pointer obtained is valid since it refers to memory owned by `slice` which is a @@ -786,9 +791,13 @@ impl<T> MaybeUninit<T> { /// It is up to the caller to guarantee that the `MaybeUninit<T>` elements /// really are in an initialized state. /// Calling this when the content is not yet fully initialized causes undefined behavior. - #[unstable(feature = "maybe_uninit_slice_assume_init", issue = "none")] + /// + /// See [`assume_init_mut`] for more details and examples. + /// + /// [`assume_init_mut`]: MaybeUninit::assume_init_mut + #[unstable(feature = "maybe_uninit_slice", issue = "63569")] #[inline(always)] - pub unsafe fn slice_get_mut(slice: &mut [Self]) -> &mut [T] { + pub unsafe fn slice_assume_init_mut(slice: &mut [Self]) -> &mut [T] { // SAFETY: similar to safety notes for `slice_get_ref`, but we have a // mutable reference which is also guaranteed to be valid for writes. unsafe { &mut *(slice as *mut [Self] as *mut [T]) } @@ -797,14 +806,14 @@ impl<T> MaybeUninit<T> { /// Gets a pointer to the first element of the array. #[unstable(feature = "maybe_uninit_slice", issue = "63569")] #[inline(always)] - pub fn first_ptr(this: &[MaybeUninit<T>]) -> *const T { + pub fn slice_as_ptr(this: &[MaybeUninit<T>]) -> *const T { this as *const [MaybeUninit<T>] as *const T } /// Gets a mutable pointer to the first element of the array. #[unstable(feature = "maybe_uninit_slice", issue = "63569")] #[inline(always)] - pub fn first_ptr_mut(this: &mut [MaybeUninit<T>]) -> *mut T { + pub fn slice_as_mut_ptr(this: &mut [MaybeUninit<T>]) -> *mut T { this as *mut [MaybeUninit<T>] as *mut T } } diff --git a/library/core/src/num/flt2dec/mod.rs b/library/core/src/num/flt2dec/mod.rs index 1f28edb412827..e8f9d6574e2d0 100644 --- a/library/core/src/num/flt2dec/mod.rs +++ b/library/core/src/num/flt2dec/mod.rs @@ -311,10 +311,10 @@ fn digits_to_dec_str<'a>( if frac_digits > buf.len() && frac_digits - buf.len() > minus_exp { parts[3] = MaybeUninit::new(Part::Zero((frac_digits - buf.len()) - minus_exp)); // SAFETY: we just initialized the elements `..4`. - unsafe { MaybeUninit::slice_get_ref(&parts[..4]) } + unsafe { MaybeUninit::slice_assume_init_ref(&parts[..4]) } } else { // SAFETY: we just initialized the elements `..3`. - unsafe { MaybeUninit::slice_get_ref(&parts[..3]) } + unsafe { MaybeUninit::slice_assume_init_ref(&parts[..3]) } } } else { let exp = exp as usize; @@ -326,10 +326,10 @@ fn digits_to_dec_str<'a>( if frac_digits > buf.len() - exp { parts[3] = MaybeUninit::new(Part::Zero(frac_digits - (buf.len() - exp))); // SAFETY: we just initialized the elements `..4`. - unsafe { MaybeUninit::slice_get_ref(&parts[..4]) } + unsafe { MaybeUninit::slice_assume_init_ref(&parts[..4]) } } else { // SAFETY: we just initialized the elements `..3`. - unsafe { MaybeUninit::slice_get_ref(&parts[..3]) } + unsafe { MaybeUninit::slice_assume_init_ref(&parts[..3]) } } } else { // the decimal point is after rendered digits: [1234][____0000] or [1234][__][.][__]. @@ -339,10 +339,10 @@ fn digits_to_dec_str<'a>( parts[2] = MaybeUninit::new(Part::Copy(b".")); parts[3] = MaybeUninit::new(Part::Zero(frac_digits)); // SAFETY: we just initialized the elements `..4`. - unsafe { MaybeUninit::slice_get_ref(&parts[..4]) } + unsafe { MaybeUninit::slice_assume_init_ref(&parts[..4]) } } else { // SAFETY: we just initialized the elements `..2`. - unsafe { MaybeUninit::slice_get_ref(&parts[..2]) } + unsafe { MaybeUninit::slice_assume_init_ref(&parts[..2]) } } } } @@ -393,7 +393,7 @@ fn digits_to_exp_str<'a>( parts[n + 1] = MaybeUninit::new(Part::Num(exp as u16)); } // SAFETY: we just initialized the elements `..n + 2`. - unsafe { MaybeUninit::slice_get_ref(&parts[..n + 2]) } + unsafe { MaybeUninit::slice_assume_init_ref(&parts[..n + 2]) } } /// Sign formatting options. @@ -487,24 +487,30 @@ where FullDecoded::Nan => { parts[0] = MaybeUninit::new(Part::Copy(b"NaN")); // SAFETY: we just initialized the elements `..1`. - Formatted { sign, parts: unsafe { MaybeUninit::slice_get_ref(&parts[..1]) } } + Formatted { sign, parts: unsafe { MaybeUninit::slice_assume_init_ref(&parts[..1]) } } } FullDecoded::Infinite => { parts[0] = MaybeUninit::new(Part::Copy(b"inf")); // SAFETY: we just initialized the elements `..1`. - Formatted { sign, parts: unsafe { MaybeUninit::slice_get_ref(&parts[..1]) } } + Formatted { sign, parts: unsafe { MaybeUninit::slice_assume_init_ref(&parts[..1]) } } } FullDecoded::Zero => { if frac_digits > 0 { // [0.][0000] parts[0] = MaybeUninit::new(Part::Copy(b"0.")); parts[1] = MaybeUninit::new(Part::Zero(frac_digits)); - // SAFETY: we just initialized the elements `..2`. - Formatted { sign, parts: unsafe { MaybeUninit::slice_get_ref(&parts[..2]) } } + Formatted { + sign, + // SAFETY: we just initialized the elements `..2`. + parts: unsafe { MaybeUninit::slice_assume_init_ref(&parts[..2]) }, + } } else { parts[0] = MaybeUninit::new(Part::Copy(b"0")); - // SAFETY: we just initialized the elements `..1`. - Formatted { sign, parts: unsafe { MaybeUninit::slice_get_ref(&parts[..1]) } } + Formatted { + sign, + // SAFETY: we just initialized the elements `..1`. + parts: unsafe { MaybeUninit::slice_assume_init_ref(&parts[..1]) }, + } } } FullDecoded::Finite(ref decoded) => { @@ -557,12 +563,12 @@ where FullDecoded::Nan => { parts[0] = MaybeUninit::new(Part::Copy(b"NaN")); // SAFETY: we just initialized the elements `..1`. - Formatted { sign, parts: unsafe { MaybeUninit::slice_get_ref(&parts[..1]) } } + Formatted { sign, parts: unsafe { MaybeUninit::slice_assume_init_ref(&parts[..1]) } } } FullDecoded::Infinite => { parts[0] = MaybeUninit::new(Part::Copy(b"inf")); // SAFETY: we just initialized the elements `..1`. - Formatted { sign, parts: unsafe { MaybeUninit::slice_get_ref(&parts[..1]) } } + Formatted { sign, parts: unsafe { MaybeUninit::slice_assume_init_ref(&parts[..1]) } } } FullDecoded::Zero => { parts[0] = if dec_bounds.0 <= 0 && 0 < dec_bounds.1 { @@ -571,7 +577,7 @@ where MaybeUninit::new(Part::Copy(if upper { b"0E0" } else { b"0e0" })) }; // SAFETY: we just initialized the elements `..1`. - Formatted { sign, parts: unsafe { MaybeUninit::slice_get_ref(&parts[..1]) } } + Formatted { sign, parts: unsafe { MaybeUninit::slice_assume_init_ref(&parts[..1]) } } } FullDecoded::Finite(ref decoded) => { let (buf, exp) = format_shortest(decoded, buf); @@ -648,12 +654,12 @@ where FullDecoded::Nan => { parts[0] = MaybeUninit::new(Part::Copy(b"NaN")); // SAFETY: we just initialized the elements `..1`. - Formatted { sign, parts: unsafe { MaybeUninit::slice_get_ref(&parts[..1]) } } + Formatted { sign, parts: unsafe { MaybeUninit::slice_assume_init_ref(&parts[..1]) } } } FullDecoded::Infinite => { parts[0] = MaybeUninit::new(Part::Copy(b"inf")); // SAFETY: we just initialized the elements `..1`. - Formatted { sign, parts: unsafe { MaybeUninit::slice_get_ref(&parts[..1]) } } + Formatted { sign, parts: unsafe { MaybeUninit::slice_assume_init_ref(&parts[..1]) } } } FullDecoded::Zero => { if ndigits > 1 { @@ -661,12 +667,18 @@ where parts[0] = MaybeUninit::new(Part::Copy(b"0.")); parts[1] = MaybeUninit::new(Part::Zero(ndigits - 1)); parts[2] = MaybeUninit::new(Part::Copy(if upper { b"E0" } else { b"e0" })); - // SAFETY: we just initialized the elements `..3`. - Formatted { sign, parts: unsafe { MaybeUninit::slice_get_ref(&parts[..3]) } } + Formatted { + sign, + // SAFETY: we just initialized the elements `..3`. + parts: unsafe { MaybeUninit::slice_assume_init_ref(&parts[..3]) }, + } } else { parts[0] = MaybeUninit::new(Part::Copy(if upper { b"0E0" } else { b"0e0" })); - // SAFETY: we just initialized the elements `..1`. - Formatted { sign, parts: unsafe { MaybeUninit::slice_get_ref(&parts[..1]) } } + Formatted { + sign, + // SAFETY: we just initialized the elements `..1`. + parts: unsafe { MaybeUninit::slice_assume_init_ref(&parts[..1]) }, + } } } FullDecoded::Finite(ref decoded) => { @@ -716,24 +728,30 @@ where FullDecoded::Nan => { parts[0] = MaybeUninit::new(Part::Copy(b"NaN")); // SAFETY: we just initialized the elements `..1`. - Formatted { sign, parts: unsafe { MaybeUninit::slice_get_ref(&parts[..1]) } } + Formatted { sign, parts: unsafe { MaybeUninit::slice_assume_init_ref(&parts[..1]) } } } FullDecoded::Infinite => { parts[0] = MaybeUninit::new(Part::Copy(b"inf")); // SAFETY: we just initialized the elements `..1`. - Formatted { sign, parts: unsafe { MaybeUninit::slice_get_ref(&parts[..1]) } } + Formatted { sign, parts: unsafe { MaybeUninit::slice_assume_init_ref(&parts[..1]) } } } FullDecoded::Zero => { if frac_digits > 0 { // [0.][0000] parts[0] = MaybeUninit::new(Part::Copy(b"0.")); parts[1] = MaybeUninit::new(Part::Zero(frac_digits)); - // SAFETY: we just initialized the elements `..2`. - Formatted { sign, parts: unsafe { MaybeUninit::slice_get_ref(&parts[..2]) } } + Formatted { + sign, + // SAFETY: we just initialized the elements `..2`. + parts: unsafe { MaybeUninit::slice_assume_init_ref(&parts[..2]) }, + } } else { parts[0] = MaybeUninit::new(Part::Copy(b"0")); - // SAFETY: we just initialized the elements `..1`. - Formatted { sign, parts: unsafe { MaybeUninit::slice_get_ref(&parts[..1]) } } + Formatted { + sign, + // SAFETY: we just initialized the elements `..1`. + parts: unsafe { MaybeUninit::slice_assume_init_ref(&parts[..1]) }, + } } } FullDecoded::Finite(ref decoded) => { @@ -754,12 +772,18 @@ where // [0.][0000] parts[0] = MaybeUninit::new(Part::Copy(b"0.")); parts[1] = MaybeUninit::new(Part::Zero(frac_digits)); - // SAFETY: we just initialized the elements `..2`. - Formatted { sign, parts: unsafe { MaybeUninit::slice_get_ref(&parts[..2]) } } + Formatted { + sign, + // SAFETY: we just initialized the elements `..2`. + parts: unsafe { MaybeUninit::slice_assume_init_ref(&parts[..2]) }, + } } else { parts[0] = MaybeUninit::new(Part::Copy(b"0")); - // SAFETY: we just initialized the elements `..1`. - Formatted { sign, parts: unsafe { MaybeUninit::slice_get_ref(&parts[..1]) } } + Formatted { + sign, + // SAFETY: we just initialized the elements `..1`. + parts: unsafe { MaybeUninit::slice_assume_init_ref(&parts[..1]) }, + } } } else { Formatted { sign, parts: digits_to_dec_str(buf, exp, frac_digits, parts) } diff --git a/library/core/src/num/flt2dec/strategy/dragon.rs b/library/core/src/num/flt2dec/strategy/dragon.rs index 8cb6763cdbe45..8ced5971ec2f9 100644 --- a/library/core/src/num/flt2dec/strategy/dragon.rs +++ b/library/core/src/num/flt2dec/strategy/dragon.rs @@ -246,7 +246,7 @@ pub fn format_shortest<'a>( // it seems that this condition is very hard to satisfy (possibly impossible), // but we are just being safe and consistent here. // SAFETY: we initialized that memory above. - if let Some(c) = round_up(unsafe { MaybeUninit::slice_get_mut(&mut buf[..i]) }) { + if let Some(c) = round_up(unsafe { MaybeUninit::slice_assume_init_mut(&mut buf[..i]) }) { buf[i] = MaybeUninit::new(c); i += 1; k += 1; @@ -254,7 +254,7 @@ pub fn format_shortest<'a>( } // SAFETY: we initialized that memory above. - (unsafe { MaybeUninit::slice_get_ref(&buf[..i]) }, k) + (unsafe { MaybeUninit::slice_assume_init_ref(&buf[..i]) }, k) } /// The exact and fixed mode implementation for Dragon. @@ -332,7 +332,7 @@ pub fn format_exact<'a>( *c = MaybeUninit::new(b'0'); } // SAFETY: we initialized that memory above. - return (unsafe { MaybeUninit::slice_get_ref(&buf[..len]) }, k); + return (unsafe { MaybeUninit::slice_assume_init_ref(&buf[..len]) }, k); } let mut d = 0; @@ -371,7 +371,7 @@ pub fn format_exact<'a>( // if rounding up changes the length, the exponent should also change. // but we've been requested a fixed number of digits, so do not alter the buffer... // SAFETY: we initialized that memory above. - if let Some(c) = round_up(unsafe { MaybeUninit::slice_get_mut(&mut buf[..len]) }) { + if let Some(c) = round_up(unsafe { MaybeUninit::slice_assume_init_mut(&mut buf[..len]) }) { // ...unless we've been requested the fixed precision instead. // we also need to check that, if the original buffer was empty, // the additional digit can only be added when `k == limit` (edge case). @@ -384,5 +384,5 @@ pub fn format_exact<'a>( } // SAFETY: we initialized that memory above. - (unsafe { MaybeUninit::slice_get_ref(&buf[..len]) }, k) + (unsafe { MaybeUninit::slice_assume_init_ref(&buf[..len]) }, k) } diff --git a/library/core/src/num/flt2dec/strategy/grisu.rs b/library/core/src/num/flt2dec/strategy/grisu.rs index ed1dd654a3add..a4cb51c6297af 100644 --- a/library/core/src/num/flt2dec/strategy/grisu.rs +++ b/library/core/src/num/flt2dec/strategy/grisu.rs @@ -276,7 +276,7 @@ pub fn format_shortest_opt<'a>( let ten_kappa = (ten_kappa as u64) << e; // scale 10^kappa back to the shared exponent return round_and_weed( // SAFETY: we initialized that memory above. - unsafe { MaybeUninit::slice_get_mut(&mut buf[..i]) }, + unsafe { MaybeUninit::slice_assume_init_mut(&mut buf[..i]) }, exp, plus1rem, delta1, @@ -327,7 +327,7 @@ pub fn format_shortest_opt<'a>( let ten_kappa = 1 << e; // implicit divisor return round_and_weed( // SAFETY: we initialized that memory above. - unsafe { MaybeUninit::slice_get_mut(&mut buf[..i]) }, + unsafe { MaybeUninit::slice_assume_init_mut(&mut buf[..i]) }, exp, r, threshold, @@ -701,7 +701,7 @@ pub fn format_exact_opt<'a>( // `10^kappa` did not overflow after all, the second check is fine. if ten_kappa - remainder > remainder && ten_kappa - 2 * remainder >= 2 * ulp { // SAFETY: our caller initialized that memory. - return Some((unsafe { MaybeUninit::slice_get_ref(&buf[..len]) }, exp)); + return Some((unsafe { MaybeUninit::slice_assume_init_ref(&buf[..len]) }, exp)); } // :<------- remainder ------>| : @@ -722,8 +722,10 @@ pub fn format_exact_opt<'a>( // as `10^kappa` is never zero). also note that `remainder - ulp <= 10^kappa`, // so the second check does not overflow. if remainder > ulp && ten_kappa - (remainder - ulp) <= remainder - ulp { - // SAFETY: our caller must have initialized that memory. - if let Some(c) = round_up(unsafe { MaybeUninit::slice_get_mut(&mut buf[..len]) }) { + if let Some(c) = + // SAFETY: our caller must have initialized that memory. + round_up(unsafe { MaybeUninit::slice_assume_init_mut(&mut buf[..len]) }) + { // only add an additional digit when we've been requested the fixed precision. // we also need to check that, if the original buffer was empty, // the additional digit can only be added when `exp == limit` (edge case). @@ -734,7 +736,7 @@ pub fn format_exact_opt<'a>( } } // SAFETY: we and our caller initialized that memory. - return Some((unsafe { MaybeUninit::slice_get_ref(&buf[..len]) }, exp)); + return Some((unsafe { MaybeUninit::slice_assume_init_ref(&buf[..len]) }, exp)); } // otherwise we are doomed (i.e., some values between `v - 1 ulp` and `v + 1 ulp` are diff --git a/library/core/src/slice/sort.rs b/library/core/src/slice/sort.rs index 972a33d6489e9..4a00124fcff3e 100644 --- a/library/core/src/slice/sort.rs +++ b/library/core/src/slice/sort.rs @@ -299,8 +299,8 @@ where if start_l == end_l { // Trace `block_l` elements from the left side. - start_l = MaybeUninit::first_ptr_mut(&mut offsets_l); - end_l = MaybeUninit::first_ptr_mut(&mut offsets_l); + start_l = MaybeUninit::slice_as_mut_ptr(&mut offsets_l); + end_l = MaybeUninit::slice_as_mut_ptr(&mut offsets_l); let mut elem = l; for i in 0..block_l { @@ -325,8 +325,8 @@ where if start_r == end_r { // Trace `block_r` elements from the right side. - start_r = MaybeUninit::first_ptr_mut(&mut offsets_r); - end_r = MaybeUninit::first_ptr_mut(&mut offsets_r); + start_r = MaybeUninit::slice_as_mut_ptr(&mut offsets_r); + end_r = MaybeUninit::slice_as_mut_ptr(&mut offsets_r); let mut elem = r; for i in 0..block_r { From 41a047dac8f8457521fcb9850623475d1320285e Mon Sep 17 00:00:00 2001 From: Ralf Jung <post@ralfj.de> Date: Wed, 2 Sep 2020 10:00:08 +0200 Subject: [PATCH 31/37] document remaining unsafety in maybe_uninit.rs --- library/core/src/mem/maybe_uninit.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/library/core/src/mem/maybe_uninit.rs b/library/core/src/mem/maybe_uninit.rs index f770df61d9c54..b64abf68c5e4a 100644 --- a/library/core/src/mem/maybe_uninit.rs +++ b/library/core/src/mem/maybe_uninit.rs @@ -3,8 +3,6 @@ use crate::fmt; use crate::intrinsics; use crate::mem::ManuallyDrop; -// ignore-tidy-undocumented-unsafe - /// A wrapper type to construct uninitialized instances of `T`. /// /// # Initialization invariant @@ -355,6 +353,7 @@ impl<T> MaybeUninit<T> { #[rustc_diagnostic_item = "maybe_uninit_zeroed"] pub fn zeroed() -> MaybeUninit<T> { let mut u = MaybeUninit::<T>::uninit(); + // SAFETY: `u.as_mut_ptr()` points to allocated memory. unsafe { u.as_mut_ptr().write_bytes(0u8, 1); } @@ -368,10 +367,9 @@ impl<T> MaybeUninit<T> { #[unstable(feature = "maybe_uninit_extra", issue = "63567")] #[inline(always)] pub fn write(&mut self, val: T) -> &mut T { - unsafe { - self.value = ManuallyDrop::new(val); - self.assume_init_mut() - } + *self = MaybeUninit::new(val); + // SAFETY: We just initialized this value. + unsafe { self.assume_init_mut() } } /// Gets a pointer to the contained value. Reading from this pointer or turning it From 18c14fde0d293a18fbd3c14487b52e1ce7daa205 Mon Sep 17 00:00:00 2001 From: Joshua Nelson <jyn514@gmail.com> Date: Sat, 29 Aug 2020 15:19:43 -0400 Subject: [PATCH 32/37] Misc cleanup - Preserve suffixes when displaying - Rename test file to match `intra-link*` - Remove unnecessary .clone()s - Improve comments and naming - Fix more bugs and add tests - Escape intra-doc link example in public documentation --- src/librustdoc/clean/types.rs | 6 +- src/librustdoc/html/markdown.rs | 65 ++++++++++++------- .../passes/collect_intra_doc_links.rs | 22 +++++-- src/test/rustdoc/disambiguator_removed.rs | 33 ---------- .../intra-link-disambiguators-removed.rs | 51 +++++++++++++++ 5 files changed, 114 insertions(+), 63 deletions(-) delete mode 100644 src/test/rustdoc/disambiguator_removed.rs create mode 100644 src/test/rustdoc/intra-link-disambiguators-removed.rs diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 05c90a7c4034f..223fda84871e9 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -439,7 +439,7 @@ pub struct ItemLink { /// The link text displayed in the HTML. /// /// This may not be the same as `link` if there was a disambiguator - /// in an intra-doc link (e.g. [`fn@f`]) + /// in an intra-doc link (e.g. \[`fn@f`\]) pub(crate) link_text: String, pub(crate) did: Option<DefId>, /// The url fragment to append to the link @@ -447,7 +447,9 @@ pub struct ItemLink { } pub struct RenderedLink { - /// The text the link was original written as + /// The text the link was original written as. + /// + /// This could potentially include disambiguators and backticks. pub(crate) original_text: String, /// The text to display in the HTML pub(crate) new_text: String, diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index 6d634bac7622d..58d75c0747295 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -338,83 +338,102 @@ impl<'a, I: Iterator<Item = Event<'a>>> Iterator for CodeBlocks<'_, 'a, I> { } /// Make headings links with anchor IDs and build up TOC. -struct LinkReplacer<'a, 'b, I: Iterator<Item = Event<'a>>> { +struct LinkReplacer<'a, I: Iterator<Item = Event<'a>>> { inner: I, links: &'a [RenderedLink], - shortcut_link: Option<&'b RenderedLink>, + shortcut_link: Option<&'a RenderedLink>, } -impl<'a, I: Iterator<Item = Event<'a>>> LinkReplacer<'a, '_, I> { +impl<'a, I: Iterator<Item = Event<'a>>> LinkReplacer<'a, I> { fn new(iter: I, links: &'a [RenderedLink]) -> Self { LinkReplacer { inner: iter, links, shortcut_link: None } } } -impl<'a: 'b, 'b, I: Iterator<Item = Event<'a>>> Iterator for LinkReplacer<'a, 'b, I> { +impl<'a, I: Iterator<Item = Event<'a>>> Iterator for LinkReplacer<'a, I> { type Item = Event<'a>; fn next(&mut self) -> Option<Self::Item> { + use pulldown_cmark::LinkType; + let mut event = self.inner.next(); - // Remove disambiguators from shortcut links (`[fn@f]`) + // Replace intra-doc links and remove disambiguators from shortcut links (`[fn@f]`). match &mut event { + // This is a shortcut link that was resolved by the broken_link_callback: `[fn@f]` + // Remove any disambiguator. Some(Event::Start(Tag::Link( - pulldown_cmark::LinkType::ShortcutUnknown, + // [fn@f] or [fn@f][] + LinkType::ShortcutUnknown | LinkType::CollapsedUnknown, dest, title, ))) => { debug!("saw start of shortcut link to {} with title {}", dest, title); - let link = if let Some(link) = - self.links.iter().find(|&link| *link.original_text == **dest) - { - // Not sure why this is necessary - maybe the broken_link_callback doesn't always work? - *dest = CowStr::Borrowed(link.href.as_ref()); - Some(link) - } else { - self.links.iter().find(|&link| *link.href == **dest) - }; + // If this is a shortcut link, it was resolved by the broken_link_callback. + // So the URL will already be updated properly. + let link = self.links.iter().find(|&link| *link.href == **dest); + // Since this is an external iterator, we can't replace the inner text just yet. + // Store that we saw a link so we know to replace it later. if let Some(link) = link { trace!("it matched"); assert!(self.shortcut_link.is_none(), "shortcut links cannot be nested"); self.shortcut_link = Some(link); } } - Some(Event::End(Tag::Link(pulldown_cmark::LinkType::ShortcutUnknown, dest, _))) => { + // Now that we're done with the shortcut link, don't replace any more text. + Some(Event::End(Tag::Link( + LinkType::ShortcutUnknown | LinkType::CollapsedUnknown, + dest, + _, + ))) => { debug!("saw end of shortcut link to {}", dest); - if let Some(_link) = self.links.iter().find(|&link| *link.href == **dest) { + if self.links.iter().find(|&link| *link.href == **dest).is_some() { assert!(self.shortcut_link.is_some(), "saw closing link without opening tag"); self.shortcut_link = None; } } - // Handle backticks in inline code blocks + // Handle backticks in inline code blocks, but only if we're in the middle of a shortcut link. + // [`fn@f`] Some(Event::Code(text)) => { trace!("saw code {}", text); if let Some(link) = self.shortcut_link { trace!("original text was {}", link.original_text); + // NOTE: this only replaces if the code block is the *entire* text. + // If only part of the link has code highlighting, the disambiguator will not be removed. + // e.g. [fn@`f`] + // This is a limitation from `collect_intra_doc_links`: it passes a full link, + // and does not distinguish at all between code blocks. + // So we could never be sure we weren't replacing too much: + // [fn@my_`f`unc] is treated the same as [my_func()] in that pass. + // + // NOTE: &[1..len() - 1] is to strip the backticks if **text == link.original_text[1..link.original_text.len() - 1] { debug!("replacing {} with {}", text, link.new_text); - *text = link.new_text.clone().into(); + *text = CowStr::Borrowed(&link.new_text); } } } - // Replace plain text in links + // Replace plain text in links, but only in the middle of a shortcut link. + // [fn@f] Some(Event::Text(text)) => { trace!("saw text {}", text); if let Some(link) = self.shortcut_link { trace!("original text was {}", link.original_text); + // NOTE: same limitations as `Event::Code` if **text == *link.original_text { debug!("replacing {} with {}", text, link.new_text); - *text = link.new_text.clone().into(); + *text = CowStr::Borrowed(&link.new_text); } } } + // If this is a link, but not a shortcut link, + // replace the URL, since the broken_link_callback was not called. Some(Event::Start(Tag::Link(_, dest, _))) => { if let Some(link) = self.links.iter().find(|&link| *link.original_text == **dest) { - // Not sure why this is necessary - maybe the broken_link_callback doesn't always work? *dest = CowStr::Borrowed(link.href.as_ref()); } } - // Anything else couldn't have been a valid Rust path + // Anything else couldn't have been a valid Rust path, so no need to replace the text. _ => {} } diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index feadf82a2b57e..9a705293376a0 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -717,11 +717,11 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { continue; } - // We stripped ` characters for `path_str`. - // The original link might have had multiple pairs of backticks, - // but we don't handle this case. - //link_text = if had_backticks { format!("`{}`", path_str) } else { path_str.to_owned() }; - link_text = path_str.to_owned(); + // We stripped `()` and `!` when parsing the disambiguator. + // Add them back to be displayed, but not prefix disambiguators. + link_text = disambiguator + .map(|d| d.display_for(path_str)) + .unwrap_or_else(|| path_str.to_owned()); // In order to correctly resolve intra-doc-links we need to // pick a base AST node to work from. If the documentation for @@ -1000,6 +1000,18 @@ enum Disambiguator { } impl Disambiguator { + /// The text that should be displayed when the path is rendered as HTML. + /// + /// NOTE: `path` is not the original link given by the user, but a name suitable for passing to `resolve`. + fn display_for(&self, path: &str) -> String { + match self { + // FIXME: this will have different output if the user had `m!()` originally. + Self::Kind(DefKind::Macro(MacroKind::Bang)) => format!("{}!", path), + Self::Kind(DefKind::Fn) => format!("{}()", path), + _ => path.to_owned(), + } + } + /// (disambiguator, path_str) fn from_str(link: &str) -> Result<(Self, &str), ()> { use Disambiguator::{Kind, Namespace as NS, Primitive}; diff --git a/src/test/rustdoc/disambiguator_removed.rs b/src/test/rustdoc/disambiguator_removed.rs deleted file mode 100644 index 74411870e9f8a..0000000000000 --- a/src/test/rustdoc/disambiguator_removed.rs +++ /dev/null @@ -1,33 +0,0 @@ -#![deny(intra_doc_link_resolution_failure)] -// first try backticks -/// Trait: [`trait@Name`], fn: [`fn@Name`], [`Name`][`macro@Name`] -// @has disambiguator_removed/struct.AtDisambiguator.html -// @has - '//a[@href="../disambiguator_removed/trait.Name.html"][code]' "Name" -// @has - '//a[@href="../disambiguator_removed/fn.Name.html"][code]' "Name" -// @has - '//a[@href="../disambiguator_removed/macro.Name.html"][code]' "Name" -pub struct AtDisambiguator; - -/// fn: [`Name()`], macro: [`Name!`] -// @has disambiguator_removed/struct.SymbolDisambiguator.html -// @has - '//a[@href="../disambiguator_removed/fn.Name.html"][code]' "Name()" -// @has - '//a[@href="../disambiguator_removed/macro.Name.html"][code]' "Name!" -pub struct SymbolDisambiguator; - -// Now make sure that backticks aren't added if they weren't already there -/// [fn@Name] -// @has disambiguator_removed/trait.Name.html -// @has - '//a[@href="../disambiguator_removed/fn.Name.html"]' "Name" -// @!has - '//a[@href="../disambiguator_removed/fn.Name.html"][code]' "Name" - -// FIXME: this will turn !() into ! alone -/// [Name!()] -// @has - '//a[@href="../disambiguator_removed/macro.Name.html"]' "Name!" -pub trait Name {} - -#[allow(non_snake_case)] -pub fn Name() {} - -#[macro_export] -macro_rules! Name { - () => () -} diff --git a/src/test/rustdoc/intra-link-disambiguators-removed.rs b/src/test/rustdoc/intra-link-disambiguators-removed.rs new file mode 100644 index 0000000000000..26d05b484b919 --- /dev/null +++ b/src/test/rustdoc/intra-link-disambiguators-removed.rs @@ -0,0 +1,51 @@ +// ignore-tidy-linelength +#![deny(intra_doc_link_resolution_failure)] +// first try backticks +/// Trait: [`trait@Name`], fn: [`fn@Name`], [`Name`][`macro@Name`] +// @has intra_link_disambiguators_removed/struct.AtDisambiguator.html +// @has - '//a[@href="../intra_link_disambiguators_removed/trait.Name.html"][code]' "Name" +// @has - '//a[@href="../intra_link_disambiguators_removed/fn.Name.html"][code]' "Name" +// @has - '//a[@href="../intra_link_disambiguators_removed/macro.Name.html"][code]' "Name" +pub struct AtDisambiguator; + +/// fn: [`Name()`], macro: [`Name!`] +// @has intra_link_disambiguators_removed/struct.SymbolDisambiguator.html +// @has - '//a[@href="../intra_link_disambiguators_removed/fn.Name.html"][code]' "Name()" +// @has - '//a[@href="../intra_link_disambiguators_removed/macro.Name.html"][code]' "Name!" +pub struct SymbolDisambiguator; + +// Now make sure that backticks aren't added if they weren't already there +/// [fn@Name] +// @has intra_link_disambiguators_removed/trait.Name.html +// @has - '//a[@href="../intra_link_disambiguators_removed/fn.Name.html"]' "Name" +// @!has - '//a[@href="../intra_link_disambiguators_removed/fn.Name.html"][code]' "Name" + +// FIXME: this will turn !() into ! alone +/// [Name!()] +// @has - '//a[@href="../intra_link_disambiguators_removed/macro.Name.html"]' "Name!" +pub trait Name {} + +#[allow(non_snake_case)] + +// Try collapsed reference links +/// [macro@Name][] +// @has intra_link_disambiguators_removed/fn.Name.html +// @has - '//a[@href="../intra_link_disambiguators_removed/macro.Name.html"]' "Name" + +// Try links that have the same text as a generated URL +/// Weird URL aligned [../intra_link_disambiguators_removed/macro.Name.html][trait@Name] +// @has - '//a[@href="../intra_link_disambiguators_removed/trait.Name.html"]' "../intra_link_disambiguators_removed/macro.Name.html" +pub fn Name() {} + +#[macro_export] +// Rustdoc doesn't currently handle links that have weird interspersing of inline code blocks. +/// [fn@Na`m`e] +// @has intra_link_disambiguators_removed/macro.Name.html +// @has - '//a[@href="../intra_link_disambiguators_removed/fn.Name.html"]' "fn@Name" + +// It also doesn't handle any case where the code block isn't the whole link text: +/// [trait@`Name`] +// @has - '//a[@href="../intra_link_disambiguators_removed/trait.Name.html"]' "trait@Name" +macro_rules! Name { + () => () +} From b97d4131fe2cb3f32c13a0f5b0e3eb6f97c4f75a Mon Sep 17 00:00:00 2001 From: Jubilee Young <workingjubilee@gmail.com> Date: Thu, 20 Aug 2020 02:37:00 -0700 Subject: [PATCH 33/37] Refactor byteorder to std in rustc_middle Use std::io::{Read, Write} and {to, from}_{le, be}_bytes methods in order to remove byteorder from librustc_middle's dependency graph. --- Cargo.lock | 1 - compiler/rustc_middle/Cargo.toml | 1 - compiler/rustc_middle/src/mir/interpret/mod.rs | 16 +++++++++------- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d4e718cb603e8..b84f6ef51ac20 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3722,7 +3722,6 @@ name = "rustc_middle" version = "0.0.0" dependencies = [ "bitflags", - "byteorder", "chalk-ir", "measureme", "polonius-engine", diff --git a/compiler/rustc_middle/Cargo.toml b/compiler/rustc_middle/Cargo.toml index 302a907538c84..a5a860a38b3e8 100644 --- a/compiler/rustc_middle/Cargo.toml +++ b/compiler/rustc_middle/Cargo.toml @@ -26,7 +26,6 @@ rustc_index = { path = "../rustc_index" } rustc_serialize = { path = "../rustc_serialize" } rustc_ast = { path = "../rustc_ast" } rustc_span = { path = "../rustc_span" } -byteorder = { version = "1.3" } chalk-ir = "0.21.0" smallvec = { version = "1.0", features = ["union", "may_dangle"] } measureme = "0.7.1" diff --git a/compiler/rustc_middle/src/mir/interpret/mod.rs b/compiler/rustc_middle/src/mir/interpret/mod.rs index c7e32dd07082b..afbb67555df4f 100644 --- a/compiler/rustc_middle/src/mir/interpret/mod.rs +++ b/compiler/rustc_middle/src/mir/interpret/mod.rs @@ -98,10 +98,10 @@ mod value; use std::convert::TryFrom; use std::fmt; use std::io; +use std::io::{Read, Write}; use std::num::NonZeroU32; use std::sync::atomic::{AtomicU32, Ordering}; -use byteorder::{BigEndian, LittleEndian, ReadBytesExt, WriteBytesExt}; use rustc_ast::LitKind; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::sync::{HashMapExt, Lock}; @@ -561,18 +561,20 @@ pub fn write_target_uint( mut target: &mut [u8], data: u128, ) -> Result<(), io::Error> { - let len = target.len(); match endianness { - Endian::Little => target.write_uint128::<LittleEndian>(data, len), - Endian::Big => target.write_uint128::<BigEndian>(data, len), - } + Endian::Little => target.write(&data.to_le_bytes())?, + Endian::Big => target.write(&data.to_be_bytes())?, + }; + Ok(()) } #[inline] pub fn read_target_uint(endianness: Endian, mut source: &[u8]) -> Result<u128, io::Error> { + let mut buf = [0; 16]; + source.read(&mut buf)?; match endianness { - Endian::Little => source.read_uint128::<LittleEndian>(source.len()), - Endian::Big => source.read_uint128::<BigEndian>(source.len()), + Endian::Little => Ok(u128::from_le_bytes(buf)), + Endian::Big => Ok(u128::from_be_bytes(buf)), } } From 74b4eea64d91d016652f991fa9c4525897b701a0 Mon Sep 17 00:00:00 2001 From: Jubilee Young <workingjubilee@gmail.com> Date: Thu, 20 Aug 2020 05:32:14 -0700 Subject: [PATCH 34/37] Remove reference to byteorder limits --- .../rustc_middle/src/mir/interpret/allocation.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_middle/src/mir/interpret/allocation.rs b/compiler/rustc_middle/src/mir/interpret/allocation.rs index 505939d56ed32..ee1ea816e0192 100644 --- a/compiler/rustc_middle/src/mir/interpret/allocation.rs +++ b/compiler/rustc_middle/src/mir/interpret/allocation.rs @@ -345,10 +345,8 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra<Tag>> Allocation<Tag, Extra> { /// Reads a *non-ZST* scalar. /// - /// ZSTs can't be read for two reasons: - /// * byte-order cannot work with zero-element buffers; - /// * in order to obtain a `Pointer`, we need to check for ZSTness anyway due to integer - /// pointers being valid for ZSTs. + /// ZSTs can't be read because in order to obtain a `Pointer`, we need to check + /// for ZSTness anyway due to integer pointers being valid for ZSTs. /// /// It is the caller's responsibility to check bounds and alignment beforehand. /// Most likely, you want to call `InterpCx::read_scalar` instead of this method. @@ -397,10 +395,8 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra<Tag>> Allocation<Tag, Extra> { /// Writes a *non-ZST* scalar. /// - /// ZSTs can't be read for two reasons: - /// * byte-order cannot work with zero-element buffers; - /// * in order to obtain a `Pointer`, we need to check for ZSTness anyway due to integer - /// pointers being valid for ZSTs. + /// ZSTs can't be read because in order to obtain a `Pointer`, we need to check + /// for ZSTness anyway due to integer pointers being valid for ZSTs. /// /// It is the caller's responsibility to check bounds and alignment beforehand. /// Most likely, you want to call `InterpCx::write_scalar` instead of this method. From fe2a867125ff80dcb12df19581bd28b342b15c28 Mon Sep 17 00:00:00 2001 From: Jubilee <46493976+workingjubilee@users.noreply.github.com> Date: Thu, 20 Aug 2020 15:55:02 -0700 Subject: [PATCH 35/37] Be explicit that we're handling bytes Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com> --- compiler/rustc_middle/src/mir/interpret/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_middle/src/mir/interpret/mod.rs b/compiler/rustc_middle/src/mir/interpret/mod.rs index afbb67555df4f..90f8195a50cb5 100644 --- a/compiler/rustc_middle/src/mir/interpret/mod.rs +++ b/compiler/rustc_middle/src/mir/interpret/mod.rs @@ -570,7 +570,7 @@ pub fn write_target_uint( #[inline] pub fn read_target_uint(endianness: Endian, mut source: &[u8]) -> Result<u128, io::Error> { - let mut buf = [0; 16]; + let mut buf = [0u8; std::mem::size_of::<u128>()]; source.read(&mut buf)?; match endianness { Endian::Little => Ok(u128::from_le_bytes(buf)), From dc00efff9f44ddda1ce318e28f69e716f58d39dd Mon Sep 17 00:00:00 2001 From: Jubilee Young <workingjubilee@gmail.com> Date: Fri, 21 Aug 2020 00:55:33 -0700 Subject: [PATCH 36/37] Explain contract of {read, write}_target_uint --- compiler/rustc_middle/src/mir/interpret/mod.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/compiler/rustc_middle/src/mir/interpret/mod.rs b/compiler/rustc_middle/src/mir/interpret/mod.rs index 90f8195a50cb5..99966121e4d97 100644 --- a/compiler/rustc_middle/src/mir/interpret/mod.rs +++ b/compiler/rustc_middle/src/mir/interpret/mod.rs @@ -561,17 +561,23 @@ pub fn write_target_uint( mut target: &mut [u8], data: u128, ) -> Result<(), io::Error> { + // This u128 holds an "any-size uint" (since smaller uints can fits in it) + // So we do not write all bytes of the u128, just the "payload". match endianness { Endian::Little => target.write(&data.to_le_bytes())?, Endian::Big => target.write(&data.to_be_bytes())?, }; + debug_assert!(target.len() == 0); // We should have filled the target buffer. Ok(()) } #[inline] pub fn read_target_uint(endianness: Endian, mut source: &[u8]) -> Result<u128, io::Error> { + // This u128 holds an "any-size uint" (since smaller uints can fits in it) let mut buf = [0u8; std::mem::size_of::<u128>()]; + // So we do not read exactly 16 bytes into the u128, just the "payload". source.read(&mut buf)?; + debug_assert!(source.len() == 0); // We should have consumed the source buffer. match endianness { Endian::Little => Ok(u128::from_le_bytes(buf)), Endian::Big => Ok(u128::from_be_bytes(buf)), From 2df552b406d8794dc0617672b34e1542498fd0ce Mon Sep 17 00:00:00 2001 From: Jubilee Young <workingjubilee@gmail.com> Date: Sat, 22 Aug 2020 03:54:15 -0700 Subject: [PATCH 37/37] Fix big endian read/write Co-authored-by: matthewjasper <mjjasper1@gmail.com> --- compiler/rustc_middle/src/mir/interpret/mod.rs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_middle/src/mir/interpret/mod.rs b/compiler/rustc_middle/src/mir/interpret/mod.rs index 99966121e4d97..cbc362d934ff8 100644 --- a/compiler/rustc_middle/src/mir/interpret/mod.rs +++ b/compiler/rustc_middle/src/mir/interpret/mod.rs @@ -565,7 +565,7 @@ pub fn write_target_uint( // So we do not write all bytes of the u128, just the "payload". match endianness { Endian::Little => target.write(&data.to_le_bytes())?, - Endian::Big => target.write(&data.to_be_bytes())?, + Endian::Big => target.write(&data.to_be_bytes()[16 - target.len()..])?, }; debug_assert!(target.len() == 0); // We should have filled the target buffer. Ok(()) @@ -576,12 +576,18 @@ pub fn read_target_uint(endianness: Endian, mut source: &[u8]) -> Result<u128, i // This u128 holds an "any-size uint" (since smaller uints can fits in it) let mut buf = [0u8; std::mem::size_of::<u128>()]; // So we do not read exactly 16 bytes into the u128, just the "payload". - source.read(&mut buf)?; + let uint = match endianness { + Endian::Little => { + source.read(&mut buf)?; + Ok(u128::from_le_bytes(buf)) + } + Endian::Big => { + source.read(&mut buf[16 - source.len()..])?; + Ok(u128::from_be_bytes(buf)) + } + }; debug_assert!(source.len() == 0); // We should have consumed the source buffer. - match endianness { - Endian::Little => Ok(u128::from_le_bytes(buf)), - Endian::Big => Ok(u128::from_be_bytes(buf)), - } + uint } ////////////////////////////////////////////////////////////////////////////////