Auto merge of #75351 - JohnTitor:rollup-q9udsyx, r=JohnTitor

Rollup of 8 pull requests

Successful merges:

 - #74200 (Std panicking unsafe block in unsafe fn)
 - #75286 (Add additional case for Path starts with)
 - #75318 (Resolve `char` as a primitive even if there is a module in scope)
 - #75320 (Detect likely `for foo of bar` JS syntax)
 - #75328 (Cleanup E0749)
 - #75344 (Rename "Important traits" to "Notable traits")
 - #75348 (Move to intra-doc links in library/core/src/time.rs)
 - #75350 (Do not ICE when lowering invalid extern fn with bodies)

Failed merges:

r? @ghost

Author: bors

library/core/src/time.rs

@@ -30,12 +30,10 @@ const MICROS_PER_SEC: u64 = 1_000_000;
 /// nanosecond-level precision, APIs binding a system timeout will typically round up
 /// the number of nanoseconds.
 ///
-/// `Duration`s implement many common traits, including [`Add`], [`Sub`], and other
-/// [`ops`] traits. It implements `Default` by returning a zero-length `Duration`.
+/// [`Duration`]s implement many common traits, including [`Add`], [`Sub`], and other
+/// [`ops`] traits. It implements [`Default`] by returning a zero-length `Duration`.
 ///
-/// [`Add`]: ../../std/ops/trait.Add.html
-/// [`Sub`]: ../../std/ops/trait.Sub.html
-/// [`ops`]: ../../std/ops/index.html
+/// [`ops`]: crate::ops
 ///
 /// # Examples
 ///
