Skip to content

Commit

Permalink
Auto merge of #39939 - petrochenkov:regres, r=eddyb
Browse files Browse the repository at this point in the history 
Fix two ICEs in path resolution

Fixes #39535
Fixes #39559
Fixes #39924

r? @eddyb
  • Loading branch information
@bors
bors committed Feb 19, 2017
2 parents 0128be9 + 8c7d007 commit 0e77277
Show file tree
Hide file tree
Showing 7 changed files with 119 additions and 65 deletions.
39 changes: 26 additions & 13 deletions src/librustc/hir/def.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
16 changes: 8 additions & 8 deletions src/librustc/hir/lowering.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})
}

Expand Down Expand Up @@ -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 => {
Expand All @@ -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))
}
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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(|| {
Expand Down
79 changes: 37 additions & 42 deletions src/librustc_resolve/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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 };
Expand Down Expand Up @@ -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) {
Expand All @@ -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() => {
Expand Down Expand Up @@ -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 },
};
}
Expand All @@ -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)) {
Expand Down Expand Up @@ -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) => {
Expand All @@ -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());
}
Expand Down Expand Up @@ -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 }),
}
Expand All @@ -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);
}
Expand All @@ -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() {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_resolve/macros.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
},
Expand Down
14 changes: 13 additions & 1 deletion src/librustc_typeck/astconv.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
31 changes: 31 additions & 0 deletions src/test/compile-fail/issue-39559.rs
View file
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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}

0 comments on commit 0e77277

Please sign in to comment.