Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use smaller def span for functions #75465

Merged
merged 1 commit into from
Aug 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/librustc_ast/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_ast/mut_visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
15 changes: 12 additions & 3 deletions src/librustc_ast_lowering/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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)
})
}
Expand Down Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions src/librustc_builtin_macros/deriving/generic/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
2 changes: 1 addition & 1 deletion src/librustc_builtin_macros/global_allocator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_builtin_macros/test_harness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand Down
1 change: 1 addition & 0 deletions src/librustc_hir/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
29 changes: 26 additions & 3 deletions src/librustc_middle/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
27 changes: 20 additions & 7 deletions src/librustc_mir_build/build/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
..
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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>>,
Expand All @@ -585,7 +598,7 @@ where

let mut builder = Builder::new(
hir,
span,
span_with_body,
arguments.len(),
safety,
return_ty,
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir_build/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
18 changes: 14 additions & 4 deletions src/librustc_parse/parser/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let mut sig_hi = self.prev_token.span;
let sig_hi = if self.check(&token::Semi) {
// Include the trailing semicolon in the span of the signature
self.token.span
} else {
self.prev_token.span
};

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will introduce a duplicate check for token::Semi - I wanted to make sure that the determination of sig_hi always stays in sync with the actual parsing logic.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, fair enough it's not critical

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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
sig_hi: &mut Span,

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue as above

) -> 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;
Comment on lines +1518 to +1519
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// 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() {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_save_analysis/sig.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 ");
Expand Down
19 changes: 10 additions & 9 deletions src/librustc_trait_selection/traits/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_typeck/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
..
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Original file line number Diff line number Diff line change
Expand Up @@ -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

Original file line number Diff line number Diff line change
Expand Up @@ -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

Original file line number Diff line number Diff line change
Expand Up @@ -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

Original file line number Diff line number Diff line change
Expand Up @@ -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

Original file line number Diff line number Diff line change
@@ -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

Loading