Skip to content

Commit

Permalink
Auto merge of #55521 - nrc:rls-fix, r=petrochenkov
Browse files Browse the repository at this point in the history
save-analysis: bug fix and optimisation.

The first commit fixes a bug in name resolution and save-analysis (introduced in #54145) and removes an unused parameter. This fixes the RLS tests, which are currently blocking distribution of the RLS. The second commit removes macro uses from save-analysis data, since these are never used, they just take up space.

r? @petrochenkov
  • Loading branch information
bors committed Oct 31, 2018
2 parents de9666f + 435d832 commit fabbe05
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 31 deletions.
11 changes: 4 additions & 7 deletions src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1754,7 +1754,6 @@ impl<'a> LoweringContext<'a> {
&mut self,
def: Def,
p: &Path,
ident: Option<Ident>,
param_mode: ParamMode,
explicit_owner: Option<NodeId>,
) -> hir::Path {
Expand All @@ -1773,15 +1772,14 @@ impl<'a> LoweringContext<'a> {
explicit_owner,
)
})
.chain(ident.map(|ident| hir::PathSegment::from_ident(ident)))
.collect(),
span: p.span,
}
}

fn lower_path(&mut self, id: NodeId, p: &Path, param_mode: ParamMode) -> hir::Path {
let def = self.expect_full_def(id);
self.lower_path_extra(def, p, None, param_mode, None)
self.lower_path_extra(def, p, param_mode, None)
}

fn lower_path_segment(
Expand Down Expand Up @@ -3014,7 +3012,7 @@ impl<'a> LoweringContext<'a> {
self.with_hir_id_owner(new_node_id, |this| {
let new_id = this.lower_node_id(new_node_id);
let path =
this.lower_path_extra(def, &path, None, ParamMode::Explicit, None);
this.lower_path_extra(def, &path, ParamMode::Explicit, None);
let item = hir::ItemKind::Use(P(path), hir::UseKind::Single);
let vis_kind = match vis.node {
hir::VisibilityKind::Public => hir::VisibilityKind::Public,
Expand Down Expand Up @@ -3053,7 +3051,7 @@ impl<'a> LoweringContext<'a> {
}

let path =
P(self.lower_path_extra(ret_def, &path, None, ParamMode::Explicit, None));
P(self.lower_path_extra(ret_def, &path, ParamMode::Explicit, None));
hir::ItemKind::Use(path, hir::UseKind::Single)
}
UseTreeKind::Glob => {
Expand Down Expand Up @@ -3140,7 +3138,7 @@ impl<'a> LoweringContext<'a> {
// the stability of `use a::{};`, to avoid it showing up as
// a re-export by accident when `pub`, e.g. in documentation.
let def = self.expect_full_def_from_use(id).next().unwrap_or(Def::Err);
let path = P(self.lower_path_extra(def, &prefix, None, ParamMode::Explicit, None));
let path = P(self.lower_path_extra(def, &prefix, ParamMode::Explicit, None));
*vis = respan(prefix.span.shrink_to_lo(), hir::VisibilityKind::Inherited);
hir::ItemKind::Use(path, hir::UseKind::ListStem)
}
Expand Down Expand Up @@ -4550,7 +4548,6 @@ impl<'a> LoweringContext<'a> {
path: P(self.lower_path_extra(
def,
path,
None,
ParamMode::Explicit,
explicit_owner,
)),
Expand Down
22 changes: 13 additions & 9 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3589,7 +3589,17 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
);

for (i, &Segment { ident, id }) in path.iter().enumerate() {
debug!("resolve_path ident {} {:?}", i, ident);
debug!("resolve_path ident {} {:?} {:?}", i, ident, id);
let record_segment_def = |this: &mut Self, def| {
if record_used {
if let Some(id) = id {
if !this.def_map.contains_key(&id) {
assert!(id != ast::DUMMY_NODE_ID, "Trying to resolve dummy id");
this.record_def(id, PathResolution::new(def));
}
}
}
};

let is_last = i == path.len() - 1;
let ns = if is_last { opt_ns.unwrap_or(TypeNS) } else { TypeNS };
Expand Down Expand Up @@ -3673,6 +3683,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
// we found a local variable or type param
Some(LexicalScopeBinding::Def(def))
if opt_ns == Some(TypeNS) || opt_ns == Some(ValueNS) => {
record_segment_def(self, def);
return PathResult::NonModule(PathResolution::with_unresolved_segments(
def, path.len() - 1
));
Expand All @@ -3690,14 +3701,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
let maybe_assoc = opt_ns != Some(MacroNS) && PathSource::Type.is_expected(def);
if let Some(next_module) = binding.module() {
module = Some(ModuleOrUniformRoot::Module(next_module));
if record_used {
if let Some(id) = id {
if !self.def_map.contains_key(&id) {
assert!(id != ast::DUMMY_NODE_ID, "Trying to resolve dummy id");
self.record_def(id, PathResolution::new(def));
}
}
}
record_segment_def(self, def);
} else if def == Def::ToolMod && i + 1 != path.len() {
let def = Def::NonMacroAttr(NonMacroAttrKind::Tool);
return PathResult::NonModule(PathResolution::new(def));
Expand Down
29 changes: 15 additions & 14 deletions src/librustc_save_analysis/dump_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ pub struct DumpVisitor<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> {
// we only write one macro def per unique macro definition, and
// one macro use per unique callsite span.
// mac_defs: FxHashSet<Span>,
macro_calls: FxHashSet<Span>,
// macro_calls: FxHashSet<Span>,
}

impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
Expand All @@ -108,7 +108,7 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
span: span_utils,
cur_scope: CRATE_NODE_ID,
// mac_defs: FxHashSet::default(),
macro_calls: FxHashSet::default(),
// macro_calls: FxHashSet::default(),
}
}

Expand Down Expand Up @@ -771,8 +771,7 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
}

fn process_path(&mut self, id: NodeId, path: &'l ast::Path) {
debug!("process_path {:?}", path);
if generated_code(path.span) {
if self.span.filter_generated(path.span) {
return;
}
self.dump_path_ref(id, path);
Expand Down Expand Up @@ -1031,18 +1030,20 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
/// If the span is not macro-generated, do nothing, else use callee and
/// callsite spans to record macro definition and use data, using the
/// mac_uses and mac_defs sets to prevent multiples.
fn process_macro_use(&mut self, span: Span) {
let source_span = span.source_callsite();
if !self.macro_calls.insert(source_span) {
return;
}
fn process_macro_use(&mut self, _span: Span) {
// FIXME if we're not dumping the defs (see below), there is no point
// dumping refs either.
// let source_span = span.source_callsite();
// if !self.macro_calls.insert(source_span) {
// return;
// }

let data = match self.save_ctxt.get_macro_use_data(span) {
None => return,
Some(data) => data,
};
// let data = match self.save_ctxt.get_macro_use_data(span) {
// None => return,
// Some(data) => data,
// };

self.dumper.macro_use(data);
// self.dumper.macro_use(data);

// FIXME write the macro def
// let mut hasher = DefaultHasher::new();
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_save_analysis/json_dumper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ impl<'b, O: DumpOutput + 'b> JsonDumper<O> {
self.result.compilation = Some(data);
}

pub fn macro_use(&mut self, data: MacroRef) {
pub fn _macro_use(&mut self, data: MacroRef) {
if self.config.pub_only || self.config.reachable_only {
return;
}
Expand Down

0 comments on commit fabbe05

Please sign in to comment.