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

Account for tail expressions when pointing at return type #64802

Merged
merged 4 commits into from
Sep 28, 2019
Merged
Show file tree
Hide file tree
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
235 changes: 127 additions & 108 deletions src/librustc/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ use syntax::source_map::Spanned;
use syntax::ext::base::MacroKind;
use syntax_pos::{Span, DUMMY_SP};

use std::result::Result::Err;

pub mod blocks;
mod collector;
mod def_collector;
Expand Down Expand Up @@ -183,6 +181,44 @@ pub struct Map<'hir> {
hir_to_node_id: FxHashMap<HirId, NodeId>,
}

struct ParentHirIterator<'map> {
current_id: HirId,
map: &'map Map<'map>,
}

impl<'map> ParentHirIterator<'map> {
fn new(current_id: HirId, map: &'map Map<'map>) -> ParentHirIterator<'map> {
ParentHirIterator {
current_id,
map,
}
}
}

impl<'map> Iterator for ParentHirIterator<'map> {
type Item = (HirId, Node<'map>);

fn next(&mut self) -> Option<Self::Item> {
if self.current_id == CRATE_HIR_ID {
return None;
}
loop { // There are nodes that do not have entries, so we need to skip them.
let parent_id = self.map.get_parent_node(self.current_id);

if parent_id == self.current_id {
self.current_id = CRATE_HIR_ID;
return None;
}

self.current_id = parent_id;
if let Some(entry) = self.map.find_entry(parent_id) {
return Some((parent_id, entry.node));
}
// If this `HirId` doesn't have an `Entry`, skip it and look for its `parent_id`.
}
}
}

impl<'hir> Map<'hir> {
#[inline]
fn lookup(&self, id: HirId) -> Option<&Entry<'hir>> {
Expand Down Expand Up @@ -682,45 +718,6 @@ impl<'hir> Map<'hir> {
}
}


/// If there is some error when walking the parents (e.g., a node does not
/// have a parent in the map or a node can't be found), then we return the
/// last good `HirId` we found. Note that reaching the crate root (`id == 0`),
/// is not an error, since items in the crate module have the crate root as
/// parent.
fn walk_parent_nodes<F, F2>(&self,
start_id: HirId,
found: F,
bail_early: F2)
-> Result<HirId, HirId>
where F: Fn(&Node<'hir>) -> bool, F2: Fn(&Node<'hir>) -> bool
{
let mut id = start_id;
loop {
let parent_id = self.get_parent_node(id);
if parent_id == CRATE_HIR_ID {
return Ok(CRATE_HIR_ID);
}
if parent_id == id {
return Err(id);
}

if let Some(entry) = self.find_entry(parent_id) {
if let Node::Crate = entry.node {
return Err(id);
}
if found(&entry.node) {
return Ok(parent_id);
} else if bail_early(&entry.node) {
return Err(parent_id);
}
id = parent_id;
} else {
return Err(id);
}
}
}

/// Retrieves the `HirId` for `id`'s enclosing method, unless there's a
/// `while` or `loop` before reaching it, as block tail returns are not
/// available in them.
Expand All @@ -744,46 +741,64 @@ impl<'hir> Map<'hir> {
/// }
/// ```
pub fn get_return_block(&self, id: HirId) -> Option<HirId> {
let match_fn = |node: &Node<'_>| {
match *node {
let mut iter = ParentHirIterator::new(id, &self).peekable();
let mut ignore_tail = false;
if let Some(entry) = self.find_entry(id) {
if let Node::Expr(Expr { kind: ExprKind::Ret(_), .. }) = entry.node {
// When dealing with `return` statements, we don't care about climbing only tail
// expressions.
ignore_tail = true;
}
}
while let Some((hir_id, node)) = iter.next() {
if let (Some((_, next_node)), false) = (iter.peek(), ignore_tail) {
match next_node {
Node::Block(Block { expr: None, .. }) => return None,
Node::Block(Block { expr: Some(expr), .. }) => {
if hir_id != expr.hir_id {
// The current node is not the tail expression of its parent.
return None;
}
}
_ => {}
}
}
match node {
Node::Item(_) |
Node::ForeignItem(_) |
Node::TraitItem(_) |
Node::Expr(Expr { kind: ExprKind::Closure(..), ..}) |
Node::ImplItem(_) => true,
_ => false,
}
};
let match_non_returning_block = |node: &Node<'_>| {
match *node {
Node::ImplItem(_) => return Some(hir_id),
Node::Expr(ref expr) => {
match expr.kind {
ExprKind::Loop(..) | ExprKind::Ret(..) => true,
_ => false,
// Ignore `return`s on the first iteration
ExprKind::Loop(..) | ExprKind::Ret(..) => return None,
_ => {}
}
}
_ => false,
Node::Local(_) => return None,
_ => {}
}
};

self.walk_parent_nodes(id, match_fn, match_non_returning_block).ok()
}
None
}

/// Retrieves the `HirId` for `id`'s parent item, or `id` itself if no
/// parent item is in this map. The "parent item" is the closest parent node
/// in the HIR which is recorded by the map and is an item, either an item
/// in a module, trait, or impl.
pub fn get_parent_item(&self, hir_id: HirId) -> HirId {
match self.walk_parent_nodes(hir_id, |node| match *node {
Node::Item(_) |
Node::ForeignItem(_) |
Node::TraitItem(_) |
Node::ImplItem(_) => true,
_ => false,
}, |_| false) {
Ok(id) => id,
Err(id) => id,
for (hir_id, node) in ParentHirIterator::new(hir_id, &self) {
match node {
Node::Crate |
Node::Item(_) |
Node::ForeignItem(_) |
Node::TraitItem(_) |
Node::ImplItem(_) => return hir_id,
_ => {}
}
}
hir_id
}

/// Returns the `DefId` of `id`'s nearest module parent, or `id` itself if no
Expand All @@ -795,60 +810,64 @@ impl<'hir> Map<'hir> {
/// Returns the `HirId` of `id`'s nearest module parent, or `id` itself if no
/// module parent is in this map.
pub fn get_module_parent_node(&self, hir_id: HirId) -> HirId {
match self.walk_parent_nodes(hir_id, |node| match *node {
Node::Item(&Item { kind: ItemKind::Mod(_), .. }) => true,
_ => false,
}, |_| false) {
Ok(id) => id,
Err(id) => id,
for (hir_id, node) in ParentHirIterator::new(hir_id, &self) {
if let Node::Item(&Item { kind: ItemKind::Mod(_), .. }) = node {
return hir_id;
}
}
CRATE_HIR_ID
}

/// Returns the nearest enclosing scope. A scope is roughly an item or block.
pub fn get_enclosing_scope(&self, hir_id: HirId) -> Option<HirId> {
self.walk_parent_nodes(hir_id, |node| match *node {
Node::Item(i) => {
match i.kind {
ItemKind::Fn(..)
| ItemKind::Mod(..)
| ItemKind::Enum(..)
| ItemKind::Struct(..)
| ItemKind::Union(..)
| ItemKind::Trait(..)
| ItemKind::Impl(..) => true,
_ => false,
}
},
Node::ForeignItem(fi) => {
match fi.kind {
ForeignItemKind::Fn(..) => true,
_ => false,
}
},
Node::TraitItem(ti) => {
match ti.kind {
TraitItemKind::Method(..) => true,
_ => false,
}
},
Node::ImplItem(ii) => {
match ii.kind {
ImplItemKind::Method(..) => true,
_ => false,
}
},
Node::Block(_) => true,
_ => false,
}, |_| false).ok()
for (hir_id, node) in ParentHirIterator::new(hir_id, &self) {
if match node {
Node::Item(i) => {
match i.kind {
ItemKind::Fn(..)
| ItemKind::Mod(..)
| ItemKind::Enum(..)
| ItemKind::Struct(..)
| ItemKind::Union(..)
| ItemKind::Trait(..)
| ItemKind::Impl(..) => true,
_ => false,
}
},
Node::ForeignItem(fi) => {
match fi.kind {
ForeignItemKind::Fn(..) => true,
_ => false,
}
},
Node::TraitItem(ti) => {
match ti.kind {
TraitItemKind::Method(..) => true,
_ => false,
}
},
Node::ImplItem(ii) => {
match ii.kind {
ImplItemKind::Method(..) => true,
_ => false,
}
},
Node::Block(_) => true,
_ => false,
} {
return Some(hir_id);
}
}
None
}

/// Returns the defining scope for an opaque type definition.
pub fn get_defining_scope(&self, id: HirId) -> Option<HirId> {
pub fn get_defining_scope(&self, id: HirId) -> HirId {
let mut scope = id;
loop {
scope = self.get_enclosing_scope(scope)?;
scope = self.get_enclosing_scope(scope).unwrap_or(CRATE_HIR_ID);
if scope == CRATE_HIR_ID {
return Some(CRATE_HIR_ID);
return CRATE_HIR_ID;
}
match self.get(scope) {
Node::Item(i) => {
Expand All @@ -861,7 +880,7 @@ impl<'hir> Map<'hir> {
_ => break,
}
}
Some(scope)
scope
}

pub fn get_parent_did(&self, id: HirId) -> DefId {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1563,7 +1563,7 @@ pub enum ExprKind {
/// Thus, `x.foo::<Bar, Baz>(a, b, c, d)` is represented as
/// `ExprKind::MethodCall(PathSegment { foo, [Bar, Baz] }, [x, a, b, c, d])`.
MethodCall(P<PathSegment>, Span, HirVec<Expr>),
/// A tuple (e.g., `(a, b, c ,d)`).
/// A tuple (e.g., `(a, b, c, d)`).
Tup(HirVec<Expr>),
/// A binary operation (e.g., `a + b`, `a * b`).
Binary(BinOp, P<Expr>, P<Expr>),
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/infer/opaque_types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1215,7 +1215,7 @@ pub fn may_define_opaque_type(
let mut hir_id = tcx.hir().as_local_hir_id(def_id).unwrap();

// Named opaque types can be defined by any siblings or children of siblings.
let scope = tcx.hir().get_defining_scope(opaque_hir_id).expect("could not get defining scope");
let scope = tcx.hir().get_defining_scope(opaque_hir_id);
// We walk up the node tree until we hit the root or the scope of the opaque type.
while hir_id != scope && hir_id != hir::CRATE_HIR_ID {
hir_id = tcx.hir().get_parent_item(hir_id);
Expand Down
8 changes: 6 additions & 2 deletions src/librustc_typeck/check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -620,8 +620,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
expr: &'tcx hir::Expr
) -> Ty<'tcx> {
if self.ret_coercion.is_none() {
struct_span_err!(self.tcx.sess, expr.span, E0572,
"return statement outside of function body").emit();
struct_span_err!(
self.tcx.sess,
expr.span,
E0572,
"return statement outside of function body",
).emit();
} else if let Some(ref e) = expr_opt {
if self.ret_coercion_span.borrow().is_none() {
*self.ret_coercion_span.borrow_mut() = Some(e.span);
Expand Down
4 changes: 1 addition & 3 deletions src/librustc_typeck/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1717,9 +1717,7 @@ fn find_opaque_ty_constraints(tcx: TyCtxt<'_>, def_id: DefId) -> Ty<'_> {
}

let hir_id = tcx.hir().as_local_hir_id(def_id).unwrap();
let scope = tcx.hir()
.get_defining_scope(hir_id)
.expect("could not get defining scope");
let scope = tcx.hir().get_defining_scope(hir_id);
let mut locator = ConstraintLocator {
def_id,
tcx,
Expand Down
3 changes: 0 additions & 3 deletions src/test/ui/struct-literal-variant-in-if.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,6 @@ LL | if x == E::V { field } {}
error[E0308]: mismatched types
--> $DIR/struct-literal-variant-in-if.rs:10:20
|
LL | fn test_E(x: E) {
| - help: try adding a return type: `-> bool`
LL | let field = true;
LL | if x == E::V { field } {}
| ^^^^^ expected (), found bool
|
Expand Down