@@ -293,7 +291,7 @@ impl Duration {
     ///            + duration.subsec_nanos() as f64 * 1e-9);
     /// ```
     ///
-    /// [`subsec_nanos`]: #method.subsec_nanos
+    /// [`subsec_nanos`]: Duration::subsec_nanos
     #[stable(feature = "duration", since = "1.3.0")]
     #[rustc_const_stable(feature = "duration", since = "1.32.0")]
     #[inline]
@@ -421,8 +419,6 @@ impl Duration {
     /// Checked `Duration` addition. Computes `self + other`, returning [`None`]
     /// if overflow occurred.
     ///
-    /// [`None`]: ../../std/option/enum.Option.html#variant.None
-    ///
     /// # Examples
     ///
     /// Basic usage:
@@ -457,8 +453,6 @@ impl Duration {
     /// Checked `Duration` subtraction. Computes `self - other`, returning [`None`]
     /// if the result would be negative or if overflow occurred.
     ///
-    /// [`None`]: ../../std/option/enum.Option.html#variant.None
-    ///
     /// # Examples
     ///
     /// Basic usage:
@@ -494,8 +488,6 @@ impl Duration {
     /// Checked `Duration` multiplication. Computes `self * other`, returning
     /// [`None`] if overflow occurred.
     ///
-    /// [`None`]: ../../std/option/enum.Option.html#variant.None
-    ///
     /// # Examples
     ///
     /// Basic usage:
@@ -526,8 +518,6 @@ impl Duration {
     /// Checked `Duration` division. Computes `self / other`, returning [`None`]
     /// if `other == 0`.
     ///
-    /// [`None`]: ../../std/option/enum.Option.html#variant.None
-    ///
     /// # Examples
     ///
     /// Basic usage:

library/std/src/panicking.rs

@@ -7,6 +7,8 @@
 //! * Executing a panic up to doing the actual implementation
 //! * Shims around "try"
 
+#![deny(unsafe_op_in_unsafe_fn)]
+
 use core::panic::{BoxMeUp, Location, PanicInfo};
 
 use crate::any::Any;
@@ -322,25 +324,48 @@ pub unsafe fn r#try<R, F: FnOnce() -> R>(f: F) -> Result<R, Box<dyn Any + Send>>
     let mut data = Data { f: ManuallyDrop::new(f) };
 
     let data_ptr = &mut data as *mut _ as *mut u8;
-    return if intrinsics::r#try(do_call::<F, R>, data_ptr, do_catch::<F, R>) == 0 {
-        Ok(ManuallyDrop::into_inner(data.r))
-    } else {
-        Err(ManuallyDrop::into_inner(data.p))
-    };
+    // SAFETY:
+    //
+    // Access to the union's fields: this is `std` and we know that the `r#try`
+    // intrinsic fills in the `r` or `p` union field based on its return value.
+    //
+    // The call to `intrinsics::r#try` is made safe by:
+    // - `do_call`, the first argument, can be called with the initial `data_ptr`.
+    // - `do_catch`, the second argument, can be called with the `data_ptr` as well.
+    // See their safety preconditions for more informations
+    unsafe {
+        return if intrinsics::r#try(do_call::<F, R>, data_ptr, do_catch::<F, R>) == 0 {
+            Ok(ManuallyDrop::into_inner(data.r))
+        } else {
+            Err(ManuallyDrop::into_inner(data.p))
+        };
+    }
 
     // We consider unwinding to be rare, so mark this function as cold. However,
     // do not mark it no-inline -- that decision is best to leave to the
     // optimizer (in most cases this function is not inlined even as a normal,
     // non-cold function, though, as of the writing of this comment).
     #[cold]
     unsafe fn cleanup(payload: *mut u8) -> Box<dyn Any + Send + 'static> {
-        let obj = Box::from_raw(__rust_panic_cleanup(payload));
+        // SAFETY: The whole unsafe block hinges on a correct implementation of
+        // the panic handler `__rust_panic_cleanup`. As such we can only
+        // assume it returns the correct thing for `Box::from_raw` to work
+        // without undefined behavior.
+        let obj = unsafe { Box::from_raw(__rust_panic_cleanup(payload)) };
         panic_count::decrease();
         obj
     }
 
+    // SAFETY:
+    // data must be non-NUL, correctly aligned, and a pointer to a `Data<F, R>`
+    // Its must contains a valid `f` (type: F) value that can be use to fill
+    // `data.r`.
+    //
+    // This function cannot be marked as `unsafe` because `intrinsics::r#try`
+    // expects normal function pointers.
     #[inline]
     fn do_call<F: FnOnce() -> R, R>(data: *mut u8) {
+        // SAFETY: this is the responsibilty of the caller, see above.
         unsafe {
             let data = data as *mut Data<F, R>;
             let data = &mut (*data);
@@ -352,8 +377,21 @@ pub unsafe fn r#try<R, F: FnOnce() -> R>(f: F) -> Result<R, Box<dyn Any + Send>>
     // We *do* want this part of the catch to be inlined: this allows the
     // compiler to properly track accesses to the Data union and optimize it
     // away most of the time.
+    //
+    // SAFETY:
+    // data must be non-NUL, correctly aligned, and a pointer to a `Data<F, R>`
+    // Since this uses `cleanup` it also hinges on a correct implementation of
+    // `__rustc_panic_cleanup`.
+    //
+    // This function cannot be marked as `unsafe` because `intrinsics::r#try`
+    // expects normal function pointers.
     #[inline]
     fn do_catch<F: FnOnce() -> R, R>(data: *mut u8, payload: *mut u8) {
+        // SAFETY: this is the responsibilty of the caller, see above.
+        //
+        // When `__rustc_panic_cleaner` is correctly implemented we can rely
+        // on `obj` being the correct thing to pass to `data.p` (after wrapping
+        // in `ManuallyDrop`).
         unsafe {
             let data = data as *mut Data<F, R>;
             let data = &mut (*data);

library/std/src/path.rs

@@ -2031,9 +2031,13 @@ impl Path {
     /// assert!(path.starts_with("/etc"));
     /// assert!(path.starts_with("/etc/"));
     /// assert!(path.starts_with("/etc/passwd"));
-    /// assert!(path.starts_with("/etc/passwd/"));
+    /// assert!(path.starts_with("/etc/passwd/")); // extra slash is okay
+    /// assert!(path.starts_with("/etc/passwd///")); // multiple extra slashes are okay
     ///
     /// assert!(!path.starts_with("/e"));
+    /// assert!(!path.starts_with("/etc/passwd.txt"));
+    ///
+    /// assert!(!Path::new("/etc/foo.rs").starts_with("/etc/foo"));
     /// ```
     #[stable(feature = "rust1", since = "1.0.0")]
     pub fn starts_with<P: AsRef<Path>>(&self, base: P) -> bool {

library/std/src/thread/local.rs

@@ -172,7 +172,11 @@ macro_rules! __thread_local_inner {
                 static __KEY: $crate::thread::__OsLocalKeyInner<$t> =
                     $crate::thread::__OsLocalKeyInner::new();
 
-                __KEY.get(__init)
+                // FIXME: remove the #[allow(...)] marker when macros don't
+                // raise warning for missing/extraneous unsafe blocks anymore.
+                // See https://github.com/rust-lang/rust/issues/74838.
+                #[allow(unused_unsafe)]
+                unsafe { __KEY.get(__init) }
             }
 
             unsafe {

src/librustc_ast_lowering/lib.rs

@@ -573,7 +573,9 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             .resolver
             .trait_map()
             .iter()
-            .map(|(&k, v)| (self.node_id_to_hir_id[k].unwrap(), v.clone()))
+            .filter_map(|(&k, v)| {
+                self.node_id_to_hir_id.get(k).and_then(|id| id.as_ref()).map(|id| (*id, v.clone()))
+            })
             .collect();
 
         let mut def_id_to_hir_id = IndexVec::default();

src/librustc_error_codes/error_codes/E0749.md

@@ -1,4 +1,19 @@
-Negative impls are not allowed to have any items. Negative impls
-declare that a trait is **not** implemented (and never will be) and
-hence there is no need to specify the values for trait methods or
-other items.
+An item was added on a negative impl.
+
+Erroneous code example:
+
+```compile_fail,E0749
+# #![feature(negative_impls)]
+trait MyTrait {
+    type Foo;
+}
+
+impl !MyTrait for u32 {
+    type Foo = i32; // error!
+}
+# fn main() {}
+```
+
+Negative impls are not allowed to have any items. Negative impls declare that a
+trait is **not** implemented (and never will be) and hence there is no need to
+specify the values for trait methods or other items.

src/librustc_parse/parser/expr.rs

@@ -1733,13 +1733,20 @@ impl<'a> Parser<'a> {
         Ok(self.mk_expr(lo.to(self.prev_token.span), kind, attrs))
     }
 
-    fn error_missing_in_for_loop(&self) {
-        let in_span = self.prev_token.span.between(self.token.span);
-        self.struct_span_err(in_span, "missing `in` in `for` loop")
+    fn error_missing_in_for_loop(&mut self) {
+        let (span, msg, sugg) = if self.token.is_ident_named(sym::of) {
+            // Possibly using JS syntax (#75311).
+            let span = self.token.span;
+            self.bump();
+            (span, "try using `in` here instead", "in")
+        } else {
+            (self.prev_token.span.between(self.token.span), "try adding `in` here", " in ")
+        };
+        self.struct_span_err(span, "missing `in` in `for` loop")
             .span_suggestion_short(
-                in_span,
-                "try adding `in` here",
-                " in ".into(),
+                span,
+                msg,
+                sugg.into(),
                 // Has been misleading, at least in the past (closed Issue #48492).
                 Applicability::MaybeIncorrect,
             )

src/librustc_span/symbol.rs

@@ -737,6 +737,7 @@ symbols! {
         not,
         note,
         object_safe_for_dispatch,
+        of,
         offset,
         omit_gdb_pretty_printer_section,
         on,

src/librustdoc/html/render/mod.rs

@@ -3450,7 +3450,7 @@ fn spotlight_decl(decl: &clean::FnDecl) -> String {
                 if impl_.trait_.def_id().map_or(false, |d| c.traits[&d].is_spotlight) {
                     if out.is_empty() {
                         out.push_str(&format!(
-                            "<h3 class=\"important\">Important traits for {}</h3>\
+                            "<h3 class=\"notable\">Notable traits for {}</h3>\
                                       <code class=\"content\">",
                             impl_.for_.print()
                         ));
@@ -3485,7 +3485,7 @@ fn spotlight_decl(decl: &clean::FnDecl) -> String {
     if !out.is_empty() {
         out.insert_str(
             0,
-            "<span class=\"important-traits\"><span class=\"important-traits-tooltip\">ⓘ<div class='important-traits-tooltiptext'><span class=\"docblock\">"
+            "<span class=\"notable-traits\"><span class=\"notable-traits-tooltip\">ⓘ<div class='notable-traits-tooltiptext'><span class=\"docblock\">"
 
         );
         out.push_str("</code></span></div></span></span>");

src/librustdoc/html/static/main.js

@@ -2636,9 +2636,9 @@ function defocusSearchBar() {
         });
     }());
 
-    onEachLazy(document.getElementsByClassName("important-traits"), function(e) {
+    onEachLazy(document.getElementsByClassName("notable-traits"), function(e) {
         e.onclick = function() {
-            this.getElementsByClassName('important-traits-tooltiptext')[0]
+            this.getElementsByClassName('notable-traits-tooltiptext')[0]
                 .classList.toggle("force-tooltip");
         };
     });