Skip to content

Commit e60aa8b

Browse files
get rid of a couple sources of single-def-per-path
1 parent ef902af commit e60aa8b

File tree

2 files changed

+31
-54
lines changed

2 files changed

+31
-54
lines changed

src/librustc/hir/lowering.rs

Lines changed: 26 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -149,12 +149,8 @@ pub trait Resolver {
149149
/// Resolve a hir path generated by the lowerer when expanding `for`, `if let`, etc.
150150
fn resolve_hir_path(&mut self, path: &mut hir::Path, is_value: bool);
151151

152-
/// Obtain the resolution for a node id. Prefers resolutions in Types, then Values, then
153-
/// Macros.
154-
fn get_resolution(&mut self, id: NodeId) -> Option<PathResolution>;
155-
156152
/// Obtain resolutions for the given node id in all namespaces.
157-
fn get_all_resolutions(&mut self, id: NodeId) -> PerNS<Option<PathResolution>>;
153+
fn get_resolutions(&mut self, id: NodeId) -> PerNS<Option<PathResolution>>;
158154

159155
/// We must keep the set of definitions up to date as we add nodes that weren't in the AST.
160156
/// This should only return `None` during testing.
@@ -566,19 +562,8 @@ impl<'a> LoweringContext<'a> {
566562
self.lower_node_id(self.sess.next_node_id())
567563
}
568564

569-
fn expect_full_def(&mut self, id: NodeId) -> Def {
570-
//FIXME: this grabs the first valid resolution - should all users of this function ask for
571-
//a specific namespace?
572-
self.resolver.get_resolution(id).map_or(Def::Err, |pr| {
573-
if pr.unresolved_segments() != 0 {
574-
bug!("path not fully resolved: {:?}", pr);
575-
}
576-
pr.base_def()
577-
})
578-
}
579-
580-
fn expect_full_def_per_ns(&mut self, id: NodeId) -> PerNS<Def> {
581-
self.resolver.get_all_resolutions(id).map(|res| res.map_or(Def::Err, |pr| {
565+
fn expect_full_defs(&mut self, id: NodeId) -> PerNS<Def> {
566+
self.resolver.get_resolutions(id).map(|res| res.map_or(Def::Err, |pr| {
582567
if pr.unresolved_segments() != 0 {
583568
bug!("path not fully resolved: {:?}", pr);
584569
}
@@ -935,11 +920,12 @@ impl<'a> LoweringContext<'a> {
935920
fn lower_loop_destination(&mut self, destination: Option<(NodeId, Label)>) -> hir::Destination {
936921
match destination {
937922
Some((id, label)) => {
938-
let target_id = if let Def::Label(loop_id) = self.expect_full_def(id) {
939-
Ok(self.lower_node_id(loop_id).node_id)
940-
} else {
941-
Err(hir::LoopIdError::UnresolvedLabel)
942-
};
923+
let target_id =
924+
if let Def::Label(loop_id) = self.expect_full_defs(id).value_ns {
925+
Ok(self.lower_node_id(loop_id).node_id)
926+
} else {
927+
Err(hir::LoopIdError::UnresolvedLabel)
928+
};
943929
hir::Destination {
944930
label: self.lower_label(Some(label)),
945931
target_id,
@@ -1085,7 +1071,7 @@ impl<'a> LoweringContext<'a> {
10851071
TyKind::ImplicitSelf => hir::TyPath(hir::QPath::Resolved(
10861072
None,
10871073
P(hir::Path {
1088-
defs: self.expect_full_def_per_ns(t.id),
1074+
defs: self.expect_full_defs(t.id),
10891075
segments: hir_vec![hir::PathSegment::from_name(keywords::SelfType.name())],
10901076
span: t.span,
10911077
}),
@@ -1395,12 +1381,15 @@ impl<'a> LoweringContext<'a> {
13951381
let qself = qself.as_ref().map(|q| self.lower_ty(&q.ty, itctx));
13961382

13971383
let resolutions = self.resolver
1398-
.get_all_resolutions(id)
1384+
.get_resolutions(id)
13991385
.map(|res| res.unwrap_or(PathResolution::new(Def::Err)));
14001386

1401-
//FIXME(misdreavus): when constructing the segments below, the code assumes only one
1402-
//resolution/def
1403-
let first_res = self.resolver.get_resolution(id).unwrap_or(PathResolution::new(Def::Err));
1387+
assert!(resolutions.into_iter().filter(|pr| pr.base_def() != Def::Err).count() <= 1,
1388+
"attempting to lower a QPath resolving to more than one Def: {:?} {:?}", id, p);
1389+
1390+
let first_res = resolutions.into_iter()
1391+
.find(|pr| pr.base_def() != Def::Err)
1392+
.unwrap_or(PathResolution::new(Def::Err));
14041393

14051394
let proj_start = p.segments.len() - first_res.unresolved_segments();
14061395
let path = P(hir::Path {
@@ -1557,7 +1546,7 @@ impl<'a> LoweringContext<'a> {
15571546
param_mode: ParamMode,
15581547
) -> hir::Path {
15591548
hir::Path {
1560-
defs: self.expect_full_def_per_ns(id),
1549+
defs: self.expect_full_defs(id),
15611550
segments: p.segments
15621551
.iter()
15631552
.map(|segment| {
@@ -1950,7 +1939,8 @@ impl<'a> LoweringContext<'a> {
19501939
&& bound_pred.bound_generic_params.is_empty() =>
19511940
{
19521941
if let Some(Def::TyParam(def_id)) = self.resolver
1953-
.get_resolution(bound_pred.bounded_ty.id)
1942+
.get_resolutions(bound_pred.bounded_ty.id)
1943+
.type_ns
19541944
.map(|d| d.base_def())
19551945
{
19561946
if let Some(node_id) =
@@ -2836,10 +2826,11 @@ impl<'a> LoweringContext<'a> {
28362826
let node = match p.node {
28372827
PatKind::Wild => hir::PatKind::Wild,
28382828
PatKind::Ident(ref binding_mode, ident, ref sub) => {
2839-
match self.resolver.get_resolution(p.id).map(|d| d.base_def()) {
2829+
match self.resolver.get_resolutions(p.id).map(|d| d.map(|d| d.base_def())) {
28402830
// `None` can occur in body-less function signatures
2841-
def @ None | def @ Some(Def::Local(_)) => {
2842-
let canonical_id = match def {
2831+
defs @ PerNS { value_ns: None, .. } |
2832+
defs @ PerNS { value_ns: Some(Def::Local(_)), .. } => {
2833+
let canonical_id = match defs.value_ns {
28432834
Some(Def::Local(id)) => id,
28442835
_ => p.id,
28452836
};
@@ -2850,11 +2841,11 @@ impl<'a> LoweringContext<'a> {
28502841
sub.as_ref().map(|x| self.lower_pat(x)),
28512842
)
28522843
}
2853-
Some(def) => hir::PatKind::Path(hir::QPath::Resolved(
2844+
defs => hir::PatKind::Path(hir::QPath::Resolved(
28542845
None,
28552846
P(hir::Path {
28562847
span: ident.span,
2857-
defs: def.into(),
2848+
defs: defs.map(|d| d.unwrap_or(Def::Err)),
28582849
segments: hir_vec![hir::PathSegment::from_name(ident.name)],
28592850
}),
28602851
)),

src/librustc_resolve/lib.rs

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1476,21 +1476,7 @@ impl<'a> hir::lowering::Resolver for Resolver<'a> {
14761476
path
14771477
}
14781478

1479-
fn get_resolution(&mut self, id: NodeId) -> Option<PathResolution> {
1480-
//for people calling this one, they don't necessarily care which namespace it's in, so just
1481-
//have a preference but check all of them
1482-
if let Some(def) = self.def_map.get(&id) {
1483-
for ns in [TypeNS, ValueNS, MacroNS].iter().cloned() {
1484-
if let Some(res) = def[ns] {
1485-
return Some(res);
1486-
}
1487-
}
1488-
}
1489-
1490-
None
1491-
}
1492-
1493-
fn get_all_resolutions(&mut self, id: NodeId) -> PerNS<Option<PathResolution>> {
1479+
fn get_resolutions(&mut self, id: NodeId) -> PerNS<Option<PathResolution>> {
14941480
self.def_map.get(&id).cloned().unwrap_or_default()
14951481
}
14961482

@@ -2433,9 +2419,9 @@ impl<'a> Resolver<'a> {
24332419

24342420
pat.walk(&mut |pat| {
24352421
if let PatKind::Ident(binding_mode, ident, ref sub_pat) = pat.node {
2436-
//FIXME(misdreavus): `get_resolution` is the "i don't care what namespace" version,
2437-
//is that valid?
2438-
if sub_pat.is_some() || match self.get_resolution(pat.id).map(|res| res.base_def()) {
2422+
if sub_pat.is_some() || match self.get_resolutions(pat.id)
2423+
.value_ns
2424+
.map(|res| res.base_def()) {
24392425
Some(Def::Local(..)) => true,
24402426
_ => false,
24412427
} {
@@ -3605,7 +3591,7 @@ impl<'a> Resolver<'a> {
36053591
if filter_fn(Def::Local(ast::DUMMY_NODE_ID)) {
36063592
if let Some(node_id) = self.current_self_type.as_ref().and_then(extract_node_id) {
36073593
// Look for a field with the same name in the current self_type.
3608-
if let Some(resolution) = self.get_resolution(node_id) {
3594+
if let Some(resolution) = self.get_resolutions(node_id).type_ns {
36093595
match resolution.base_def() {
36103596
Def::Struct(did) | Def::Union(did)
36113597
if resolution.unresolved_segments() == 0 => {

0 commit comments

Comments
 (0)