Auto merge of #97566 - compiler-errors:rollup-qfxw4j8, r=compiler-errors

Rollup of 6 pull requests

Successful merges:

 - #89685 (refactor: VecDeques Iter fields to private)
 - #97172 (Optimize the diagnostic generation for `extern unsafe`)
 - #97395 (Miri call ABI check: ensure type size+align stay the same)
 - #97431 (don't do `Sized` and other return type checks on RPIT's real type)
 - #97555 (Source code page: line number click adds `NaN`)
 - #97558 (Fix typos in comment)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup

13 files changed, 120 insertions, 61 deletions

diff --git a/compiler/rustc_const_eval/src/interpret/terminator.rs b/compiler/rustc_const_eval/src/interpret/terminator.rs
index a5c7d4c8e20..10da2f803af 100644
--- a/compiler/rustc_const_eval/src/interpret/terminator.rs
+++ b/compiler/rustc_const_eval/src/interpret/terminator.rs
@@ -185,7 +185,14 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 // No question
                 return true;
             }
-            // Compare layout
+            if caller_abi.layout.size != callee_abi.layout.size
+                || caller_abi.layout.align.abi != callee_abi.layout.align.abi
+            {
+                // This cannot go well...
+                // FIXME: What about unsized types?
+                return false;
+            }
+            // The rest *should* be okay, but we are extra conservative.
             match (caller_abi.layout.abi, callee_abi.layout.abi) {
                 // Different valid ranges are okay (once we enforce validity,
                 // that will take care to make it UB to leave the range, just
diff --git a/compiler/rustc_data_structures/src/stable_hasher.rs b/compiler/rustc_data_structures/src/stable_hasher.rs
index c8bb4fc5e6a..a915a4daa95 100644
--- a/compiler/rustc_data_structures/src/stable_hasher.rs
+++ b/compiler/rustc_data_structures/src/stable_hasher.rs
@@ -632,10 +632,10 @@ fn stable_hash_reduce<HCX, I, C, F>(
     }
 }
 
-/// Controls what data we do or not not hash.
+/// Controls what data we do or do not hash.
 /// Whenever a `HashStable` implementation caches its
 /// result, it needs to include `HashingControls` as part
-/// of the key, to ensure that is does not produce an incorrect
+/// of the key, to ensure that it does not produce an incorrect
 /// result (for example, using a `Fingerprint` produced while
 /// hashing `Span`s when a `Fingerprint` without `Span`s is
 /// being requested)
diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs
index e99347206fe..6720399aacb 100644
--- a/compiler/rustc_parse/src/parser/item.rs
+++ b/compiler/rustc_parse/src/parser/item.rs
@@ -996,35 +996,24 @@ impl<'a> Parser<'a> {
     fn parse_item_foreign_mod(
         &mut self,
         attrs: &mut Vec<Attribute>,
-        unsafety: Unsafe,
+        mut unsafety: Unsafe,
     ) -> PResult<'a, ItemInfo> {
-        let sp_start = self.prev_token.span;
         let abi = self.parse_abi(); // ABI?
-        match self.parse_item_list(attrs, |p| p.parse_foreign_item(ForceCollect::No)) {
-            Ok(items) => {
-                let module = ast::ForeignMod { unsafety, abi, items };
-                Ok((Ident::empty(), ItemKind::ForeignMod(module)))
-            }
-            Err(mut err) => {
-                let current_qual_sp = self.prev_token.span;
-                let current_qual_sp = current_qual_sp.to(sp_start);
-                if let Ok(current_qual) = self.span_to_snippet(current_qual_sp) {
-                    // FIXME(davidtwco): avoid depending on the error message text
-                    if err.message[0].0.expect_str() == "expected `{`, found keyword `unsafe`" {
-                        let invalid_qual_sp = self.token.uninterpolated_span();
-                        let invalid_qual = self.span_to_snippet(invalid_qual_sp).unwrap();
-
-                        err.span_suggestion(
-                                current_qual_sp.to(invalid_qual_sp),
-                                &format!("`{}` must come before `{}`", invalid_qual, current_qual),
-                                format!("{} {}", invalid_qual, current_qual),
-                                Applicability::MachineApplicable,
-                            ).note("keyword order for functions declaration is `pub`, `default`, `const`, `async`, `unsafe`, `extern`");
-                    }
-                }
-                Err(err)
-            }
+        if unsafety == Unsafe::No
+            && self.token.is_keyword(kw::Unsafe)
+            && self.look_ahead(1, |t| t.kind == token::OpenDelim(Delimiter::Brace))
+        {
+            let mut err = self.expect(&token::OpenDelim(Delimiter::Brace)).unwrap_err();
+            err.emit();
+            unsafety = Unsafe::Yes(self.token.span);
+            self.eat_keyword(kw::Unsafe);
         }
+        let module = ast::ForeignMod {
+            unsafety,
+            abi,
+            items: self.parse_item_list(attrs, |p| p.parse_foreign_item(ForceCollect::No))?,
+        };
+        Ok((Ident::empty(), ItemKind::ForeignMod(module)))
     }
 
     /// Parses a foreign item (one in an `extern { ... }` block).
diff --git a/compiler/rustc_typeck/src/check/check.rs b/compiler/rustc_typeck/src/check/check.rs
index 09854336599..7499e5efdee 100644
--- a/compiler/rustc_typeck/src/check/check.rs
+++ b/compiler/rustc_typeck/src/check/check.rs
@@ -105,12 +105,6 @@ pub(super) fn check_fn<'a, 'tcx>(
             DUMMY_SP,
             param_env,
         ));
