Auto merge of #74388 - Manishearth:rollup-i7iueu8, r=Manishearth

Rollup of 7 pull requests

Successful merges:

 - #73421 (Clarify effect of orphan rule changes on From/Into)
 - #74037 (Update reference to CONTRIBUTING.md)
 - #74203 (Enforce the static symbol order.)
 - #74295 (Add and fix BTreeMap comments)
 - #74352 (Use local links in the alloc docs.)
 - #74377 (Move libstd's default feature to libtest)
 - #74381 (Update docs for str::as_bytes_mut.)

Failed merges:

r? @ghost

CONTRIBUTING.md

@@ -4,5 +4,12 @@ Thank you for your interest in contributing to Rust!
 
 To get started, read the [Getting Started] guide in the [rustc-dev-guide].
 
+## Bug reports
+
+Did a compiler error message tell you to come here? If you want to create an ICE report,
+refer to [this section][contributing-bug-reports] and [open an issue][issue template].
+
 [Getting Started]: https://rustc-dev-guide.rust-lang.org/getting-started.html
 [rustc-dev-guide]: https://rustc-dev-guide.rust-lang.org/
+[contributing-bug-reports]: https://rustc-dev-guide.rust-lang.org/contributing.html#bug-reports
+[issue template]: https://github.com/rust-lang/rust/issues/new/choose

src/bootstrap/builder.rs

