Skip to content

Commit 61a551b

Browse files
committedNov 6, 2019
Auto merge of #65830 - Quantumplation:master, r=davidtwco,estebank
Use ident.span instead of def_span in dead-code pass Hello! First time contributor! :) This should fix #58729. According to @estebank in the duplicate #63064, def_span scans forward on the line until it finds a {, and if it can't find one, falls back to the span for the whole item. This was apparently written before the identifier span was explicitly tracked on each node. This means that if an unused function signature spans multiple lines, the entire function (potentially hundreds of lines) gets flagged as dead code. This could, for example, cause IDEs to add error squiggly's to the whole function. By using the span from the ident instead, we narrow the scope of this in most cases. In a wider sense, it's probably safe to use ident.span instead of def_span in most locations throughout the whole code base, but since this is my first contribution, I kept it small. Some interesting points that came up while I was working on this: - I reorganized the tests a bit to bring some of the dead code ones all into the same location - A few tests were for things unrelated to dead code (like the path-lookahead for parens), so I added #![allow(dead_code)] and cleaned up the stderr file to reduce noise in the future - The same fix doesn't apply to const and static declarations. I tried adding these cases to the match expression, but that created a much wider change to tests and error messages, so I left it off until I could get some code review to validate the approach.
2 parents e8b190a + 6186ede commit 61a551b

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

49 files changed

+139
-109
lines changed
 

‎src/librustc_passes/dead.rs

+14-3
Original file line numberDiff line numberDiff line change
@@ -569,16 +569,27 @@ impl Visitor<'tcx> for DeadVisitor<'tcx> {
569569

570570
fn visit_item(&mut self, item: &'tcx hir::Item) {
571571
if self.should_warn_about_item(item) {
572-
// For items that have a definition with a signature followed by a
573-
// block, point only at the signature.
572+
// For most items, we want to highlight its identifier
574573
let span = match item.kind {
575574
hir::ItemKind::Fn(..) |
576575
hir::ItemKind::Mod(..) |
577576
hir::ItemKind::Enum(..) |
578577
hir::ItemKind::Struct(..) |
579578
hir::ItemKind::Union(..) |
580579
hir::ItemKind::Trait(..) |
581-
hir::ItemKind::Impl(..) => self.tcx.sess.source_map().def_span(item.span),
580+
hir::ItemKind::Impl(..) => {
581+
// FIXME(66095): Because item.span is annotated with things
582+
// like expansion data, and ident.span isn't, we use the
583+
// def_span method if it's part of a macro invocation
584+
// (and thus has asource_callee set).
585+
// We should probably annotate ident.span with the macro
586+
// context, but that's a larger change.
587+
if item.span.source_callee().is_some() {
588+
self.tcx.sess.source_map().def_span(item.span)
589+
} else {
590+
item.ident.span
591+
}
592+
},
582593
_ => item.span,
583594
};
584595
let participle = match item.kind {

‎src/test/ui-fulldeps/lint-plugin-cmdline-allow.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,10 @@ LL | #![plugin(lint_plugin_test)]
77
= note: `#[warn(deprecated)]` on by default
88

99
warning: function is never used: `lintme`
10-
--> $DIR/lint-plugin-cmdline-allow.rs:10:1
10+
--> $DIR/lint-plugin-cmdline-allow.rs:10:4
1111
|
1212
LL | fn lintme() { }
13-
| ^^^^^^^^^^^
13+
| ^^^^^^
1414
|
1515
note: lint level defined here
1616
--> $DIR/lint-plugin-cmdline-allow.rs:7:9

0 commit comments

Comments
 (0)
Please sign in to comment.