Skip to content

Commit 98e4b68

Browse files
committed
Auto merge of #45709 - nrc:rls-bugs-2, r=eddyb
Fix a bunch of minor save-analysis bugs r? @eddyb
2 parents dcd343b + 5d3be12 commit 98e4b68

File tree

2 files changed

+112
-65
lines changed

2 files changed

+112
-65
lines changed

src/librustc_save_analysis/dump_visitor.rs

+60-47
Original file line numberDiff line numberDiff line change
@@ -322,25 +322,24 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
322322
let mut collector = PathCollector::new();
323323
collector.visit_pat(&arg.pat);
324324
let span_utils = self.span.clone();
325-
for &(id, ref p, ..) in &collector.collected_paths {
325+
326+
for (id, i, sp, ..) in collector.collected_idents {
326327
let hir_id = self.tcx.hir.node_to_hir_id(id);
327328
let typ = match self.save_ctxt.tables.node_id_to_type_opt(hir_id) {
328329
Some(s) => s.to_string(),
329330
None => continue,
330331
};
331-
// get the span only for the name of the variable (I hope the path is only ever a
332-
// variable name, but who knows?)
333-
let sub_span = span_utils.span_for_last_ident(p.span);
334-
if !self.span.filter_generated(sub_span, p.span) {
332+
let sub_span = span_utils.span_for_last_ident(sp);
333+
if !self.span.filter_generated(sub_span, sp) {
335334
let id = ::id_from_node_id(id, &self.save_ctxt);
336335
let span = self.span_from_span(sub_span.expect("No span found for variable"));
337336

338337
self.dumper.dump_def(false, Def {
339338
kind: DefKind::Local,
340339
id,
341340
span,
342-
name: path_to_string(p),
343-
qualname: format!("{}::{}", qualname, path_to_string(p)),
341+
name: i.to_string(),
342+
qualname: format!("{}::{}", qualname, i.to_string()),
344343
value: typ,
345344
parent: None,
346345
children: vec![],
@@ -395,14 +394,6 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
395394
}
396395
}
397396

398-
fn process_trait_ref(&mut self, trait_ref: &'l ast::TraitRef) {
399-
let trait_ref_data = self.save_ctxt.get_trait_ref_data(trait_ref);
400-
if let Some(trait_ref_data) = trait_ref_data {
401-
self.dumper.dump_ref(trait_ref_data);
402-
}
403-
self.process_path(trait_ref.ref_id, &trait_ref.path);
404-
}
405-
406397
fn process_struct_field_def(&mut self, field: &ast::StructField, parent_id: NodeId) {
407398
let field_data = self.save_ctxt.get_field_data(field, parent_id);
408399
if let Some(field_data) = field_data {
@@ -792,7 +783,8 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
792783
}
793784
}
794785

795-
fn process_path(&mut self, id: NodeId, path: &ast::Path) {
786+
fn process_path(&mut self, id: NodeId, path: &'l ast::Path) {
787+
debug!("process_path {:?}", path);
796788
let path_data = self.save_ctxt.get_path_data(id, path);
797789
if generated_code(path.span) && path_data.is_none() {
798790
return;
@@ -807,6 +799,27 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
807799

808800
self.dumper.dump_ref(path_data);
809801

802+
// Type parameters
803+
for seg in &path.segments {
804+
if let Some(ref params) = seg.parameters {
805+
match **params {
806+
ast::PathParameters::AngleBracketed(ref data) => {
807+
for t in &data.types {
808+
self.visit_ty(t);
809+
}
810+
}
811+
ast::PathParameters::Parenthesized(ref data) => {
812+
for t in &data.inputs {
813+
self.visit_ty(t);
814+
}
815+
if let Some(ref t) = data.output {
816+
self.visit_ty(t);
817+
}
818+
}
819+
}
820+
}
821+
}
822+
810823
// Modules or types in the path prefix.
811824
match self.save_ctxt.get_path_def(id) {
812825
HirDef::Method(did) => {
@@ -859,14 +872,26 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
859872
walk_list!(self, visit_expr, base);
860873
}
861874

862-
fn process_method_call(&mut self, ex: &'l ast::Expr, args: &'l [P<ast::Expr>]) {
875+
fn process_method_call(&mut self,
876+
ex: &'l ast::Expr,
877+
seg: &'l ast::PathSegment,
878+
args: &'l [P<ast::Expr>]) {
863879
if let Some(mcd) = self.save_ctxt.get_expr_data(ex) {
864880
down_cast_data!(mcd, RefData, ex.span);
865881
if !generated_code(ex.span) {
866882
self.dumper.dump_ref(mcd);
867883
}
868884
}
869885

886+
// Explicit types in the turbo-fish.
887+
if let Some(ref params) = seg.parameters {
888+
if let ast::PathParameters::AngleBracketed(ref data) = **params {
889+
for t in &data.types {
890+
self.visit_ty(t);
891+
}
892+
}
893+
}
894+
870895
// walk receiver and args
871896
walk_list!(self, visit_expr, args);
872897
}
@@ -913,7 +938,7 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
913938
collector.visit_pat(&p);
914939
self.visit_pat(&p);
915940

916-
for &(id, ref p, immut) in &collector.collected_paths {
941+
for (id, i, sp, immut) in collector.collected_idents {
917942
let mut value = match immut {
918943
ast::Mutability::Immutable => value.to_string(),
919944
_ => String::new(),
@@ -933,18 +958,18 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
933958

934959
// Get the span only for the name of the variable (I hope the path
935960
// is only ever a variable name, but who knows?).
936-
let sub_span = self.span.span_for_last_ident(p.span);
961+
let sub_span = self.span.span_for_last_ident(sp);
937962
// Rust uses the id of the pattern for var lookups, so we'll use it too.
938-
if !self.span.filter_generated(sub_span, p.span) {
939-
let qualname = format!("{}${}", path_to_string(p), id);
963+
if !self.span.filter_generated(sub_span, sp) {
964+
let qualname = format!("{}${}", i.to_string(), id);
940965
let id = ::id_from_node_id(id, &self.save_ctxt);
941966
let span = self.span_from_span(sub_span.expect("No span found for variable"));
942967

943968
self.dumper.dump_def(false, Def {
944969
kind: DefKind::Local,
945970
id,
946971
span,
947-
name: path_to_string(p),
972+
name: i.to_string(),
948973
qualname,
949974
value: typ,
950975
parent: None,
@@ -1274,7 +1299,7 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> Visitor<'l> for DumpVisitor<'l, 'tc
12741299
for param in generics.ty_params.iter() {
12751300
for bound in param.bounds.iter() {
12761301
if let ast::TraitTyParamBound(ref trait_ref, _) = *bound {
1277-
self.process_trait_ref(&trait_ref.trait_ref);
1302+
self.process_path(trait_ref.trait_ref.ref_id, &trait_ref.trait_ref.path)
12781303
}
12791304
}
12801305
if let Some(ref ty) = param.default {
@@ -1329,7 +1354,7 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> Visitor<'l> for DumpVisitor<'l, 'tc
13291354
let def = self.save_ctxt.get_path_def(hir_expr.id);
13301355
self.process_struct_lit(ex, path, fields, adt.variant_of_def(def), base)
13311356
}
1332-
ast::ExprKind::MethodCall(.., ref args) => self.process_method_call(ex, args),
1357+
ast::ExprKind::MethodCall(ref seg, ref args) => self.process_method_call(ex, seg, args),
13331358
ast::ExprKind::Field(ref sub_ex, _) => {
13341359
self.visit_expr(&sub_ex);
13351360

@@ -1401,15 +1426,15 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> Visitor<'l> for DumpVisitor<'l, 'tc
14011426
let value = self.span.snippet(subexpression.span);
14021427
self.process_var_decl(pattern, value);
14031428
debug!("for loop, walk sub-expr: {:?}", subexpression.node);
1404-
visit::walk_expr(self, subexpression);
1429+
self.visit_expr(subexpression);
14051430
visit::walk_block(self, block);
14061431
}
14071432
ast::ExprKind::IfLet(ref pattern, ref subexpression, ref block, ref opt_else) => {
14081433
let value = self.span.snippet(subexpression.span);
14091434
self.process_var_decl(pattern, value);
1410-
visit::walk_expr(self, subexpression);
1435+
self.visit_expr(subexpression);
14111436
visit::walk_block(self, block);
1412-
opt_else.as_ref().map(|el| visit::walk_expr(self, el));
1437+
opt_else.as_ref().map(|el| self.visit_expr(el));
14131438
}
14141439
ast::ExprKind::Repeat(ref element, ref count) => {
14151440
self.visit_expr(element);
@@ -1441,15 +1466,12 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> Visitor<'l> for DumpVisitor<'l, 'tc
14411466
self.visit_pat(&pattern);
14421467
}
14431468

1444-
// This is to get around borrow checking, because we need mut self to call process_path.
1445-
let mut paths_to_process = vec![];
1446-
14471469
// process collected paths
1448-
for &(id, ref p, immut) in &collector.collected_paths {
1470+
for (id, i, sp, immut) in collector.collected_idents {
14491471
match self.save_ctxt.get_path_def(id) {
14501472
HirDef::Local(id) => {
14511473
let mut value = if immut == ast::Mutability::Immutable {
1452-
self.span.snippet(p.span).to_string()
1474+
self.span.snippet(sp).to_string()
14531475
} else {
14541476
"<mutable>".to_string()
14551477
};
@@ -1462,18 +1484,16 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> Visitor<'l> for DumpVisitor<'l, 'tc
14621484
value.push_str(": ");
14631485
value.push_str(&typ);
14641486

1465-
assert!(p.segments.len() == 1,
1466-
"qualified path for local variable def in arm");
1467-
if !self.span.filter_generated(Some(p.span), p.span) {
1468-
let qualname = format!("{}${}", path_to_string(p), id);
1487+
if !self.span.filter_generated(Some(sp), sp) {
1488+
let qualname = format!("{}${}", i.to_string(), id);
14691489
let id = ::id_from_node_id(id, &self.save_ctxt);
1470-
let span = self.span_from_span(p.span);
1490+
let span = self.span_from_span(sp);
14711491

14721492
self.dumper.dump_def(false, Def {
14731493
kind: DefKind::Local,
14741494
id,
14751495
span,
1476-
name: path_to_string(p),
1496+
name: i.to_string(),
14771497
qualname,
14781498
value: typ,
14791499
parent: None,
@@ -1485,19 +1505,12 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> Visitor<'l> for DumpVisitor<'l, 'tc
14851505
});
14861506
}
14871507
}
1488-
HirDef::StructCtor(..) | HirDef::VariantCtor(..) |
1489-
HirDef::Const(..) | HirDef::AssociatedConst(..) |
1490-
HirDef::Struct(..) | HirDef::Variant(..) |
1491-
HirDef::TyAlias(..) | HirDef::AssociatedTy(..) |
1492-
HirDef::SelfTy(..) => {
1493-
paths_to_process.push((id, p.clone()))
1494-
}
1495-
def => error!("unexpected definition kind when processing collected paths: {:?}",
1508+
def => error!("unexpected definition kind when processing collected idents: {:?}",
14961509
def),
14971510
}
14981511
}
14991512

1500-
for &(id, ref path) in &paths_to_process {
1513+
for (id, ref path) in collector.collected_paths {
15011514
self.process_path(id, path);
15021515
}
15031516
walk_list!(self, visit_expr, &arm.guard);

src/librustc_save_analysis/lib.rs

+52-18
Original file line numberDiff line numberDiff line change
@@ -579,8 +579,8 @@ impl<'l, 'tcx: 'l> SaveContext<'l, 'tcx> {
579579
Node::NodeItem(&hir::Item { node: hir::ItemUse(ref path, _), .. }) |
580580
Node::NodeVisibility(&hir::Visibility::Restricted { ref path, .. }) => path.def,
581581

582-
Node::NodeExpr(&hir::Expr { node: hir::ExprPath(ref qpath), .. }) |
583582
Node::NodeExpr(&hir::Expr { node: hir::ExprStruct(ref qpath, ..), .. }) |
583+
Node::NodeExpr(&hir::Expr { node: hir::ExprPath(ref qpath), .. }) |
584584
Node::NodePat(&hir::Pat { node: hir::PatKind::Path(ref qpath), .. }) |
585585
Node::NodePat(&hir::Pat { node: hir::PatKind::Struct(ref qpath, ..), .. }) |
586586
Node::NodePat(&hir::Pat { node: hir::PatKind::TupleStruct(ref qpath, ..), .. }) => {
@@ -614,6 +614,19 @@ impl<'l, 'tcx: 'l> SaveContext<'l, 'tcx> {
614614
}
615615

616616
pub fn get_path_data(&self, id: NodeId, path: &ast::Path) -> Option<Ref> {
617+
// Returns true if the path is function type sugar, e.g., `Fn(A) -> B`.
618+
fn fn_type(path: &ast::Path) -> bool {
619+
if path.segments.len() != 1 {
620+
return false;
621+
}
622+
if let Some(ref params) = path.segments[0].parameters {
623+
if let ast::PathParameters::Parenthesized(_) = **params {
624+
return true;
625+
}
626+
}
627+
false
628+
}
629+
617630
let def = self.get_path_def(id);
618631
let sub_span = self.span_utils.span_for_last_ident(path.span);
619632
filter!(self.span_utils, sub_span, path.span, None);
@@ -630,7 +643,6 @@ impl<'l, 'tcx: 'l> SaveContext<'l, 'tcx> {
630643
HirDef::Static(..) |
631644
HirDef::Const(..) |
632645
HirDef::AssociatedConst(..) |
633-
HirDef::StructCtor(..) |
634646
HirDef::VariantCtor(..) => {
635647
let span = self.span_from_span(sub_span.unwrap());
636648
Some(Ref {
@@ -639,6 +651,16 @@ impl<'l, 'tcx: 'l> SaveContext<'l, 'tcx> {
639651
ref_id: id_from_def_id(def.def_id()),
640652
})
641653
}
654+
HirDef::Trait(def_id) if fn_type(path) => {
655+
// Function type bounds are desugared in the parser, so we have to
656+
// special case them here.
657+
let fn_span = self.span_utils.span_for_first_ident(path.span);
658+
fn_span.map(|span| Ref {
659+
kind: RefKind::Type,
660+
span: self.span_from_span(span),
661+
ref_id: id_from_def_id(def_id),
662+
})
663+
}
642664
HirDef::Struct(def_id) |
643665
HirDef::Variant(def_id, ..) |
644666
HirDef::Union(def_id) |
@@ -655,6 +677,18 @@ impl<'l, 'tcx: 'l> SaveContext<'l, 'tcx> {
655677
ref_id: id_from_def_id(def_id),
656678
})
657679
}
680+
HirDef::StructCtor(def_id, _) => {
681+
// This is a reference to a tuple struct where the def_id points
682+
// to an invisible constructor function. That is not a very useful
683+
// def, so adjust to point to the tuple struct itself.
684+
let span = self.span_from_span(sub_span.unwrap());
685+
let parent_def_id = self.tcx.parent_def_id(def_id).unwrap();
686+
Some(Ref {
687+
kind: RefKind::Type,
688+
span,
689+
ref_id: id_from_def_id(parent_def_id),
690+
})
691+
}
658692
HirDef::Method(decl_id) => {
659693
let sub_span = self.span_utils.sub_span_for_meth_name(path.span);
660694
filter!(self.span_utils, sub_span, path.span, None);
@@ -818,29 +852,31 @@ fn make_signature(decl: &ast::FnDecl, generics: &ast::Generics) -> String {
818852
sig
819853
}
820854

821-
// An AST visitor for collecting paths from patterns.
822-
struct PathCollector {
823-
// The Row field identifies the kind of pattern.
824-
collected_paths: Vec<(NodeId, ast::Path, ast::Mutability)>,
855+
// An AST visitor for collecting paths (e.g., the names of structs) and formal
856+
// variables (idents) from patterns.
857+
struct PathCollector<'l> {
858+
collected_paths: Vec<(NodeId, &'l ast::Path)>,
859+
collected_idents: Vec<(NodeId, ast::Ident, Span, ast::Mutability)>,
825860
}
826861

827-
impl PathCollector {
828-
fn new() -> PathCollector {
829-
PathCollector { collected_paths: vec![] }
862+
impl<'l> PathCollector<'l> {
863+
fn new() -> PathCollector<'l> {
864+
PathCollector {
865+
collected_paths: vec![],
866+
collected_idents: vec![],
867+
}
830868
}
831869
}
832870

833-
impl<'a> Visitor<'a> for PathCollector {
834-
fn visit_pat(&mut self, p: &ast::Pat) {
871+
impl<'l, 'a: 'l> Visitor<'a> for PathCollector<'l> {
872+
fn visit_pat(&mut self, p: &'a ast::Pat) {
835873
match p.node {
836874
PatKind::Struct(ref path, ..) => {
837-
self.collected_paths.push((p.id, path.clone(),
838-
ast::Mutability::Mutable));
875+
self.collected_paths.push((p.id, path));
839876
}
840877
PatKind::TupleStruct(ref path, ..) |
841878
PatKind::Path(_, ref path) => {
842-
self.collected_paths.push((p.id, path.clone(),
843-
ast::Mutability::Mutable));
879+
self.collected_paths.push((p.id, path));
844880
}
845881
PatKind::Ident(bm, ref path1, _) => {
846882
debug!("PathCollector, visit ident in pat {}: {:?} {:?}",
@@ -854,9 +890,7 @@ impl<'a> Visitor<'a> for PathCollector {
854890
ast::BindingMode::ByRef(_) => ast::Mutability::Immutable,
855891
ast::BindingMode::ByValue(mt) => mt,
856892
};
857-
// collect path for either visit_local or visit_arm
858-
let path = ast::Path::from_ident(path1.span, path1.node);
859-
self.collected_paths.push((p.id, path, immut));
893+
self.collected_idents.push((p.id, path1.node, path1.span, immut));
860894
}
861895
_ => {}
862896
}

0 commit comments

Comments
 (0)