bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Auto merge of #54151 - ljedrz:cleanup_hir, r=michaelwoerister

Browse source 
A few cleanups for hir

- prefer `if let` to `match` when only 1 branch matters
- `chain` iterable items that are looped over in sequence
- `sort_by_key` instead of `sort_by` when possible
- change cloning `map`s to `cloned()`
- use `unwrap_or_else` and `ok` when applicable
- a few other minor readability improvements
- whitespace fixes
This commit is contained in:
bors 2018-09-15 02:56:13 +00:00
commit ed45f9cbf4
11 changed files with 93 additions and 155 deletions
Download patch file Download diff file Expand all files Collapse all files

2
src/librustc/hir/check_attr.rs
View file

@ -125,7 +125,7 @@ impl<'a, 'tcx> CheckAttrVisitor<'a, 'tcx> {
.iter()
.filter(|attr| attr.name() == "repr")
.filter_map(|attr| attr.meta_item_list())
.flat_map(|hints| hints)
.flatten()
.collect();
let mut int_reprs = 0;

23
src/librustc/hir/intravisit.rs
View file

@ -50,7 +50,6 @@ use super::itemlikevisit::DeepVisitor;
use std::cmp;
use std::u32;
use std::result::Result::Err;
#[derive(Copy, Clone)]
pub enum FnKind<'a> {
@ -1067,31 +1066,26 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr) {
ExprKind::Break(ref destination, ref opt_expr) => {
if let Some(ref label) = destination.label {
visitor.visit_label(label);
match destination.target_id {
Ok(node_id) => visitor.visit_def_mention(Def::Label(node_id)),
Err(_) => {},
};
if let Ok(node_id) = destination.target_id {
visitor.visit_def_mention(Def::Label(node_id))
}
}
walk_list!(visitor, visit_expr, opt_expr);
}
ExprKind::Continue(ref destination) => {
if let Some(ref label) = destination.label {
visitor.visit_label(label);
match destination.target_id {
Ok(node_id) => visitor.visit_def_mention(Def::Label(node_id)),
Err(_) => {},
};
if let Ok(node_id) = destination.target_id {
visitor.visit_def_mention(Def::Label(node_id))
}
}
}
ExprKind::Ret(ref optional_expression) => {
walk_list!(visitor, visit_expr, optional_expression);
}
ExprKind::InlineAsm(_, ref outputs, ref inputs) => {
for output in outputs {
visitor.visit_expr(output)
}
for input in inputs {
visitor.visit_expr(input)
for expr in outputs.iter().chain(inputs.iter()) {
visitor.visit_expr(expr)
}
}
ExprKind::Yield(ref subexpression) => {
@ -1156,7 +1150,6 @@ impl IdRange {
self.min = cmp::min(self.min, id);
self.max = cmp::max(self.max, NodeId::from_u32(id.as_u32() + 1));
}
}

4
src/librustc/hir/lowering.rs
View file

@ -597,7 +597,7 @@ impl<'a> LoweringContext<'a> {
})
}
fn expect_full_def_from_use(&mut self, id: NodeId) -> impl Iterator<Item=Def> {
fn expect_full_def_from_use(&mut self, id: NodeId) -> impl Iterator<Item = Def> {
self.resolver.get_import(id).present_items().map(|pr| {
if pr.unresolved_segments() != 0 {
bug!("path not fully resolved: {:?}", pr);
@ -990,7 +990,7 @@ impl<'a> LoweringContext<'a> {
None => {
self.loop_scopes
.last()
.map(|innermost_loop_id| *innermost_loop_id)
.cloned()
.map(|id| Ok(self.lower_node_id(id).node_id))
.unwrap_or(Err(hir::LoopIdError::OutsideLoopScope))
.into()

4
src/librustc/hir/map/blocks.rs
View file

@ -260,9 +260,7 @@ impl<'a> FnLikeNode<'a> {
ast::ImplItemKind::Method(ref sig, body) => {
method(ii.id, ii.ident, sig, Some(&ii.vis), body, ii.span, &ii.attrs)
}
_ => {
bug!("impl method FnLikeNode that is not fn-like")
}
_ => bug!("impl method FnLikeNode that is not fn-like")
}
},
map::Node::Expr(e) => match e.node {

22
src/librustc/hir/map/collector.rs
View file

@ -67,7 +67,6 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
// them explicitly here.
ref attrs,
span,
// These fields are handled separately:
exported_macros: _,
items: _,
@ -128,14 +127,14 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
cstore: &dyn CrateStore,
source_map: &SourceMap,
commandline_args_hash: u64)
-> (Vec<Option<Entry<'hir>>>, Svh) {
self.hir_body_nodes
.sort_unstable_by(|&(ref d1, _), &(ref d2, _)| d1.cmp(d2));
-> (Vec<Option<Entry<'hir>>>, Svh)
{
self.hir_body_nodes.sort_unstable_by_key(|bn| bn.0);
let node_hashes = self
.hir_body_nodes
.iter()
.fold(Fingerprint::ZERO, |fingerprint , &(def_path_hash, dep_node_index)| {
.fold(Fingerprint::ZERO, |fingerprint, &(def_path_hash, dep_node_index)| {
fingerprint.combine(
def_path_hash.0.combine(self.dep_graph.fingerprint_of(dep_node_index))
)
@ -143,15 +142,12 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
let mut upstream_crates: Vec<_> = cstore.crates_untracked().iter().map(|&cnum| {
let name = cstore.crate_name_untracked(cnum).as_str();
let disambiguator = cstore.crate_disambiguator_untracked(cnum)
.to_fingerprint();
let disambiguator = cstore.crate_disambiguator_untracked(cnum).to_fingerprint();
let hash = cstore.crate_hash_untracked(cnum);
(name, disambiguator, hash)
}).collect();
upstream_crates.sort_unstable_by(|&(name1, dis1, _), &(name2, dis2, _)| {
(name1, dis1).cmp(&(name2, dis2))
});
upstream_crates.sort_unstable_by_key(|&(name, dis, _)| (name, dis));
// We hash the final, remapped names of all local source files so we
// don't have to include the path prefix remapping commandline args.
@ -229,7 +225,6 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
}
self.insert_entry(id, entry);
}
fn with_parent<F: FnOnce(&mut Self)>(&mut self, parent_id: NodeId, f: F) {
@ -311,15 +306,12 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
self.with_dep_node_owner(i.hir_id.owner, i, |this| {
this.insert(i.id, Node::Item(i));
this.with_parent(i.id, |this| {
match i.node {
ItemKind::Struct(ref struct_def, _) => {
if let ItemKind::Struct(ref struct_def, _) = i.node {
// If this is a tuple-like struct, register the constructor.
if !struct_def.is_struct() {
this.insert(struct_def.id(), Node::StructCtor(struct_def));
}
}
_ => {}
}
intravisit::walk_item(this, i);
});
});

5
src/librustc/hir/map/def_collector.rs
View file

@ -348,14 +348,11 @@ impl<'a> visit::Visitor<'a> for DefCollector<'a> {
fn visit_token(&mut self, t: Token) {
if let Token::Interpolated(nt) = t {
match nt.0 {
token::NtExpr(ref expr) => {
if let token::NtExpr(ref expr) = nt.0 {
if let ExprKind::Mac(..) = expr.node {
self.visit_macro_invoc(expr.id);
}
}
_ => {}
}
}
}
}

6
src/librustc/hir/map/definitions.rs
View file

@ -317,10 +317,8 @@ pub enum DefPathData {
// they are treated specially by the `def_path` function.
/// The crate root (marker)
CrateRoot,
// Catch-all for random DefId things like DUMMY_NODE_ID
Misc,
// Different kinds of items and item-like things:
/// An impl
Impl,
@ -342,7 +340,6 @@ pub enum DefPathData {
MacroDef(InternedString),
/// A closure expression
ClosureExpr,
// Subportions of items
/// A type parameter (generic parameter)
TypeParam(InternedString),
@ -358,7 +355,6 @@ pub enum DefPathData {
AnonConst,
/// An `impl Trait` type node
ImplTrait,
/// GlobalMetaData identifies a piece of crate metadata that is global to
/// a whole crate (as opposed to just one item). GlobalMetaData components
/// are only supposed to show up right below the crate root.
@ -656,10 +652,8 @@ impl DefPathData {
GlobalMetaData(name) => {
return name
}
// note that this does not show up in user printouts
CrateRoot => "{{root}}",
Impl => "{{impl}}",
Misc => "{{?}}",
ClosureExpr => "{{closure}}",

4
src/librustc/hir/map/hir_id_validator.rs
View file

@ -100,8 +100,8 @@ impl<'a, 'hir: 'a> HirIdValidator<'a, 'hir> {
if max != self.hir_ids_seen.len() - 1 {
// Collect the missing ItemLocalIds
let missing: Vec<_> = (0 .. max + 1)
.filter(|&i| !self.hir_ids_seen.contains_key(&ItemLocalId(i as u32)))
let missing: Vec<_> = (0 .. max as u32 + 1)
.filter(|&i| !self.hir_ids_seen.contains_key(&ItemLocalId(i)))
.collect();
// Try to map those to something more useful

64
src/librustc/hir/map/mod.rs
View file

@ -291,8 +291,7 @@ impl<'hir> Map<'hir> {
};
match item.node {
ItemKind::Static(_, m, _) => Some(Def::Static(def_id(),
m == MutMutable)),
ItemKind::Static(_, m, _) => Some(Def::Static(def_id(), m == MutMutable)),
ItemKind::Const(..) => Some(Def::Const(def_id())),
ItemKind::Fn(..) => Some(Def::Fn(def_id())),
ItemKind::Mod(..) => Some(Def::Mod(def_id())),
@ -408,7 +407,7 @@ impl<'hir> Map<'hir> {
pub fn fn_decl(&self, node_id: ast::NodeId) -> Option<FnDecl> {
if let Some(entry) = self.find_entry(node_id) {
entry.fn_decl().map(|fd| fd.clone())
entry.fn_decl().cloned()
} else {
bug!("no entry for node_id `{}`", node_id)
}
@ -471,18 +470,13 @@ impl<'hir> Map<'hir> {
match self.get(id) {
Node::Item(&Item { node: ItemKind::Trait(..), .. }) => id,
Node::GenericParam(_) => self.get_parent_node(id),
_ => {
bug!("ty_param_owner: {} not a type parameter",
self.node_to_string(id))
}
_ => bug!("ty_param_owner: {} not a type parameter", self.node_to_string(id))
}
}
pub fn ty_param_name(&self, id: NodeId) -> Name {
match self.get(id) {
Node::Item(&Item { node: ItemKind::Trait(..), .. }) => {
keywords::SelfType.name()
}
Node::Item(&Item { node: ItemKind::Trait(..), .. }) => keywords::SelfType.name(),
Node::GenericParam(param) => param.name.ident().name,
_ => bug!("ty_param_name: {} not a type parameter", self.node_to_string(id)),
}
@ -521,10 +515,8 @@ impl<'hir> Map<'hir> {
/// Retrieve the Node corresponding to `id`, panicking if it cannot
/// be found.
pub fn get(&self, id: NodeId) -> Node<'hir> {
match self.find(id) {
Some(node) => node, // read recorded by `find`
None => bug!("couldn't find node id {} in the AST map", id)
}
// read recorded by `find`
self.find(id).unwrap_or_else(|| bug!("couldn't find node id {} in the AST map", id))
}
pub fn get_if_local(&self, id: DefId) -> Option<Node<'hir>> {
@ -697,10 +689,7 @@ impl<'hir> Map<'hir> {
}
};
match self.walk_parent_nodes(id, match_fn, match_non_returning_block) {
Ok(id) => Some(id),
Err(_) => None,
}
self.walk_parent_nodes(id, match_fn, match_non_returning_block).ok()
}
/// Retrieve the NodeId for `id`'s parent item, or `id` itself if no
@ -738,17 +727,14 @@ impl<'hir> Map<'hir> {
/// and associated types probably shouldn't, for example. Behavior in this
/// regard should be expected to be highly unstable.
pub fn get_enclosing_scope(&self, id: NodeId) -> Option<NodeId> {
match self.walk_parent_nodes(id, |node| match *node {
self.walk_parent_nodes(id, |node| match *node {
Node::Item(_) |
Node::ForeignItem(_) |
Node::TraitItem(_) |
Node::ImplItem(_) |
Node::Block(_) => true,
_ => false,
}, |_| false) {
Ok(id) => Some(id),
Err(_) => None,
}
}, |_| false).ok()
}
pub fn get_parent_did(&self, id: NodeId) -> DefId {
@ -758,14 +744,12 @@ impl<'hir> Map<'hir> {
pub fn get_foreign_abi(&self, id: NodeId) -> Abi {
let parent = self.get_parent(id);
if let Some(entry) = self.find_entry(parent) {
match entry {
Entry { node: Node::Item(Item { node: ItemKind::ForeignMod(ref nm), .. }), .. }
=> {
if let Entry {
node: Node::Item(Item { node: ItemKind::ForeignMod(ref nm), .. }), .. } = entry
{
self.read(id); // reveals some of the content of a node
return nm.abi;
}
_ => {}
}
}
bug!("expected foreign mod or inlined parent, found {}", self.node_to_string(parent))
}
@ -797,18 +781,12 @@ impl<'hir> Map<'hir> {
match i.node {
ItemKind::Struct(ref struct_def, _) |
ItemKind::Union(ref struct_def, _) => struct_def,
_ => {
bug!("struct ID bound to non-struct {}",
self.node_to_string(id));
}
_ => bug!("struct ID bound to non-struct {}", self.node_to_string(id))
}
}
Some(Node::StructCtor(data)) => data,
Some(Node::Variant(variant)) => &variant.node.data,
_ => {
bug!("expected struct or variant, found {}",
self.node_to_string(id));
}
_ => bug!("expected struct or variant, found {}", self.node_to_string(id))
}
}
@ -866,9 +844,7 @@ impl<'hir> Map<'hir> {
Some(Node::GenericParam(param)) => Some(&param.attrs[..]),
// unit/tuple structs take the attributes straight from
// the struct definition.
Some(Node::StructCtor(_)) => {
return self.attrs(self.get_parent(id));
}
Some(Node::StructCtor(_)) => return self.attrs(self.get_parent(id)),
_ => None
};
attrs.unwrap_or(&[])
@ -917,7 +893,6 @@ impl<'hir> Map<'hir> {
Some(Node::Visibility(v)) => bug!("unexpected Visibility {:?}", v),
Some(Node::Local(local)) => local.span,
Some(Node::MacroDef(macro_def)) => macro_def.span,
Some(Node::Crate) => self.forest.krate.span,
None => bug!("hir::map::Map::span: id not in map: {:?}", id),
}
@ -976,10 +951,10 @@ impl<'a, 'hir> NodesMatchingSuffix<'a, 'hir> {
// chain, then returns `None`.
fn find_first_mod_parent<'a>(map: &'a Map, mut id: NodeId) -> Option<(NodeId, Name)> {
loop {
match map.find(id)? {
Node::Item(item) if item_is_mod(&item) =>
return Some((id, item.name)),
_ => {}
if let Node::Item(item) = map.find(id)? {
if item_is_mod(&item) {
return Some((id, item.name))
}
}
let parent = map.get_parent(id);
if parent == id { return None }
@ -1041,7 +1016,6 @@ impl Named for StructField { fn name(&self) -> Name { self.ident.name } }
impl Named for TraitItem { fn name(&self) -> Name { self.ident.name } }
impl Named for ImplItem { fn name(&self) -> Name { self.ident.name } }
pub fn map_crate<'hir>(sess: &::session::Session,
cstore: &dyn CrateStore,
forest: &'hir mut Forest,

9
src/librustc/hir/mod.rs
View file

@ -831,9 +831,10 @@ impl Pat {
s.walk_(it)
}
PatKind::Slice(ref before, ref slice, ref after) => {
before.iter().all(|p| p.walk_(it)) &&
slice.iter().all(|p| p.walk_(it)) &&
after.iter().all(|p| p.walk_(it))
before.iter()
.chain(slice.iter())
.chain(after.iter())
.all(|p| p.walk_(it))
}
PatKind::Wild |
PatKind::Lit(_) |
@ -1658,7 +1659,6 @@ pub struct TypeBinding {
pub span: Span,
}
#[derive(Clone, RustcEncodable, RustcDecodable)]
pub struct Ty {
pub id: NodeId,
@ -2284,7 +2284,6 @@ pub type TraitMap = NodeMap<Vec<TraitCandidate>>;
// imported.
pub type GlobMap = NodeMap<FxHashSet<Name>>;
pub fn provide(providers: &mut Providers) {
providers.describe_def = map::describe_def;
}

25
src/librustc/hir/print.rs
View file

@ -251,6 +251,7 @@ impl<'a> State<'a> {
pub fn bclose_(&mut self, span: syntax_pos::Span, indented: usize) -> io::Result<()> {
self.bclose_maybe_open(span, indented, true)
}
pub fn bclose_maybe_open(&mut self,
span: syntax_pos::Span,
indented: usize,
@ -264,6 +265,7 @@ impl<'a> State<'a> {
}
Ok(())
}
pub fn bclose(&mut self, span: syntax_pos::Span) -> io::Result<()> {
self.bclose_(span, indent_unit)
}
@ -274,12 +276,14 @@ impl<'a> State<'a> {
None => false,
}
}
pub fn space_if_not_bol(&mut self) -> io::Result<()> {
if !self.is_bol() {
self.s.space()?;
}
Ok(())
}
pub fn break_offset_if_not_bol(&mut self, n: usize, off: isize) -> io::Result<()> {
if !self.is_bol() {
self.s.break_offset(n, off)
@ -304,7 +308,6 @@ impl<'a> State<'a> {
self.s.word("*/")
}
pub fn commasep_cmnt<T, F, G>(&mut self,
b: Breaks,
elts: &[T],
@ -689,21 +692,15 @@ impl<'a> State<'a> {
self.s.space()?;
}
match polarity {
hir::ImplPolarity::Negative => {
if let hir::ImplPolarity::Negative = polarity {
self.s.word("!")?;
}
_ => {}
}
match opt_trait {
&Some(ref t) => {
if let Some(ref t) = opt_trait {
self.print_trait_ref(t)?;
self.s.space()?;
self.word_space("for")?;
}
&None => {}
}
self.print_type(&ty)?;
self.print_where_clause(&generics.where_clause)?;
@ -1061,14 +1058,11 @@ impl<'a> State<'a> {
for st in &blk.stmts {
self.print_stmt(st)?;
}
match blk.expr {
Some(ref expr) => {
if let Some(ref expr) = blk.expr {
self.space_if_not_bol()?;
self.print_expr(&expr)?;
self.maybe_print_trailing_comment(expr.span, Some(blk.span.hi()))?;
}
_ => (),
}
self.bclose_maybe_open(blk.span, indented, close_box)?;
self.ann.post(self, AnnNode::Block(blk))
}
@ -1479,13 +1473,10 @@ impl<'a> State<'a> {
}
hir::ExprKind::Ret(ref result) => {
self.s.word("return")?;
match *result {
Some(ref expr) => {
if let Some(ref expr) = *result {
self.s.word(" ")?;
self.print_expr_maybe_paren(&expr, parser::PREC_JUMP)?;
}
_ => (),
}
}
hir::ExprKind::InlineAsm(ref a, ref outputs, ref inputs) => {
self.s.word("asm!")?;