-    // HACK(oli-obk): we rewrite the declared return type, too, so that we don't end up inferring all
-    // unconstrained RPIT to have `()` as their hidden type. This would happen because further down we
-    // compare the ret_coercion with declared_ret_ty, and anything uninferred would be inferred to the
-    // opaque type itself. That again would cause writeback to assume we have a recursive call site
-    // and do the sadly stabilized fallback to `()`.
-    let declared_ret_ty = ret_ty;
     fcx.ret_coercion = Some(RefCell::new(CoerceMany::new(ret_ty)));
     fcx.ret_type_span = Some(decl.output.span());
 
@@ -254,7 +248,12 @@ pub(super) fn check_fn<'a, 'tcx>(
             fcx.next_ty_var(TypeVariableOrigin { kind: TypeVariableOriginKind::DynReturnFn, span });
         debug!("actual_return_ty replaced with {:?}", actual_return_ty);
     }
-    fcx.demand_suptype(span, declared_ret_ty, actual_return_ty);
+
+    // HACK(oli-obk, compiler-errors): We should be comparing this against
+    // `declared_ret_ty`, but then anything uninferred would be inferred to
+    // the opaque type itself. That again would cause writeback to assume
+    // we have a recursive call site and do the sadly stabilized fallback to `()`.
+    fcx.demand_suptype(span, ret_ty, actual_return_ty);
 
     // Check that a function marked as `#[panic_handler]` has signature `fn(&PanicInfo) -> !`
     if let Some(panic_impl_did) = tcx.lang_items().panic_impl()
diff --git a/library/alloc/src/collections/vec_deque/iter.rs b/library/alloc/src/collections/vec_deque/iter.rs
index 19198ab3aa1..e696d7ed636 100644
--- a/library/alloc/src/collections/vec_deque/iter.rs
+++ b/library/alloc/src/collections/vec_deque/iter.rs
@@ -13,9 +13,15 @@ use super::{count, wrap_index, RingSlices};
 /// [`iter`]: super::VecDeque::iter
 #[stable(feature = "rust1", since = "1.0.0")]
 pub struct Iter<'a, T: 'a> {
-    pub(crate) ring: &'a [MaybeUninit<T>],
-    pub(crate) tail: usize,
-    pub(crate) head: usize,
+    ring: &'a [MaybeUninit<T>],
+    tail: usize,
+    head: usize,
+}
+
+impl<'a, T> Iter<'a, T> {
+    pub(super) fn new(ring: &'a [MaybeUninit<T>], tail: usize, head: usize) -> Self {
+        Iter { ring, tail, head }
+    }
 }
 
 #[stable(feature = "collection_debug", since = "1.17.0")]