@@ -773,7 +773,8 @@ impl<'a> Builder<'a> {
             let my_out = match mode {
                 // This is the intended out directory for compiler documentation.
                 Mode::Rustc | Mode::ToolRustc | Mode::Codegen => self.compiler_doc_out(target),
-                _ => self.crate_doc_out(target),
+                Mode::Std => out_dir.join(target).join("doc"),
+                _ => panic!("doc mode {:?} not expected", mode),
             };
             let rustdoc = self.rustdoc(compiler);
             self.clear_if_dirty(&my_out, &rustdoc);

src/bootstrap/doc.rs

@@ -417,34 +417,16 @@ impl Step for Std {
         builder.ensure(compile::Std { compiler, target });
         let out_dir = builder.stage_out(compiler, Mode::Std).join(target).join("doc");
 
-        // Here what we're doing is creating a *symlink* (directory junction on
-        // Windows) to the final output location. This is not done as an
-        // optimization but rather for correctness. We've got three trees of
-        // documentation, one for std, one for test, and one for rustc. It's then
-        // our job to merge them all together.
-        //
-        // Unfortunately rustbuild doesn't know nearly as well how to merge doc
-        // trees as rustdoc does itself, so instead of actually having three
-        // separate trees we just have rustdoc output to the same location across
-        // all of them.
-        //
-        // This way rustdoc generates output directly into the output, and rustdoc
-        // will also directly handle merging.
-        let my_out = builder.crate_doc_out(target);
-        t!(symlink_dir_force(&builder.config, &my_out, &out_dir));
         t!(fs::copy(builder.src.join("src/doc/rust.css"), out.join("rust.css")));
 
         let run_cargo_rustdoc_for = |package: &str| {
             let mut cargo =
                 builder.cargo(compiler, Mode::Std, SourceType::InTree, target, "rustdoc");
             compile::std_cargo(builder, target, compiler.stage, &mut cargo);
 
-            cargo.arg("-p").arg(package);
-            // Create all crate output directories first to make sure rustdoc uses
-            // relative links.
-            // FIXME: Cargo should probably do this itself.
-            t!(fs::create_dir_all(out_dir.join(package)));
             cargo
+                .arg("-p")
+                .arg(package)
                 .arg("--")
                 .arg("--markdown-css")
                 .arg("rust.css")
@@ -462,11 +444,17 @@ impl Step for Std {
         // folder structure, that would also build internal crates that we do
         // not want to show in documentation. These crates will later be visited
         // by the rustc step, so internal documentation will show them.
-        let krates = ["alloc", "core", "std", "proc_macro", "test"];
+        //
+        // Note that the order here is important! The crates need to be
+        // processed starting from the leaves, otherwise rustdoc will not
+        // create correct links between crates because rustdoc depends on the
+        // existence of the output directories to know if it should be a local
+        // or remote link.
+        let krates = ["core", "alloc", "std", "proc_macro", "test"];
         for krate in &krates {
             run_cargo_rustdoc_for(krate);
         }
-        builder.cp_r(&my_out, &out);
+        builder.cp_r(&out_dir, &out);
 
         // Look for src/libstd, src/libcore etc in the `x.py doc` arguments and
         // open the corresponding rendered docs.
@@ -529,8 +517,11 @@ impl Step for Rustc {
         // Build rustc.
         builder.ensure(compile::Rustc { compiler, target });
 
-        // We do not symlink to the same shared folder that already contains std library
-        // documentation from previous steps as we do not want to include that.
+        // This uses a shared directory so that librustdoc documentation gets
+        // correctly built and merged with the rustc documentation. This is
+        // needed because rustdoc is built in a different directory from
+        // rustc. rustdoc needs to be able to see everything, for example when
+        // merging the search index, or generating local (relative) links.
         let out_dir = builder.stage_out(compiler, Mode::Rustc).join(target).join("doc");
         t!(symlink_dir_force(&builder.config, &out, &out_dir));
 

src/bootstrap/lib.rs

@@ -612,13 +612,6 @@ impl Build {
         INTERNER.intern_path(self.out.join(&*target).join("md-doc"))
     }
 
-    /// Output directory for all crate documentation for a target (temporary)
-    ///
-    /// The artifacts here are then copied into `doc_out` above.
-    fn crate_doc_out(&self, target: Interned<String>) -> PathBuf {
-        self.out.join(&*target).join("crate-docs")
-    }
-
     /// Returns `true` if no custom `llvm-config` is set for the specified target.
     ///
     /// If no custom `llvm-config` was specified then Rust's llvm will be used.

src/liballoc/collections/btree/map.rs

@@ -1697,6 +1697,8 @@ where
     pred: F,
     inner: DrainFilterInner<'a, K, V>,
 }
+/// Most of the implementation of DrainFilter, independent of the type
+/// of the predicate, thus also serving for BTreeSet::DrainFilter.
 pub(super) struct DrainFilterInner<'a, K: 'a, V: 'a> {
     length: &'a mut usize,
     cur_leaf_edge: Option<Handle<NodeRef<marker::Mut<'a>, K, V, marker::Leaf>, marker::Edge>>,

src/liballoc/collections/btree/navigate.rs

@@ -161,15 +161,16 @@ impl<'a, K, V> Handle<NodeRef<marker::Mut<'a>, K, V, marker::Leaf>, marker::Edge
 impl<K, V> Handle<NodeRef<marker::Owned, K, V, marker::Leaf>, marker::Edge> {
     /// Moves the leaf edge handle to the next leaf edge and returns the key and value
     /// in between, while deallocating any node left behind.
-    /// Unsafe for three reasons:
+    /// Unsafe for two reasons:
     /// - The caller must ensure that the leaf edge is not the last one in the tree
     ///   and is not a handle previously resulting from counterpart `next_back_unchecked`.
-    /// - If the leaf edge is the last edge of a node, that node and possibly ancestors
+    /// - Further use of the updated leaf edge handle is very dangerous. In particular,
+    ///   if the leaf edge is the last edge of a node, that node and possibly ancestors
     ///   will be deallocated, while the reference to those nodes in the surviving ancestor
-    ///   is left dangling; thus further use of the leaf edge handle is dangerous.
-    ///   It is, however, safe to call this method again on the updated handle.
-    ///   if the two preconditions above hold.
-    /// - Using the updated handle may well invalidate the returned references.
+    ///   is left dangling.
+    ///   The only safe way to proceed with the updated handle is to compare it, drop it,
+    ///   call this method again subject to both preconditions listed in the first point,
+    ///   or call counterpart `next_back_unchecked` subject to its preconditions.
     pub unsafe fn next_unchecked(&mut self) -> (K, V) {
         unsafe {
             replace(self, |leaf_edge| {
@@ -183,15 +184,16 @@ impl<K, V> Handle<NodeRef<marker::Owned, K, V, marker::Leaf>, marker::Edge> {
 
     /// Moves the leaf edge handle to the previous leaf edge and returns the key
     /// and value in between, while deallocating any node left behind.
-    /// Unsafe for three reasons:
+    /// Unsafe for two reasons:
     /// - The caller must ensure that the leaf edge is not the first one in the tree
     ///   and is not a handle previously resulting from counterpart `next_unchecked`.
-    /// - If the lead edge is the first edge of a node, that node and possibly ancestors
+    /// - Further use of the updated leaf edge handle is very dangerous. In particular,
+    ///   if the leaf edge is the first edge of a node, that node and possibly ancestors
     ///   will be deallocated, while the reference to those nodes in the surviving ancestor
-    ///   is left dangling; thus further use of the leaf edge handle is dangerous.
-    ///   It is, however, safe to call this method again on the updated handle.
-    ///   if the two preconditions above hold.
-    /// - Using the updated handle may well invalidate the returned references.
+    ///   is left dangling.
+    ///   The only safe way to proceed with the updated handle is to compare it, drop it,
+    ///   call this method again subject to both preconditions listed in the first point,
+    ///   or call counterpart `next_unchecked` subject to its preconditions.
     pub unsafe fn next_back_unchecked(&mut self) -> (K, V) {
         unsafe {
             replace(self, |leaf_edge| {

src/liballoc/collections/btree/node.rs

@@ -94,7 +94,8 @@ struct InternalNode<K, V> {
     data: LeafNode<K, V>,
 
     /// The pointers to the children of this node. `len + 1` of these are considered
-    /// initialized and valid.
+    /// initialized and valid. Although during the process of `into_iter` or `drop`,
+    /// some pointers are dangling while others still need to be traversed.
     edges: [MaybeUninit<BoxedNode<K, V>>; 2 * B],
 }
 
@@ -408,7 +409,7 @@ impl<K, V> NodeRef<marker::Owned, K, V, marker::LeafOrInternal> {
 
 impl<'a, K, V, Type> NodeRef<marker::Mut<'a>, K, V, Type> {
     /// Unsafely asserts to the compiler some static information about whether this
-    /// node is a `Leaf`.
+    /// node is a `Leaf` or an `Internal`.
     unsafe fn cast_unchecked<NewType>(&mut self) -> NodeRef<marker::Mut<'_>, K, V, NewType> {
         NodeRef { height: self.height, node: self.node, root: self.root, _marker: PhantomData }
     }
@@ -515,7 +516,7 @@ impl<'a, K: 'a, V: 'a, Type> NodeRef<marker::Mut<'a>, K, V, Type> {
 }
 
 impl<'a, K, V> NodeRef<marker::Mut<'a>, K, V, marker::Leaf> {
-    /// Adds a key/value pair the end of the node.
+    /// Adds a key/value pair to the end of the node.
     pub fn push(&mut self, key: K, val: V) {
         assert!(self.len() < CAPACITY);
 
@@ -602,8 +603,10 @@ impl<'a, K, V> NodeRef<marker::Mut<'a>, K, V, marker::Internal> {
 }
 
 impl<'a, K, V> NodeRef<marker::Mut<'a>, K, V, marker::LeafOrInternal> {
-    /// Removes a key/value pair from the end of this node. If this is an internal node,
-    /// also removes the edge that was to the right of that pair.
+    /// Removes a key/value pair from the end of this node and returns the pair.
+    /// If this is an internal node, also removes the edge that was to the right
+    /// of that pair and returns the orphaned node that this edge owned with its
+    /// parent erased.
     pub fn pop(&mut self) -> (K, V, Option<Root<K, V>>) {
         assert!(self.len() > 0);
 
@@ -883,7 +886,7 @@ impl<'a, K, V> Handle<NodeRef<marker::Mut<'a>, K, V, marker::Internal>, marker::
     }
 
     /// Unsafely asserts to the compiler some static information about whether the underlying
-    /// node of this handle is a `Leaf`.
+    /// node of this handle is a `Leaf` or an `Internal`.
     unsafe fn cast_unchecked<NewType>(
         &mut self,
     ) -> Handle<NodeRef<marker::Mut<'_>, K, V, NewType>, marker::Edge> {

src/libcore/convert/mod.rs

@@ -18,8 +18,9 @@
 //! [`TryFrom<T>`][`TryFrom`] rather than [`Into<U>`][`Into`] or [`TryInto<U>`][`TryInto`],
 //! as [`From`] and [`TryFrom`] provide greater flexibility and offer
 //! equivalent [`Into`] or [`TryInto`] implementations for free, thanks to a
-//! blanket implementation in the standard library. Only implement [`Into`] or [`TryInto`]
-//! when a conversion to a type outside the current crate is required.
+//! blanket implementation in the standard library. When targeting a version prior to Rust 1.41, it
+//! may be necessary to implement [`Into`] or [`TryInto`] directly when converting to a type
+//! outside the current crate.
 //!
 //! # Generic Implementations
 //!
@@ -298,8 +299,10 @@ pub trait Into<T>: Sized {
 /// because implementing `From` automatically provides one with an implementation of [`Into`]
 /// thanks to the blanket implementation in the standard library.
 ///
-/// Only implement [`Into`] if a conversion to a type outside the current crate is required.
-/// `From` cannot do these type of conversions because of Rust's orphaning rules.
+/// Only implement [`Into`] when targeting a version prior to Rust 1.41 and converting to a type
+/// outside the current crate.
+/// `From` was not able to do these types of conversions in earlier versions because of Rust's
+/// orphaning rules.
 /// See [`Into`] for more details.
 ///
 /// Prefer using [`Into`] over using `From` when specifying trait bounds on a generic function.

src/libcore/str/mod.rs

@@ -2374,11 +2374,14 @@ impl str {
         unsafe { Slices { str: self }.slice }
     }
 
-    /// Converts a mutable string slice to a mutable byte slice. To convert the
-    /// mutable byte slice back into a mutable string slice, use the
-    /// [`str::from_utf8_mut`] function.
+    /// Converts a mutable string slice to a mutable byte slice.
     ///
-    /// [`str::from_utf8_mut`]: ./str/fn.from_utf8_mut.html
+    /// # Safety
+    ///
+    /// The caller must ensure that the content of the slice is valid UTF-8
+    /// before the borrow ends and the underlying `str` is used.
+    ///
+    /// Use of a `str` whose contents are not valid UTF-8 is undefined behavior.
     ///
     /// # Examples
     ///

src/librustc_driver/lib.rs

@@ -65,8 +65,8 @@ pub const EXIT_SUCCESS: i32 = 0;
 /// Exit status code used for compilation failures and invalid flags.
 pub const EXIT_FAILURE: i32 = 1;
 
-const BUG_REPORT_URL: &str = "https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.\
-                              md#bug-reports";
+const BUG_REPORT_URL: &str = "https://github.com/rust-lang/rust/issues/new\
+    ?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md";
 
 const ICE_REPORT_COMPILER_FLAGS: &[&str] = &["Z", "C", "crate-type"];
 

src/librustc_macros/src/symbols.rs

@@ -87,18 +87,29 @@ pub fn symbols(input: TokenStream) -> TokenStream {
     let mut prefill_stream = quote! {};
     let mut counter = 0u32;
     let mut keys = HashSet::<String>::new();
+    let mut prev_key: Option<String> = None;
+    let mut errors = Vec::<String>::new();
 
-    let mut check_dup = |str: &str| {
+    let mut check_dup = |str: &str, errors: &mut Vec<String>| {
         if !keys.insert(str.to_string()) {
-            panic!("Symbol `{}` is duplicated", str);
+            errors.push(format!("Symbol `{}` is duplicated", str));
         }
     };
 
+    let mut check_order = |str: &str, errors: &mut Vec<String>| {
+        if let Some(ref prev_str) = prev_key {
+            if str < prev_str {
+                errors.push(format!("Symbol `{}` must precede `{}`", str, prev_str));
+            }
+        }
+        prev_key = Some(str.to_string());
+    };
+
     // Generate the listed keywords.
     for keyword in &input.keywords.0 {
         let name = &keyword.name;
         let value = &keyword.value;
-        check_dup(&value.value());
+        check_dup(&value.value(), &mut errors);
         prefill_stream.extend(quote! {
             #value,
         });
@@ -116,7 +127,8 @@ pub fn symbols(input: TokenStream) -> TokenStream {
             Some(value) => value.value(),
             None => name.to_string(),
         };
-        check_dup(&value);
+        check_dup(&value, &mut errors);
+        check_order(&name.to_string(), &mut errors);
         prefill_stream.extend(quote! {
             #value,
         });
@@ -131,7 +143,7 @@ pub fn symbols(input: TokenStream) -> TokenStream {
     // Generate symbols for the strings "0", "1", ..., "9".
     for n in 0..10 {
         let n = n.to_string();
-        check_dup(&n);
+        check_dup(&n, &mut errors);
         prefill_stream.extend(quote! {
             #n,
         });
@@ -141,6 +153,13 @@ pub fn symbols(input: TokenStream) -> TokenStream {
         counter += 1;
     }
 
+    if !errors.is_empty() {
+        for error in errors.into_iter() {
+            eprintln!("error: {}", error)
+        }
+        panic!("errors in `Keywords` and/or `Symbols`");
+    }
+
     let tt = TokenStream::from(quote! {
         macro_rules! keywords {
             () => {