Auto merge of #129202 - matthiaskrgr:rollup-wev7uur, r=matthiaskrgr

Rollup of 6 pull requests

Successful merges:

 - #128989 (Emit an error for invalid use of the linkage attribute)
 - #129167 (mir/pretty: use `Option` instead of `Either<Once, Empty>`)
 - #129168 (Return correct HirId when finding body owner in diagnostics)
 - #129191 (rustdoc-json: Clean up serialization and printing.)
 - #129192 (Remove useless attributes in merged doctest generated code)
 - #129196 (Remove a useless ref/id/ref round-trip from `pattern_from_hir`)

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

Author: bors

Diff for: compiler/rustc_middle/src/hir/map/mod.rs

@@ -554,53 +554,43 @@ impl<'hir> Map<'hir> {
     /// }
     /// ```
     pub fn get_fn_id_for_return_block(self, id: HirId) -> Option<HirId> {
-        let mut iter = self.parent_iter(id).peekable();
-        let mut ignore_tail = false;
-        if let Node::Expr(Expr { kind: ExprKind::Ret(_), .. }) = self.tcx.hir_node(id) {
-            // When dealing with `return` statements, we don't care about climbing only tail
-            // expressions.
-            ignore_tail = true;
-        }
+        let enclosing_body_owner = self.tcx.local_def_id_to_hir_id(self.enclosing_body_owner(id));
+
+        // Return `None` if the `id` expression is not the returned value of the enclosing body
+        let mut iter = [id].into_iter().chain(self.parent_id_iter(id)).peekable();
+        while let Some(cur_id) = iter.next() {
+            if enclosing_body_owner == cur_id {
+                break;
+            }
+
+            // A return statement is always the value returned from the enclosing body regardless of
+            // what the parent expressions are.
+            if let Node::Expr(Expr { kind: ExprKind::Ret(_), .. }) = self.tcx.hir_node(cur_id) {
+                break;
+            }
 
-        let mut prev_hir_id = None;
-        while let Some((hir_id, node)) = iter.next() {
-            if let (Some((_, next_node)), false) = (iter.peek(), ignore_tail) {
-                match next_node {
-                    Node::Block(Block { expr: None, .. }) => return None,
-                    // The current node is not the tail expression of its parent.
-                    Node::Block(Block { expr: Some(e), .. }) if hir_id != e.hir_id => return None,
+            // If the current expression's value doesnt get used as the parent expressions value then return `None`
+            if let Some(&parent_id) = iter.peek() {
+                match self.tcx.hir_node(parent_id) {
+                    // The current node is not the tail expression of the block expression parent expr.
+                    Node::Block(Block { expr: Some(e), .. }) if cur_id != e.hir_id => return None,
                     Node::Block(Block { expr: Some(e), .. })
                         if matches!(e.kind, ExprKind::If(_, _, None)) =>
                     {
                         return None;
                     }
+
+                    // The current expression's value does not pass up through these parent expressions
+                    Node::Block(Block { expr: None, .. })
+                    | Node::Expr(Expr { kind: ExprKind::Loop(..), .. })
+                    | Node::LetStmt(..) => return None,
+
                     _ => {}
                 }
             }
-            match node {
-                Node::Item(_)
-                | Node::ForeignItem(_)
-                | Node::TraitItem(_)
-                | Node::Expr(Expr { kind: ExprKind::Closure(_), .. })
-                | Node::ImplItem(_)
-                    // The input node `id` must be enclosed in the method's body as opposed
-                    // to some other place such as its return type (fixes #114918).
-                    // We verify that indirectly by checking that the previous node is the
-                    // current node's body
-                    if node.body_id().map(|b| b.hir_id) == prev_hir_id =>  {
-                        return Some(hir_id)
-                }
-                // Ignore `return`s on the first iteration
-                Node::Expr(Expr { kind: ExprKind::Loop(..) | ExprKind::Ret(..), .. })
-                | Node::LetStmt(_) => {
-                    return None;
-                }
-                _ => {}
-            }
-
-            prev_hir_id = Some(hir_id);
         }
-        None
+
+        Some(enclosing_body_owner)
     }
 
     /// Retrieves the `OwnerId` for `id`'s parent item, or `id` itself if no

Diff for: compiler/rustc_middle/src/mir/pretty.rs

@@ -1418,21 +1418,19 @@ pub fn write_allocations<'tcx>(
         alloc.inner().provenance().ptrs().values().map(|p| p.alloc_id())
     }
 
-    fn alloc_ids_from_const_val(val: ConstValue<'_>) -> impl Iterator<Item = AllocId> + '_ {
+    fn alloc_id_from_const_val(val: ConstValue<'_>) -> Option<AllocId> {
         match val {
-            ConstValue::Scalar(interpret::Scalar::Ptr(ptr, _)) => {
-                Either::Left(std::iter::once(ptr.provenance.alloc_id()))
-            }
-            ConstValue::Scalar(interpret::Scalar::Int { .. }) => Either::Right(std::iter::empty()),
-            ConstValue::ZeroSized => Either::Right(std::iter::empty()),
+            ConstValue::Scalar(interpret::Scalar::Ptr(ptr, _)) => Some(ptr.provenance.alloc_id()),
+            ConstValue::Scalar(interpret::Scalar::Int { .. }) => None,
+            ConstValue::ZeroSized => None,
             ConstValue::Slice { .. } => {
                 // `u8`/`str` slices, shouldn't contain pointers that we want to print.
-                Either::Right(std::iter::empty())
+                None
             }
             ConstValue::Indirect { alloc_id, .. } => {
                 // FIXME: we don't actually want to print all of these, since some are printed nicely directly as values inline in MIR.
                 // Really we'd want `pretty_print_const_value` to decide which allocations to print, instead of having a separate visitor.
-                Either::Left(std::iter::once(alloc_id))
+                Some(alloc_id)
             }
         }
     }
@@ -1443,7 +1441,9 @@ pub fn write_allocations<'tcx>(
             match c.const_ {
                 Const::Ty(_, _) | Const::Unevaluated(..) => {}
                 Const::Val(val, _) => {
-                    self.0.extend(alloc_ids_from_const_val(val));
+                    if let Some(id) = alloc_id_from_const_val(val) {
+                        self.0.insert(id);
+                    }
                 }
             }
         }

Diff for: compiler/rustc_mir_build/src/thir/cx/mod.rs

@@ -8,7 +8,7 @@ use rustc_hir as hir;
 use rustc_hir::def::DefKind;
 use rustc_hir::def_id::{DefId, LocalDefId};
 use rustc_hir::lang_items::LangItem;
-use rustc_hir::{HirId, Node};
+use rustc_hir::HirId;
 use rustc_middle::bug;
 use rustc_middle::middle::region;
 use rustc_middle::thir::*;
@@ -110,11 +110,7 @@ impl<'tcx> Cx<'tcx> {
     }
 
     #[instrument(level = "debug", skip(self))]
-    fn pattern_from_hir(&mut self, p: &hir::Pat<'_>) -> Box<Pat<'tcx>> {
-        let p = match self.tcx.hir_node(p.hir_id) {
-            Node::Pat(p) => p,
-            node => bug!("pattern became {:?}", node),
-        };
+    fn pattern_from_hir(&mut self, p: &'tcx hir::Pat<'tcx>) -> Box<Pat<'tcx>> {
         pat_from_hir(self.tcx, self.param_env, self.typeck_results(), p)
     }
 

Diff for: compiler/rustc_passes/messages.ftl

@@ -430,6 +430,10 @@ passes_link_section =
     .warn = {-passes_previously_accepted}
     .label = not a function or static
 
+passes_linkage =
+    attribute should be applied to a function or static
+    .label = not a function definition or static
+
 passes_macro_export =
     `#[macro_export]` only has an effect on macro definitions
 

Diff for: compiler/rustc_passes/src/check_attr.rs

@@ -243,6 +243,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
                 [sym::coroutine, ..] => {
                     self.check_coroutine(attr, target);
                 }
+                [sym::linkage, ..] => self.check_linkage(attr, span, target),
                 [
                     // ok
                     sym::allow
@@ -256,7 +257,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
                     | sym::cfi_encoding // FIXME(cfi_encoding)
                     | sym::may_dangle // FIXME(dropck_eyepatch)
                     | sym::pointee // FIXME(derive_smart_pointer)
-                    | sym::linkage // FIXME(linkage)
                     | sym::omit_gdb_pretty_printer_section // FIXME(omit_gdb_pretty_printer_section)
                     | sym::used // handled elsewhere to restrict to static items
                     | sym::repr // handled elsewhere to restrict to type decls items
@@ -2347,6 +2347,19 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
             }
         }
     }
+
+    fn check_linkage(&self, attr: &Attribute, span: Span, target: Target) {
+        match target {
+            Target::Fn
+            | Target::Method(..)
+            | Target::Static
+            | Target::ForeignStatic
+            | Target::ForeignFn => {}
+            _ => {
+                self.dcx().emit_err(errors::Linkage { attr_span: attr.span, span });
+            }
+        }
+    }
 }
 
 impl<'tcx> Visitor<'tcx> for CheckAttrVisitor<'tcx> {

Diff for: compiler/rustc_passes/src/errors.rs

@@ -643,6 +643,15 @@ pub struct CoroutineOnNonClosure {
     pub span: Span,
 }
 
+#[derive(Diagnostic)]
+#[diag(passes_linkage)]
+pub struct Linkage {
+    #[primary_span]
+    pub attr_span: Span,
+    #[label]
+    pub span: Span,
+}
+
 #[derive(Diagnostic)]
 #[diag(passes_empty_confusables)]
 pub(crate) struct EmptyConfusables {

Diff for: src/librustdoc/doctest/runner.rs

@@ -75,7 +75,6 @@ impl DocTestRunner {
 #![allow(internal_features)]
 #![feature(test)]
 #![feature(rustc_attrs)]
-#![feature(coverage_attribute)]
 "
         .to_string();
 
@@ -135,7 +134,6 @@ mod __doctest_mod {{
 }}
 
 #[rustc_main]
-#[coverage(off)]
 fn main() -> std::process::ExitCode {{
 const TESTS: [test::TestDescAndFn; {nb_tests}] = [{ids}];
 let bin_marker = std::ffi::OsStr::new(__doctest_mod::BIN_OPTION);
@@ -235,11 +233,9 @@ fn main() {returns_result} {{
     writeln!(
         output,
         "
-#[rustc_test_marker = {test_name:?}]
 pub const TEST: test::TestDescAndFn = test::TestDescAndFn::new_doctest(
 {test_name:?}, {ignore}, {file:?}, {line}, {no_run}, {should_panic},
 test::StaticTestFn(
-    #[coverage(off)]
     || {{{runner}}},
 ));
 }}",

Diff for: src/librustdoc/json/mod.rs

@@ -39,8 +39,10 @@ pub(crate) struct JsonRenderer<'tcx> {
     /// A mapping of IDs that contains all local items for this crate which gets output as a top
     /// level field of the JSON blob.
     index: Rc<RefCell<FxHashMap<types::Id, types::Item>>>,
-    /// The directory where the blob will be written to.
-    out_path: Option<PathBuf>,
+    /// The directory where the JSON blob should be written to.
+    ///
+    /// If this is `None`, the blob will be printed to `stdout` instead.
+    out_dir: Option<PathBuf>,
     cache: Rc<Cache>,
     imported_items: DefIdSet,
 }
@@ -101,18 +103,20 @@ impl<'tcx> JsonRenderer<'tcx> {
             .unwrap_or_default()
     }
 
-    fn write<T: Write>(
+    fn serialize_and_write<T: Write>(
         &self,
-        output: types::Crate,
+        output_crate: types::Crate,
         mut writer: BufWriter<T>,
         path: &str,
     ) -> Result<(), Error> {
-        self.tcx
-            .sess
-            .time("rustdoc_json_serialization", || serde_json::ser::to_writer(&mut writer, &output))
-            .unwrap();
-        try_err!(writer.flush(), path);
-        Ok(())
+        self.sess().time("rustdoc_json_serialize_and_write", || {
+            try_err!(
+                serde_json::ser::to_writer(&mut writer, &output_crate).map_err(|e| e.to_string()),
+                path
+            );
+            try_err!(writer.flush(), path);
+            Ok(())
+        })
     }
 }
 
@@ -137,7 +141,7 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> {
             JsonRenderer {
                 tcx,
                 index: Rc::new(RefCell::new(FxHashMap::default())),
-                out_path: if options.output_to_stdout { None } else { Some(options.output) },
+                out_dir: if options.output_to_stdout { None } else { Some(options.output) },
                 cache: Rc::new(cache),
                 imported_items,
             },
@@ -237,7 +241,7 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> {
         let index = (*self.index).clone().into_inner();
 
         debug!("Constructing Output");
-        let output = types::Crate {
+        let output_crate = types::Crate {
             root: types::Id(format!("0:0:{}", e.name(self.tcx).as_u32())),
             crate_version: self.cache.crate_version.clone(),
             includes_private: self.cache.document_private,
@@ -278,20 +282,20 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> {
                 .collect(),
             format_version: types::FORMAT_VERSION,
         };
-        if let Some(ref out_path) = self.out_path {
-            let out_dir = out_path.clone();
+        if let Some(ref out_dir) = self.out_dir {
             try_err!(create_dir_all(&out_dir), out_dir);
 
-            let mut p = out_dir;
-            p.push(output.index.get(&output.root).unwrap().name.clone().unwrap());
+            let mut p = out_dir.clone();
+            p.push(output_crate.index.get(&output_crate.root).unwrap().name.clone().unwrap());
             p.set_extension("json");
-            self.write(
-                output,
+
+            self.serialize_and_write(
+                output_crate,
                 BufWriter::new(try_err!(File::create(&p), p)),
                 &p.display().to_string(),
             )
         } else {
-            self.write(output, BufWriter::new(stdout()), "<stdout>")
+            self.serialize_and_write(output_crate, BufWriter::new(stdout().lock()), "<stdout>")
         }
     }
 

Diff for: tests/crashes/128810.rs

@@ -0,0 +1,42 @@
+#![feature(linkage)]
+#![feature(stmt_expr_attributes)]
+#![deny(unused_attributes)]
+#![allow(dead_code)]
+
+#[linkage = "weak"] //~ ERROR attribute should be applied to a function or static
+type InvalidTy = ();
+
+#[linkage = "weak"] //~ ERROR attribute should be applied to a function or static
+mod invalid_module {}
+
+#[linkage = "weak"] //~ ERROR attribute should be applied to a function or static
+struct F;
+
+#[linkage = "weak"] //~ ERROR attribute should be applied to a function or static
+impl F {
+    #[linkage = "weak"]
+    fn valid(&self) {}
+}
+
+#[linkage = "weak"]
+fn f() {
+    #[linkage = "weak"]
+    {
+        1
+    };
+    //~^^^^ ERROR attribute should be applied to a function or static
+}
+
+extern "C" {
+    #[linkage = "weak"]
+    static A: *const ();
+
+    #[linkage = "weak"]
+    fn bar();
+}
+
+fn main() {
+    let _ = #[linkage = "weak"]
+    (|| 1);
+    //~^^ ERROR attribute should be applied to a function or static
+}