diff --git a/library/alloc/src/collections/vec_deque/mod.rs b/library/alloc/src/collections/vec_deque/mod.rs
index 04900ead579..e28a94386c7 100644
--- a/library/alloc/src/collections/vec_deque/mod.rs
+++ b/library/alloc/src/collections/vec_deque/mod.rs
@@ -1013,7 +1013,7 @@ impl<T, A: Allocator> VecDeque<T, A> {
     /// ```
     #[stable(feature = "rust1", since = "1.0.0")]
     pub fn iter(&self) -> Iter<'_, T> {
-        Iter { tail: self.tail, head: self.head, ring: unsafe { self.buffer_as_slice() } }
+        Iter::new(unsafe { self.buffer_as_slice() }, self.tail, self.head)
     }
 
     /// Returns a front-to-back iterator that returns mutable references.
@@ -1192,12 +1192,8 @@ impl<T, A: Allocator> VecDeque<T, A> {
         R: RangeBounds<usize>,
     {
         let (tail, head) = self.range_tail_head(range);
-        Iter {
-            tail,
-            head,
-            // The shared reference we have in &self is maintained in the '_ of Iter.
-            ring: unsafe { self.buffer_as_slice() },
-        }
+        // The shared reference we have in &self is maintained in the '_ of Iter.
+        Iter::new(unsafe { self.buffer_as_slice() }, tail, head)
     }
 
     /// Creates an iterator that covers the specified mutable range in the deque.
@@ -1313,16 +1309,15 @@ impl<T, A: Allocator> VecDeque<T, A> {
         self.head = drain_tail;
 
         let deque = NonNull::from(&mut *self);
-        let iter = Iter {
-            tail: drain_tail,
-            head: drain_head,
+        unsafe {
             // Crucially, we only create shared references from `self` here and read from
             // it.  We do not write to `self` nor reborrow to a mutable reference.
             // Hence the raw pointer we created above, for `deque`, remains valid.
-            ring: unsafe { self.buffer_as_slice() },
-        };
+            let ring = self.buffer_as_slice();
+            let iter = Iter::new(ring, drain_tail, drain_head);
 
-        unsafe { Drain::new(drain_head, head, iter, deque) }
+            Drain::new(drain_head, head, iter, deque)
+        }
     }
 
     /// Clears the deque, removing all values.
diff --git a/src/librustdoc/html/static/js/source-script.js b/src/librustdoc/html/static/js/source-script.js
index aaac878d3a3..58c036e0b3c 100644
--- a/src/librustdoc/html/static/js/source-script.js
+++ b/src/librustdoc/html/static/js/source-script.js
@@ -205,6 +205,10 @@ const handleSourceHighlight = (function() {
 
     return ev => {
         let cur_line_id = parseInt(ev.target.id, 10);
+        // It can happen when clicking not on a line number span.
+        if (isNaN(cur_line_id)) {
+            return;
+        }
         ev.preventDefault();
 
         if (ev.shiftKey && prev_line_id) {
diff --git a/src/test/rustdoc-gui/source-code-page.goml b/src/test/rustdoc-gui/source-code-page.goml
index ad7080c39b8..509739c9f29 100644
--- a/src/test/rustdoc-gui/source-code-page.goml
+++ b/src/test/rustdoc-gui/source-code-page.goml
@@ -2,9 +2,9 @@
 goto: file://|DOC_PATH|/src/test_docs/lib.rs.html
 // Check that we can click on the line number.
 click: ".line-numbers > span:nth-child(4)" // This is the span for line 4.
-// Unfortunately, "#4" isn't a valid query selector, so we have to go around that limitation
-// by instead getting the nth span.
-assert-attribute: (".line-numbers > span:nth-child(4)", {"class": "line-highlighted"})
+// Ensure that the page URL was updated.
+assert-document-property: ({"URL": "lib.rs.html#4"}, ENDS_WITH)
+assert-attribute: ("//*[@id='4']", {"class": "line-highlighted"})
 // We now check that the good spans are highlighted
 goto: file://|DOC_PATH|/src/test_docs/lib.rs.html#4-6
 assert-attribute-false: (".line-numbers > span:nth-child(3)", {"class": "line-highlighted"})
@@ -17,3 +17,13 @@ compare-elements-position: ("//*[@id='1']", ".rust > code > span", ("y"))
 
 // Assert that the line numbers text is aligned to the right.
 assert-css: (".line-numbers", {"text-align": "right"})
+
+// Now let's check that clicking on something else than the line number doesn't
+// do anything (and certainly not add a `#NaN` to the URL!).
+show-text: true
+goto: file://|DOC_PATH|/src/test_docs/lib.rs.html
+// We use this assert-position to know where we will click.
+assert-position: ("//*[@id='1']", {"x": 104, "y": 103})
+// We click on the left of the "1" span but still in the "line-number" `<pre>`.
+click: (103, 103)
+assert-document-property: ({"URL": "/lib.rs.html"}, ENDS_WITH)
diff --git a/src/test/ui/impl-trait/rpit-not-sized.rs b/src/test/ui/impl-trait/rpit-not-sized.rs
new file mode 100644
index 00000000000..bd25940780a
--- /dev/null
+++ b/src/test/ui/impl-trait/rpit-not-sized.rs
@@ -0,0 +1,6 @@
+fn foo() -> impl ?Sized {
+    //~^ ERROR the size for values of type `impl ?Sized` cannot be known at compilation time
+    ()
+}
+
+fn main() {}
diff --git a/src/test/ui/impl-trait/rpit-not-sized.stderr b/src/test/ui/impl-trait/rpit-not-sized.stderr
new file mode 100644
index 00000000000..608c94fc072
--- /dev/null
+++ b/src/test/ui/impl-trait/rpit-not-sized.stderr
@@ -0,0 +1,12 @@
+error[E0277]: the size for values of type `impl ?Sized` cannot be known at compilation time
+  --> $DIR/rpit-not-sized.rs:1:13
+   |
+LL | fn foo() -> impl ?Sized {
+   |             ^^^^^^^^^^^ doesn't have a size known at compile-time
+   |
+   = help: the trait `Sized` is not implemented for `impl ?Sized`
+   = note: the return type of a function must have a statically known size
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0277`.
diff --git a/src/test/ui/parser/issues/issue-19398.stderr b/src/test/ui/parser/issues/issue-19398.stderr
index bbd85374b4b..1da00960adf 100644
--- a/src/test/ui/parser/issues/issue-19398.stderr
+++ b/src/test/ui/parser/issues/issue-19398.stderr
@@ -4,15 +4,10 @@ error: expected `{`, found keyword `unsafe`
 LL | trait T {
    |         - while parsing this item list starting here
 LL |     extern "Rust" unsafe fn foo();
-   |     --------------^^^^^^
-   |     |             |
-   |     |             expected `{`
-   |     help: `unsafe` must come before `extern "Rust"`: `unsafe extern "Rust"`
+   |                   ^^^^^^ expected `{`
 LL |
 LL | }
    | - the item list ends here
-   |
-   = note: keyword order for functions declaration is `pub`, `default`, `const`, `async`, `unsafe`, `extern`
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/parser/unsafe-foreign-mod-2.rs b/src/test/ui/parser/unsafe-foreign-mod-2.rs
new file mode 100644
index 00000000000..77856fb6734
--- /dev/null
+++ b/src/test/ui/parser/unsafe-foreign-mod-2.rs
@@ -0,0 +1,8 @@
+extern "C" unsafe {
+               //~^ ERROR expected `{`, found keyword `unsafe`
+               //~| ERROR extern block cannot be declared unsafe
+    unsafe fn foo();
+        //~^ ERROR functions in `extern` blocks cannot have qualifiers
+}
+
+fn main() {}
diff --git a/src/test/ui/parser/unsafe-foreign-mod-2.stderr b/src/test/ui/parser/unsafe-foreign-mod-2.stderr
new file mode 100644
index 00000000000..7cc2de141ae
--- /dev/null
+++ b/src/test/ui/parser/unsafe-foreign-mod-2.stderr
@@ -0,0 +1,28 @@
+error: expected `{`, found keyword `unsafe`
+  --> $DIR/unsafe-foreign-mod-2.rs:1:12
+   |
+LL | extern "C" unsafe {
+   |            ^^^^^^ expected `{`
+
+error: extern block cannot be declared unsafe
+  --> $DIR/unsafe-foreign-mod-2.rs:1:12
+   |
+LL | extern "C" unsafe {
+   |            ^^^^^^
+
+error: functions in `extern` blocks cannot have qualifiers
+  --> $DIR/unsafe-foreign-mod-2.rs:4:15
+   |
+LL | extern "C" unsafe {
+   | ----------------- in this `extern` block
+...
+LL |     unsafe fn foo();
+   |               ^^^
+   |
+help: remove the qualifiers
+   |
+LL |     fn foo();
+   |     ~~
+
+error: aborting due to 3 previous errors
+