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.

Sign up for GitHub

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

Jump to bottom

Fix two ICEs in path resolution #39939

Merged
merged 2 commits into from
Feb 19, 2017
Merged

Fix two ICEs in path resolution #39939

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bf95c29
Privatize fields of PathResolution
petrochenkov Feb 18, 2017
8c7d007
Avoid ICE in Self::Assoc in impl headers
petrochenkov Feb 18, 2017
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
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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() {}