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

Fix two ICEs in path resolution #39939

Merged
merged 2 commits into from
Feb 19, 2017
Merged
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
39 changes: 26 additions & 13 deletions src/librustc/hir/def.rs
Original file line number Diff line number Diff line change
@@ -59,32 +59,45 @@ pub enum Def {
Err,
}

/// The result of resolving a path.
/// Before type checking completes, `depth` represents the number of
/// trailing segments which are yet unresolved. Afterwards, if there
/// were no errors, all paths should be fully resolved, with `depth`
/// set to `0` and `base_def` representing the final resolution.
///
/// The result of resolving a path before lowering to HIR.
/// `base_def` is definition of resolved part of the
/// path, `unresolved_segments` is the number of unresolved
/// segments.
/// module::Type::AssocX::AssocY::MethodOrAssocType
/// ^~~~~~~~~~~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/// base_def depth = 3
/// base_def unresolved_segments = 3
///
/// <T as Trait>::AssocX::AssocY::MethodOrAssocType
/// ^~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~~~~~~~~~~
/// base_def depth = 2
/// base_def unresolved_segments = 2
#[derive(Copy, Clone, Debug)]
pub struct PathResolution {
pub base_def: Def,
pub depth: usize
base_def: Def,
unresolved_segments: usize,
}

