Auto merge of #75465 - Aaron1011:feature/short-fn-def-span, r=estebank

Use smaller def span for functions

Currently, the def span of a function encompasses the entire function
signature and body. However, this is usually unnecessarily verbose - when we are
pointing at an entire function in a diagnostic, we almost always want to
point at the signature. The actual contents of the body tends to be
irrelevant to the diagnostic we are emitting, and just takes up
additional screen space.

This commit changes the `def_span` of all function items (freestanding
functions, `impl`-block methods, and `trait`-block methods) to be the
span of the signature. For example, the function

```rust
pub fn foo<T>(val: T) -> T { val }
```

now has a `def_span` corresponding to `pub fn foo<T>(val: T) -> T`
(everything before the opening curly brace).

Trait methods without a body have a `def_span` which includes the
trailing semicolon. For example:

```rust
trait Foo {
    fn bar();
}
```

the function definition `Foo::bar` has a `def_span` of `fn bar();`

This makes our diagnostic output much shorter, and emphasizes
information that is relevant to whatever diagnostic we are reporting.

We continue to use the full span (including the body) in a few of
places:

* MIR building uses the full span when building source scopes.
* 'Outlives suggestions' use the full span to sort the diagnostics being
  emitted.
* The `#[rustc_on_unimplemented(enclosing_scope="in this scope")]`
attribute points the entire scope body.

All of these cases work only with local items, so we don't need to
add anything extra to crate metadata.

src/librustc_ast/ast.rs

@@ -1671,6 +1671,7 @@ pub struct MutTy {
 pub struct FnSig {
     pub header: FnHeader,
     pub decl: P<FnDecl>,
+    pub span: Span,
 }
 
 #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]

src/librustc_ast/mut_visit.rs

@@ -363,9 +363,10 @@ pub fn visit_bounds<T: MutVisitor>(bounds: &mut GenericBounds, vis: &mut T) {
 }
 
 // No `noop_` prefix because there isn't a corresponding method in `MutVisitor`.
-pub fn visit_fn_sig<T: MutVisitor>(FnSig { header, decl }: &mut FnSig, vis: &mut T) {
+pub fn visit_fn_sig<T: MutVisitor>(FnSig { header, decl, span }: &mut FnSig, vis: &mut T) {
     vis.visit_fn_header(header);
     vis.visit_fn_decl(decl);
+    vis.visit_span(span);
 }
 
 // No `noop_` prefix because there isn't a corresponding method in `MutVisitor`.

src/librustc_ast_lowering/item.rs

@@ -263,7 +263,12 @@ impl<'hir> LoweringContext<'_, 'hir> {
                 let (ty, body_id) = self.lower_const_item(t, span, e.as_deref());
                 hir::ItemKind::Const(ty, body_id)
             }
-            ItemKind::Fn(_, FnSig { ref decl, header }, ref generics, ref body) => {
+            ItemKind::Fn(
+                _,
+                FnSig { ref decl, header, span: fn_sig_span },
+                ref generics,
+                ref body,
+            ) => {
                 let fn_def_id = self.resolver.local_def_id(id);
                 self.with_new_scopes(|this| {
                     this.current_item = Some(ident.span);
@@ -290,7 +295,11 @@ impl<'hir> LoweringContext<'_, 'hir> {
                             )
                         },
                     );
-                    let sig = hir::FnSig { decl, header: this.lower_fn_header(header) };
+                    let sig = hir::FnSig {
+                        decl,
+                        header: this.lower_fn_header(header),
+                        span: fn_sig_span,
+                    };
                     hir::ItemKind::Fn(sig, generics, body_id)
                 })
             }
@@ -1243,7 +1252,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
                 )
             },
         );