impl PathResolution {
pub fn new(def: Def) -> PathResolution {
PathResolution { base_def: def, depth: 0 }
pub fn new(def: Def) -> Self {
PathResolution { base_def: def, unresolved_segments: 0 }
}

pub fn with_unresolved_segments(def: Def, mut unresolved_segments: usize) -> Self {
if def == Def::Err { unresolved_segments = 0 }
PathResolution { base_def: def, unresolved_segments: unresolved_segments }
}

#[inline]
pub fn base_def(&self) -> Def {
self.base_def
}

#[inline]
pub fn unresolved_segments(&self) -> usize {
self.unresolved_segments
}

pub fn kind_name(&self) -> &'static str {
if self.depth != 0 {
if self.unresolved_segments != 0 {
"associated item"
} else {
self.base_def.kind_name()
16 changes: 8 additions & 8 deletions src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
@@ -217,10 +217,10 @@ impl<'a> LoweringContext<'a> {

fn expect_full_def(&mut self, id: NodeId) -> Def {
self.resolver.get_resolution(id).map_or(Def::Err, |pr| {
if pr.depth != 0 {
if pr.unresolved_segments() != 0 {
bug!("path not fully resolved: {:?}", pr);
}
pr.base_def
pr.base_def()
})
}

@@ -421,9 +421,9 @@ impl<'a> LoweringContext<'a> {
let resolution = self.resolver.get_resolution(id)
.unwrap_or(PathResolution::new(Def::Err));

let proj_start = p.segments.len() - resolution.depth;
let proj_start = p.segments.len() - resolution.unresolved_segments();
let path = P(hir::Path {
def: resolution.base_def,
def: resolution.base_def(),
segments: p.segments[..proj_start].iter().enumerate().map(|(i, segment)| {
let param_mode = match (qself_position, param_mode) {
(Some(j), ParamMode::Optional) if i < j => {
@@ -443,7 +443,7 @@ impl<'a> LoweringContext<'a> {
index: this.def_key(def_id).parent.expect("missing parent")
}
};
let type_def_id = match resolution.base_def {
let type_def_id = match resolution.base_def() {
Def::AssociatedTy(def_id) if i + 2 == proj_start => {
Some(parent_def_id(self, def_id))
}
@@ -474,7 +474,7 @@ impl<'a> LoweringContext<'a> {

// Simple case, either no projections, or only fully-qualified.
// E.g. `std::mem::size_of` or `<I as Iterator>::Item`.
if resolution.depth == 0 {
if resolution.unresolved_segments() == 0 {
return hir::QPath::Resolved(qself, path);
}

@@ -749,7 +749,7 @@ impl<'a> LoweringContext<'a> {
bound_pred.bound_lifetimes.is_empty() => {
if let Some(Def::TyParam(def_id)) =
self.resolver.get_resolution(bound_pred.bounded_ty.id)
.map(|d| d.base_def) {
.map(|d| d.base_def()) {
if let Some(node_id) =
self.resolver.definitions().as_local_node_id(def_id) {
for ty_param in &g.ty_params {
@@ -1295,7 +1295,7 @@ impl<'a> LoweringContext<'a> {
PatKind::Wild => hir::PatKind::Wild,
PatKind::Ident(ref binding_mode, pth1, ref sub) => {
self.with_parent_def(p.id, |this| {
match this.resolver.get_resolution(p.id).map(|d| d.base_def) {
match this.resolver.get_resolution(p.id).map(|d| d.base_def()) {
// `None` can occur in body-less function signatures
def @ None | def @ Some(Def::Local(_)) => {
let def_id = def.map(|d| d.def_id()).unwrap_or_else(|| {
79 changes: 37 additions & 42 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
@@ -1195,7 +1195,8 @@ impl<'a> hir::lowering::Resolver for Resolver<'a> {
let path: Vec<_> = segments.iter().map(|seg| Ident::with_empty_ctxt(seg.name)).collect();
match self.resolve_path(&path, Some(namespace), Some(span)) {
PathResult::Module(module) => *def = module.def().unwrap(),
PathResult::NonModule(path_res) if path_res.depth == 0 => *def = path_res.base_def,
PathResult::NonModule(path_res) if path_res.unresolved_segments() == 0 =>
*def = path_res.base_def(),
PathResult::NonModule(..) => match self.resolve_path(&path, None, Some(span)) {
PathResult::Failed(msg, _) => {
resolve_error(self, span, ResolutionError::FailedToResolve(&msg));
@@ -1718,7 +1719,7 @@ impl<'a> Resolver<'a> {
let mut new_id = None;
if let Some(trait_ref) = opt_trait_ref {
let def = self.smart_resolve_path(trait_ref.ref_id, None,
&trait_ref.path, PathSource::Trait).base_def;
&trait_ref.path, PathSource::Trait).base_def();
if def != Def::Err {
new_val = Some((def.def_id(), trait_ref.clone()));
new_id = Some(def.def_id());
@@ -1849,8 +1850,8 @@ impl<'a> Resolver<'a> {

pat.walk(&mut |pat| {
if let PatKind::Ident(binding_mode, ident, ref sub_pat) = pat.node {
if sub_pat.is_some() || match self.def_map.get(&pat.id) {
Some(&PathResolution { base_def: Def::Local(..), .. }) => true,
if sub_pat.is_some() || match self.def_map.get(&pat.id).map(|res| res.base_def()) {
Some(Def::Local(..)) => true,
_ => false,
} {
let binding_info = BindingInfo { span: ident.span, binding_mode: binding_mode };
@@ -2248,14 +2249,14 @@ impl<'a> Resolver<'a> {
let resolution = match self.resolve_qpath_anywhere(id, qself, path, ns, span,
source.defer_to_typeck(),
source.global_by_default()) {
Some(resolution) if resolution.depth == 0 => {
if is_expected(resolution.base_def) || resolution.base_def == Def::Err {
Some(resolution) if resolution.unresolved_segments() == 0 => {
if is_expected(resolution.base_def()) || resolution.base_def() == Def::Err {
resolution
} else {
// Add a temporary hack to smooth the transition to new struct ctor
// visibility rules. See #38932 for more details.
let mut res = None;
if let Def::Struct(def_id) = resolution.base_def {
if let Def::Struct(def_id) = resolution.base_def() {
if let Some((ctor_def, ctor_vis))
= self.struct_constructors.get(&def_id).cloned() {
if is_expected(ctor_def) && self.is_accessible(ctor_vis) {
@@ -2268,7 +2269,7 @@ impl<'a> Resolver<'a> {
}
}

res.unwrap_or_else(|| report_errors(self, Some(resolution.base_def)))
res.unwrap_or_else(|| report_errors(self, Some(resolution.base_def())))
}
}
Some(resolution) if source.defer_to_typeck() => {
@@ -2321,7 +2322,8 @@ impl<'a> Resolver<'a> {
match self.resolve_qpath(id, qself, path, ns, span, global_by_default) {
// If defer_to_typeck, then resolution > no resolution,
// otherwise full resolution > partial resolution > no resolution.
Some(res) if res.depth == 0 || defer_to_typeck => return Some(res),
Some(res) if res.unresolved_segments() == 0 || defer_to_typeck =>
return Some(res),
res => if fin_res.is_none() { fin_res = res },
};
}
@@ -2346,19 +2348,17 @@ impl<'a> Resolver<'a> {
if let Some(qself) = qself {
if qself.position == 0 {
// FIXME: Create some fake resolution that can't possibly be a type.
return Some(PathResolution {
base_def: Def::Mod(DefId::local(CRATE_DEF_INDEX)),
depth: path.len(),
});
return Some(PathResolution::with_unresolved_segments(
Def::Mod(DefId::local(CRATE_DEF_INDEX)), path.len()
));
}
// Make sure `A::B` in `<T as A>::B::C` is a trait item.
let ns = if qself.position + 1 == path.len() { ns } else { TypeNS };
let mut res = self.smart_resolve_path_fragment(id, None, &path[..qself.position + 1],
span, PathSource::TraitItem(ns));
if res.base_def != Def::Err {
res.depth += path.len() - qself.position - 1;
}
return Some(res);
let res = self.smart_resolve_path_fragment(id, None, &path[..qself.position + 1],
span, PathSource::TraitItem(ns));
return Some(PathResolution::with_unresolved_segments(
res.base_def(), res.unresolved_segments() + path.len() - qself.position - 1
));
}

let result = match self.resolve_path(&path, Some(ns), Some(span)) {
@@ -2393,10 +2393,7 @@ impl<'a> Resolver<'a> {
}
_ => {}
}
PathResolution {
base_def: Def::PrimTy(prim),
depth: path.len() - 1,
}
PathResolution::with_unresolved_segments(Def::PrimTy(prim), path.len() - 1)
}
PathResult::Module(module) => PathResolution::new(module.def().unwrap()),
PathResult::Failed(msg, false) => {
@@ -2407,16 +2404,16 @@ impl<'a> Resolver<'a> {
PathResult::Indeterminate => bug!("indetermined path result in resolve_qpath"),
};

if path.len() > 1 && !global_by_default && result.base_def != Def::Err &&
if path.len() > 1 && !global_by_default && result.base_def() != Def::Err &&
path[0].name != keywords::CrateRoot.name() && path[0].name != "$crate" {
let unqualified_result = {
match self.resolve_path(&[*path.last().unwrap()], Some(ns), None) {
PathResult::NonModule(path_res) => path_res.base_def,
PathResult::NonModule(path_res) => path_res.base_def(),
PathResult::Module(module) => module.def().unwrap(),
_ => return Some(result),
}
};
if result.base_def == unqualified_result {
if result.base_def() == unqualified_result {
let lint = lint::builtin::UNUSED_QUALIFICATIONS;
self.session.add_lint(lint, id, span, "unnecessary qualification".to_string());
}
@@ -2470,10 +2467,9 @@ impl<'a> Resolver<'a> {
Some(LexicalScopeBinding::Item(binding)) => Ok(binding),
Some(LexicalScopeBinding::Def(def))
if opt_ns == Some(TypeNS) || opt_ns == Some(ValueNS) => {
return PathResult::NonModule(PathResolution {
base_def: def,
depth: path.len() - 1,
});
return PathResult::NonModule(PathResolution::with_unresolved_segments(
def, path.len() - 1
));
}
_ => Err(if record_used.is_some() { Determined } else { Undetermined }),
}
@@ -2488,10 +2484,9 @@ impl<'a> Resolver<'a> {
} else if def == Def::Err {
return PathResult::NonModule(err_path_resolution());
} else if opt_ns.is_some() && (is_last || maybe_assoc) {
return PathResult::NonModule(PathResolution {
base_def: def,
depth: path.len() - i - 1,
});
return PathResult::NonModule(PathResolution::with_unresolved_segments(
def, path.len() - i - 1
));
} else {
return PathResult::Failed(format!("Not a module `{}`", ident), is_last);
}
@@ -2500,10 +2495,9 @@ impl<'a> Resolver<'a> {
Err(Determined) => {
if let Some(module) = module {
if opt_ns.is_some() && !module.is_normal() {
return PathResult::NonModule(PathResolution {
base_def: module.def().unwrap(),
depth: path.len() - i,
});
return PathResult::NonModule(PathResolution::with_unresolved_segments(
module.def().unwrap(), path.len() - i
));
}
}
let msg = if module.and_then(ModuleData::def) == self.graph_root.def() {
@@ -2672,8 +2666,9 @@ impl<'a> Resolver<'a> {
if let Some(node_id) = self.current_self_type.as_ref().and_then(extract_node_id) {
// Look for a field with the same name in the current self_type.
if let Some(resolution) = self.def_map.get(&node_id) {
match resolution.base_def {
Def::Struct(did) | Def::Union(did) if resolution.depth == 0 => {
match resolution.base_def() {
Def::Struct(did) | Def::Union(did)
if resolution.unresolved_segments() == 0 => {
if let Some(field_names) = self.field_names.get(&did) {
if field_names.iter().any(|&field_name| name == field_name) {
return Some(AssocSuggestion::Field);
@@ -3057,7 +3052,6 @@ impl<'a> Resolver<'a> {

fn record_def(&mut self, node_id: NodeId, resolution: PathResolution) {
debug!("(recording def) recording {:?} for {}", resolution, node_id);
assert!(resolution.depth == 0 || resolution.base_def != Def::Err);
if let Some(prev_res) = self.def_map.insert(node_id, resolution) {
panic!("path resolved multiple times ({:?} before, {:?} now)", prev_res, resolution);
}
@@ -3071,7 +3065,8 @@ impl<'a> Resolver<'a> {
ty::Visibility::Restricted(self.current_module.normal_ancestor_id)
}
ast::Visibility::Restricted { ref path, id } => {
let def = self.smart_resolve_path(id, None, path, PathSource::Visibility).base_def;
let def = self.smart_resolve_path(id, None, path,
PathSource::Visibility).base_def();
if def == Def::Err {
ty::Visibility::Public
} else {
2 changes: 1 addition & 1 deletion src/librustc_resolve/macros.rs
Original file line number Diff line number Diff line change
@@ -262,7 +262,7 @@ impl<'a> base::Resolver for Resolver<'a> {
}

let ext = match self.resolve_path(&path, Some(MacroNS), None) {
PathResult::NonModule(path_res) => match path_res.base_def {
PathResult::NonModule(path_res) => match path_res.base_def() {
Def::Err => Err(Determinacy::Determined),
def @ _ => Ok(self.get_macro(def)),
},
14 changes: 13 additions & 1 deletion src/librustc_typeck/astconv.rs
Original file line number Diff line number Diff line change
@@ -918,7 +918,19 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o {
(_, Def::SelfTy(Some(_), Some(impl_def_id))) => {
// `Self` in an impl of a trait - we have a concrete self type and a
// trait reference.
let trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap();
// FIXME: Self type is not always computed when we are here because type parameter
// bounds may affect Self type and have to be converted before it.
let trait_ref = if impl_def_id.is_local() {
tcx.impl_trait_refs.borrow().get(&impl_def_id).cloned().and_then(|x| x)
} else {
tcx.impl_trait_ref(impl_def_id)
};
let trait_ref = if let Some(trait_ref) = trait_ref {
trait_ref
} else {
tcx.sess.span_err(span, "`Self` type is used before it's determined");
return (tcx.types.err, Def::Err);
};
let trait_ref = if let Some(free_substs) = self.get_free_substs() {
trait_ref.subst(tcx, free_substs)
} else {
31 changes: 31 additions & 0 deletions src/test/compile-fail/issue-39559.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

trait Dim {
fn dim() -> usize;
}

enum Dim3 {}

impl Dim for Dim3 {
fn dim() -> usize {
3
}
}

pub struct Vector<T, D: Dim> {
entries: [T; D::dim()]
//~^ ERROR cannot use an outer type parameter in this context
//~| ERROR constant evaluation error
}

fn main() {
let array: [usize; Dim3::dim()] = [0; Dim3::dim()];
}
3 changes: 3 additions & 0 deletions src/test/compile-fail/resolve-self-in-impl.rs
Original file line number Diff line number Diff line change
@@ -18,9 +18,12 @@ impl<T: Tr<Self>> Tr<T> for S {} //~ ERROR `Self` type is used before it's deter
impl<T = Self> Tr<T> for S {} //~ ERROR `Self` type is used before it's determined
impl Tr for S where Self: Copy {} //~ ERROR `Self` type is used before it's determined
impl Tr for S where S<Self>: Copy {} //~ ERROR `Self` type is used before it's determined
impl Tr for S where Self::Assoc: Copy {} //~ ERROR `Self` type is used before it's determined
//~^ ERROR `Self` type is used before it's determined
impl Tr for Self {} //~ ERROR `Self` type is used before it's determined
impl Tr for S<Self> {} //~ ERROR `Self` type is used before it's determined
impl Self {} //~ ERROR `Self` type is used before it's determined
impl S<Self> {} //~ ERROR `Self` type is used before it's determined
impl Tr<Self::Assoc> for S {} //~ ERROR `Self` type is used before it's determined

fn main() {}