-        (generics, hir::FnSig { header, decl })
+        (generics, hir::FnSig { header, decl, span: sig.span })
     }
 
     fn lower_fn_header(&mut self, h: FnHeader) -> hir::FnHeader {

src/librustc_builtin_macros/deriving/generic/mod.rs

@@ -924,6 +924,7 @@ impl<'a> MethodDef<'a> {
         let sig = ast::FnSig {
             header: ast::FnHeader { unsafety, ext: ast::Extern::None, ..ast::FnHeader::default() },
             decl: fn_decl,
+            span: trait_.span,
         };
         let def = ast::Defaultness::Final;
 

src/librustc_builtin_macros/global_allocator.rs

@@ -67,7 +67,7 @@ impl AllocFnFactory<'_, '_> {
         let (output_ty, output_expr) = self.ret_ty(&method.output, result);
         let decl = self.cx.fn_decl(abi_args, ast::FnRetTy::Ty(output_ty));
         let header = FnHeader { unsafety: Unsafe::Yes(self.span), ..FnHeader::default() };
-        let sig = FnSig { decl, header };
+        let sig = FnSig { decl, header, span: self.span };
         let block = Some(self.cx.block_expr(output_expr));
         let kind = ItemKind::Fn(ast::Defaultness::Final, sig, Generics::default(), block);
         let item = self.cx.item(

src/librustc_builtin_macros/test_harness.rs

@@ -318,7 +318,7 @@ fn mk_main(cx: &mut TestCtxt<'_>) -> P<ast::Item> {
     };
 
     let decl = ecx.fn_decl(vec![], ast::FnRetTy::Ty(main_ret_ty));
-    let sig = ast::FnSig { decl, header: ast::FnHeader::default() };
+    let sig = ast::FnSig { decl, header: ast::FnHeader::default(), span: sp };
     let def = ast::Defaultness::Final;
     let main = ast::ItemKind::Fn(def, sig, ast::Generics::default(), Some(main_body));
 

src/librustc_hir/hir.rs

@@ -1851,6 +1851,7 @@ pub struct MutTy<'hir> {
 pub struct FnSig<'hir> {
     pub header: FnHeader,
     pub decl: &'hir FnDecl<'hir>,
+    pub span: Span,
 }
 
 // The bodies for items are stored "out of line", in a separate

src/librustc_middle/hir/map/mod.rs

@@ -828,13 +828,24 @@ impl<'hir> Map<'hir> {
         attrs.unwrap_or(&[])
     }
 
+    /// Gets the span of the definition of the specified HIR node.
+    /// This is used by `tcx.get_span`
     pub fn span(&self, hir_id: HirId) -> Span {
         match self.find_entry(hir_id).map(|entry| entry.node) {
             Some(Node::Param(param)) => param.span,
-            Some(Node::Item(item)) => item.span,
+            Some(Node::Item(item)) => match &item.kind {
+                ItemKind::Fn(sig, _, _) => sig.span,
+                _ => item.span,
+            },
             Some(Node::ForeignItem(foreign_item)) => foreign_item.span,
-            Some(Node::TraitItem(trait_method)) => trait_method.span,
-            Some(Node::ImplItem(impl_item)) => impl_item.span,
+            Some(Node::TraitItem(trait_item)) => match &trait_item.kind {
+                TraitItemKind::Fn(sig, _) => sig.span,
+                _ => trait_item.span,
+            },
+            Some(Node::ImplItem(impl_item)) => match &impl_item.kind {
+                ImplItemKind::Fn(sig, _) => sig.span,
+                _ => impl_item.span,
+            },
             Some(Node::Variant(variant)) => variant.span,
             Some(Node::Field(field)) => field.span,
             Some(Node::AnonConst(constant)) => self.body(constant.body).value.span,
@@ -866,6 +877,18 @@ impl<'hir> Map<'hir> {
         }
     }
 
+    /// Like `hir.span()`, but includes the body of function items
+    /// (instead of just the function header)
+    pub fn span_with_body(&self, hir_id: HirId) -> Span {
+        match self.find_entry(hir_id).map(|entry| entry.node) {
+            Some(Node::TraitItem(item)) => item.span,
+            Some(Node::ImplItem(impl_item)) => impl_item.span,
+            Some(Node::Item(item)) => item.span,
+            Some(_) => self.span(hir_id),
+            _ => bug!("hir::map::Map::span_with_body: id not in map: {:?}", hir_id),
+        }
+    }
+
     pub fn span_if_local(&self, id: DefId) -> Option<Span> {
         id.as_local().map(|id| self.span(self.local_def_id_to_hir_id(id)))
     }

src/librustc_mir/borrow_check/diagnostics/outlives_suggestion.rs

@@ -257,7 +257,7 @@ impl OutlivesSuggestionBuilder {
         };
 
         // We want this message to appear after other messages on the mir def.
-        let mir_span = mbcx.infcx.tcx.def_span(mbcx.mir_def_id);
+        let mir_span = mbcx.body.span;
         diag.sort_span = mir_span.shrink_to_hi();
 
         // Buffer the diagnostic

src/librustc_mir_build/build/mod.rs

@@ -37,22 +37,29 @@ fn mir_build(tcx: TyCtxt<'_>, def: ty::WithOptConstParam<LocalDefId>) -> Body<'_
     let id = tcx.hir().local_def_id_to_hir_id(def.did);
 
     // Figure out what primary body this item has.
-    let (body_id, return_ty_span) = match tcx.hir().get(id) {
+    let (body_id, return_ty_span, span_with_body) = match tcx.hir().get(id) {
         Node::Expr(hir::Expr { kind: hir::ExprKind::Closure(_, decl, body_id, _, _), .. }) => {
-            (*body_id, decl.output.span())
+            (*body_id, decl.output.span(), None)
         }
         Node::Item(hir::Item {
             kind: hir::ItemKind::Fn(hir::FnSig { decl, .. }, _, body_id),
+            span,
             ..
         })
         | Node::ImplItem(hir::ImplItem {
             kind: hir::ImplItemKind::Fn(hir::FnSig { decl, .. }, body_id),
+            span,
             ..
         })
         | Node::TraitItem(hir::TraitItem {
             kind: hir::TraitItemKind::Fn(hir::FnSig { decl, .. }, hir::TraitFn::Provided(body_id)),
+            span,
             ..
-        }) => (*body_id, decl.output.span()),
+        }) => {
+            // Use the `Span` of the `Item/ImplItem/TraitItem` as the body span,
+            // since the def span of a function does not include the body
+            (*body_id, decl.output.span(), Some(*span))
+        }
         Node::Item(hir::Item {
             kind: hir::ItemKind::Static(ty, _, body_id) | hir::ItemKind::Const(ty, body_id),
             ..
@@ -61,12 +68,16 @@ fn mir_build(tcx: TyCtxt<'_>, def: ty::WithOptConstParam<LocalDefId>) -> Body<'_
         | Node::TraitItem(hir::TraitItem {
             kind: hir::TraitItemKind::Const(ty, Some(body_id)),
             ..
-        }) => (*body_id, ty.span),
-        Node::AnonConst(hir::AnonConst { body, hir_id, .. }) => (*body, tcx.hir().span(*hir_id)),
+        }) => (*body_id, ty.span, None),
+        Node::AnonConst(hir::AnonConst { body, hir_id, .. }) => (*body, tcx.hir().span(*hir_id), None),
 
         _ => span_bug!(tcx.hir().span(id), "can't build MIR for {:?}", def.did),
     };
 
+    // If we don't have a specialized span for the body, just use the
+    // normal def span.
+    let span_with_body = span_with_body.unwrap_or_else(|| tcx.hir().span(id));
+
     tcx.infer_ctxt().enter(|infcx| {
         let cx = Cx::new(&infcx, def, id);
         let body = if let Some(ErrorReported) = cx.typeck_results().tainted_by_errors {
@@ -167,6 +178,7 @@ fn mir_build(tcx: TyCtxt<'_>, def: ty::WithOptConstParam<LocalDefId>) -> Body<'_
                 return_ty,
                 return_ty_span,
                 body,
+                span_with_body
             );
             mir.yield_ty = yield_ty;
             mir
@@ -571,6 +583,7 @@ fn construct_fn<'a, 'tcx, A>(
     return_ty: Ty<'tcx>,
     return_ty_span: Span,
     body: &'tcx hir::Body<'tcx>,
+    span_with_body: Span
 ) -> Body<'tcx>
 where
     A: Iterator<Item = ArgInfo<'tcx>>,
@@ -585,7 +598,7 @@ where
 
     let mut builder = Builder::new(
         hir,
-        span,
+        span_with_body,
         arguments.len(),
         safety,
         return_ty,
@@ -628,7 +641,7 @@ where
                 )
             );
             // Attribute epilogue to function's closing brace
-            let fn_end = span.shrink_to_hi();
+            let fn_end = span_with_body.shrink_to_hi();
             let source_info = builder.source_info(fn_end);
             let return_block = builder.return_block();
             builder.cfg.goto(block, source_info, return_block);

src/librustc_mir_build/lints.rs

@@ -38,7 +38,7 @@ crate fn check<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, def_id: LocalDefId) {
         vis.reachable_recursive_calls.sort();
 
         let hir_id = tcx.hir().local_def_id_to_hir_id(def_id);
-        let sp = tcx.sess.source_map().guess_head_span(tcx.hir().span(hir_id));
+        let sp = tcx.sess.source_map().guess_head_span(tcx.hir().span_with_body(hir_id));
         tcx.struct_span_lint_hir(UNCONDITIONAL_RECURSION, hir_id, sp, |lint| {
             let mut db = lint.build("function cannot return without recursing");
             db.span_label(sp, "cannot return without recursing");

src/librustc_parse/parser/item.rs

@@ -227,7 +227,7 @@ impl<'a> Parser<'a> {
             (Ident::invalid(), ItemKind::Use(P(tree)))
         } else if self.check_fn_front_matter() {
             // FUNCTION ITEM
-            let (ident, sig, generics, body) = self.parse_fn(attrs, req_name)?;
+            let (ident, sig, generics, body) = self.parse_fn(attrs, req_name, lo)?;
             (ident, ItemKind::Fn(def(), sig, generics, body))
         } else if self.eat_keyword(kw::Extern) {
             if self.eat_keyword(kw::Crate) {
@@ -1492,21 +1492,31 @@ impl<'a> Parser<'a> {
         &mut self,
         attrs: &mut Vec<Attribute>,
         req_name: ReqName,
+        sig_lo: Span,
     ) -> PResult<'a, (Ident, FnSig, Generics, Option<P<Block>>)> {
         let header = self.parse_fn_front_matter()?; // `const ... fn`
         let ident = self.parse_ident()?; // `foo`
         let mut generics = self.parse_generics()?; // `<'a, T, ...>`
         let decl = self.parse_fn_decl(req_name, AllowPlus::Yes)?; // `(p: u8, ...)`
         generics.where_clause = self.parse_where_clause()?; // `where T: Ord`
-        let body = self.parse_fn_body(attrs)?; // `;` or `{ ... }`.
-        Ok((ident, FnSig { header, decl }, generics, body))
+
+        let mut sig_hi = self.prev_token.span;
+        let body = self.parse_fn_body(attrs, &mut sig_hi)?; // `;` or `{ ... }`.
+        let fn_sig_span = sig_lo.to(sig_hi);
+        Ok((ident, FnSig { header, decl, span: fn_sig_span }, generics, body))
     }
 
     /// Parse the "body" of a function.
     /// This can either be `;` when there's no body,
     /// or e.g. a block when the function is a provided one.
-    fn parse_fn_body(&mut self, attrs: &mut Vec<Attribute>) -> PResult<'a, Option<P<Block>>> {
+    fn parse_fn_body(
+        &mut self,
+        attrs: &mut Vec<Attribute>,
+        sig_hi: &mut Span,
+    ) -> PResult<'a, Option<P<Block>>> {
         let (inner_attrs, body) = if self.check(&token::Semi) {
+            // Include the trailing semicolon in the span of the signature
+            *sig_hi = self.token.span;
             self.bump(); // `;`
             (Vec::new(), None)
         } else if self.check(&token::OpenDelim(token::Brace)) || self.token.is_whole_block() {

src/librustc_save_analysis/sig.rs

@@ -377,7 +377,7 @@ impl<'hir> Sig for hir::Item<'hir> {
 
                 Ok(extend_sig(ty, text, defs, vec![]))
             }
-            hir::ItemKind::Fn(hir::FnSig { ref decl, header }, ref generics, _) => {
+            hir::ItemKind::Fn(hir::FnSig { ref decl, header, span: _ }, ref generics, _) => {
                 let mut text = String::new();
                 if let hir::Constness::Const = header.constness {
                     text.push_str("const ");

src/librustc_trait_selection/traits/error_reporting/mod.rs

@@ -399,16 +399,17 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
                             err.note(s.as_str());
                         }
                         if let Some(ref s) = enclosing_scope {
-                            let enclosing_scope_span = tcx.def_span(
-                                tcx.hir()
-                                    .opt_local_def_id(obligation.cause.body_id)
-                                    .unwrap_or_else(|| {
-                                        tcx.hir().body_owner_def_id(hir::BodyId {
-                                            hir_id: obligation.cause.body_id,
-                                        })
+                            let body = tcx
+                                .hir()
+                                .opt_local_def_id(obligation.cause.body_id)
+                                .unwrap_or_else(|| {
+                                    tcx.hir().body_owner_def_id(hir::BodyId {
+                                        hir_id: obligation.cause.body_id,
                                     })
-                                    .to_def_id(),
-                            );
+                                });
+
+                            let enclosing_scope_span =
+                                tcx.hir().span_with_body(tcx.hir().local_def_id_to_hir_id(body));
 
                             err.span_label(enclosing_scope_span, s.as_str());
                         }

src/librustc_typeck/collect.rs

@@ -1543,7 +1543,7 @@ fn fn_sig(tcx: TyCtxt<'_>, def_id: DefId) -> ty::PolyFnSig<'_> {
         }
 
         TraitItem(hir::TraitItem {
-            kind: TraitItemKind::Fn(FnSig { header, decl }, _),
+            kind: TraitItemKind::Fn(FnSig { header, decl, span: _ }, _),
             ident,
             generics,
             ..

src/test/ui/associated-types/bound-lifetime-in-binding-only.ok.stderr

@@ -2,7 +2,7 @@ error: fatal error triggered by #[rustc_error]
   --> $DIR/bound-lifetime-in-binding-only.rs:71:1
    |
 LL | fn main() { }
-   | ^^^^^^^^^^^^^
+   | ^^^^^^^^^
 
 error: aborting due to previous error
 

src/test/ui/associated-types/bound-lifetime-in-return-only.ok.stderr

@@ -2,7 +2,7 @@ error: fatal error triggered by #[rustc_error]
   --> $DIR/bound-lifetime-in-return-only.rs:49:1
    |
 LL | fn main() { }
-   | ^^^^^^^^^^^^^
+   | ^^^^^^^^^
 
 error: aborting due to previous error
 

src/test/ui/associated-types/cache/project-fn-ret-contravariant.ok.stderr

@@ -2,7 +2,7 @@ error: fatal error triggered by #[rustc_error]
   --> $DIR/project-fn-ret-contravariant.rs:50:1
    |
 LL | fn main() { }
-   | ^^^^^^^^^^^^^
+   | ^^^^^^^^^
 
 error: aborting due to previous error
 

src/test/ui/associated-types/cache/project-fn-ret-contravariant.oneuse.stderr

@@ -2,7 +2,7 @@ error: fatal error triggered by #[rustc_error]
   --> $DIR/project-fn-ret-contravariant.rs:50:1
    |
 LL | fn main() { }
-   | ^^^^^^^^^^^^^
+   | ^^^^^^^^^
 
 error: aborting due to previous error
 

src/test/ui/associated-types/cache/project-fn-ret-invariant.ok.stderr

@@ -2,7 +2,7 @@ error: fatal error triggered by #[rustc_error]
   --> $DIR/project-fn-ret-invariant.rs:60:1
    |
 LL | fn main() {}
-   | ^^^^^^^^^^^^
+   | ^^^^^^^^^
 
 error: aborting due to previous error
 

src/test/ui/associated-types/higher-ranked-projection.good.stderr

@@ -1,11 +1,8 @@
 error: fatal error triggered by #[rustc_error]
   --> $DIR/higher-ranked-projection.rs:24:1
    |
-LL | / fn main() {
-LL | |     foo(());
-LL | |
-LL | | }
-   | |_^
+LL | fn main() {
+   | ^^^^^^^^^
 
 error: aborting due to previous error