Skip to content

Commit

Permalink
Avoid comparing fields by name when possible
Browse files Browse the repository at this point in the history
Resolve them into field indices once and then use those resolutions

+ Fix rebase
  • Loading branch information
petrochenkov committed Apr 12, 2018
1 parent 44acea4 commit 4f69b7f
Show file tree
Hide file tree
Showing 27 changed files with 244 additions and 226 deletions.
2 changes: 2 additions & 0 deletions src/librustc/hir/intravisit.rs
Expand Up @@ -658,6 +658,7 @@ pub fn walk_pat<'v, V: Visitor<'v>>(visitor: &mut V, pattern: &'v Pat) {
PatKind::Struct(ref qpath, ref fields, _) => {
visitor.visit_qpath(qpath, pattern.id, pattern.span);
for field in fields {
visitor.visit_id(field.node.id);
visitor.visit_name(field.span, field.node.name);
visitor.visit_pat(&field.node.pat)
}
Expand Down Expand Up @@ -959,6 +960,7 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr) {
ExprStruct(ref qpath, ref fields, ref optional_base) => {
visitor.visit_qpath(qpath, expression.id, expression.span);
for field in fields {
visitor.visit_id(field.id);
visitor.visit_name(field.name.span, field.name.node);
visitor.visit_expr(&field.expr)
}
Expand Down
3 changes: 3 additions & 0 deletions src/librustc/hir/lowering.rs
Expand Up @@ -2100,6 +2100,7 @@ impl<'a> LoweringContext<'a> {

fn lower_field(&mut self, f: &Field) -> hir::Field {
hir::Field {
id: self.next_id().node_id,
name: respan(f.ident.span, self.lower_ident(f.ident)),
expr: P(self.lower_expr(&f.expr)),
span: f.span,
Expand Down Expand Up @@ -2863,6 +2864,7 @@ impl<'a> LoweringContext<'a> {
.map(|f| Spanned {
span: f.span,
node: hir::FieldPat {
id: self.next_id().node_id,
name: self.lower_ident(f.node.ident),
pat: self.lower_pat(&f.node.pat),
is_shorthand: f.node.is_shorthand,
Expand Down Expand Up @@ -3741,6 +3743,7 @@ impl<'a> LoweringContext<'a> {

fn field(&mut self, name: Name, expr: P<hir::Expr>, span: Span) -> hir::Field {
hir::Field {
id: self.next_id().node_id,
name: Spanned { node: name, span },
span,
expr,
Expand Down
2 changes: 2 additions & 0 deletions src/librustc/hir/mod.rs
Expand Up @@ -827,6 +827,7 @@ impl Pat {
/// except is_shorthand is true
#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
pub struct FieldPat {
pub id: NodeId,
/// The identifier for the field
pub name: Name,
/// The pattern the field is destructured to
Expand Down Expand Up @@ -1172,6 +1173,7 @@ pub struct Arm {

#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
pub struct Field {
pub id: NodeId,
pub name: Spanned<Name>,
pub expr: P<Expr>,
pub span: Span,
Expand Down
46 changes: 35 additions & 11 deletions src/librustc/ich/impls_hir.rs
Expand Up @@ -420,11 +420,23 @@ impl<'a> HashStable<StableHashingContext<'a>> for hir::Pat {
}

impl_stable_hash_for_spanned!(hir::FieldPat);
impl_stable_hash_for!(struct hir::FieldPat {
name,
pat,
is_shorthand
});

impl<'a> HashStable<StableHashingContext<'a>> for hir::FieldPat {
fn hash_stable<W: StableHasherResult>(&self,
hcx: &mut StableHashingContext<'a>,
hasher: &mut StableHasher<W>) {
let hir::FieldPat {
id: _,
name,
ref pat,
is_shorthand,
} = *self;

name.hash_stable(hcx, hasher);
pat.hash_stable(hcx, hasher);
is_shorthand.hash_stable(hcx, hasher);
}
}

impl_stable_hash_for!(enum hir::BindingAnnotation {
Unannotated,
Expand Down Expand Up @@ -507,12 +519,24 @@ impl_stable_hash_for!(struct hir::Arm {
body
});

impl_stable_hash_for!(struct hir::Field {
name,
expr,
span,
is_shorthand
});
impl<'a> HashStable<StableHashingContext<'a>> for hir::Field {
fn hash_stable<W: StableHasherResult>(&self,
hcx: &mut StableHashingContext<'a>,
hasher: &mut StableHasher<W>) {
let hir::Field {
id: _,
name,
ref expr,
span,
is_shorthand,
} = *self;

name.hash_stable(hcx, hasher);
expr.hash_stable(hcx, hasher);
span.hash_stable(hcx, hasher);
is_shorthand.hash_stable(hcx, hasher);
}
}

impl_stable_hash_for_spanned!(ast::Name);

Expand Down
37 changes: 15 additions & 22 deletions src/librustc/middle/dead.rs
Expand Up @@ -13,7 +13,7 @@
// from live codes are live, and everything else is dead.

use hir::map as hir_map;
use hir::{self, Item_, PatKind};
use hir::{self, PatKind};
use hir::intravisit::{self, Visitor, NestedVisitorMap};
use hir::itemlikevisit::ItemLikeVisitor;

Expand Down Expand Up @@ -99,10 +99,11 @@ impl<'a, 'tcx> MarkSymbolVisitor<'a, 'tcx> {
self.check_def_id(self.tables.type_dependent_defs()[id].def_id());
}

fn handle_field_access(&mut self, lhs: &hir::Expr, name: ast::Name) {
fn handle_field_access(&mut self, lhs: &hir::Expr, node_id: ast::NodeId) {
match self.tables.expr_ty_adjusted(lhs).sty {
ty::TyAdt(def, _) => {
self.insert_def_id(def.non_enum_variant().field_named(name).did);
let index = self.tcx.field_index(node_id, self.tables);
self.insert_def_id(def.non_enum_variant().fields[index].did);
}
ty::TyTuple(..) => {}
_ => span_bug!(lhs.span, "named field access on non-ADT"),
Expand All @@ -119,7 +120,8 @@ impl<'a, 'tcx> MarkSymbolVisitor<'a, 'tcx> {
if let PatKind::Wild = pat.node.pat.node {
continue;
}
self.insert_def_id(variant.field_named(pat.node.name).did);
let index = self.tcx.field_index(pat.node.id, self.tables);
self.insert_def_id(variant.fields[index].did);
}
}

Expand Down Expand Up @@ -182,18 +184,11 @@ impl<'a, 'tcx> MarkSymbolVisitor<'a, 'tcx> {
self.inherited_pub_visibility = had_inherited_pub_visibility;
}

fn mark_as_used_if_union(&mut self, did: DefId, fields: &hir::HirVec<hir::Field>) {
if let Some(node_id) = self.tcx.hir.as_local_node_id(did) {
if let Some(hir_map::NodeItem(item)) = self.tcx.hir.find(node_id) {
if let Item_::ItemUnion(ref variant, _) = item.node {
if variant.fields().len() > 1 {
for field in variant.fields() {
if fields.iter().find(|x| x.name.node == field.name).is_some() {
self.live_symbols.insert(field.id);
}
}
}
}
fn mark_as_used_if_union(&mut self, adt: &ty::AdtDef, fields: &hir::HirVec<hir::Field>) {
if adt.is_union() && adt.non_enum_variant().fields.len() > 1 && adt.did.is_local() {
for field in fields {
let index = self.tcx.field_index(field.id, self.tables);
self.insert_def_id(adt.non_enum_variant().fields[index].did);
}
}
}
Expand Down Expand Up @@ -233,14 +228,12 @@ impl<'a, 'tcx> Visitor<'tcx> for MarkSymbolVisitor<'a, 'tcx> {
hir::ExprMethodCall(..) => {
self.lookup_and_handle_method(expr.hir_id);
}
hir::ExprField(ref lhs, ref name) => {
self.handle_field_access(&lhs, name.node);
hir::ExprField(ref lhs, ..) => {
self.handle_field_access(&lhs, expr.id);
}
hir::ExprStruct(_, ref fields, _) => {
if let ty::TypeVariants::TyAdt(ref def, _) = self.tables.expr_ty(expr).sty {
if def.is_union() {
self.mark_as_used_if_union(def.did, fields);
}
if let ty::TypeVariants::TyAdt(ref adt, _) = self.tables.expr_ty(expr).sty {
self.mark_as_used_if_union(adt, fields);
}
}
_ => ()
Expand Down
16 changes: 6 additions & 10 deletions src/librustc/middle/expr_use_visitor.rs
Expand Up @@ -659,11 +659,15 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
match with_cmt.ty.sty {
ty::TyAdt(adt, substs) if adt.is_struct() => {
// Consume those fields of the with expression that are needed.
for with_field in &adt.non_enum_variant().fields {
if !contains_field_named(with_field, fields) {
for (f_index, with_field) in adt.non_enum_variant().fields.iter().enumerate() {
let is_mentioned = fields.iter().any(|f| {
self.tcx().field_index(f.id, self.mc.tables) == f_index
});
if !is_mentioned {
let cmt_field = self.mc.cat_field(
&*with_expr,
with_cmt.clone(),
f_index,
with_field.name,
with_field.ty(self.tcx(), substs)
);
Expand All @@ -687,14 +691,6 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
// walk the with expression so that complex expressions
// are properly handled.
self.walk_expr(with_expr);

fn contains_field_named(field: &ty::FieldDef,
fields: &[hir::Field])
-> bool
{
fields.iter().any(
|f| f.name.node == field.name)
}
}

// Invoke the appropriate delegate calls for anything that gets
Expand Down
39 changes: 28 additions & 11 deletions src/librustc/middle/mem_categorization.rs
Expand Up @@ -84,6 +84,7 @@ use syntax::ast::{self, Name};
use syntax_pos::Span;

use std::fmt;
use std::hash::{Hash, Hasher};
use rustc_data_structures::sync::Lrc;
use std::rc::Rc;
use util::nodemap::ItemLocalSet;
Expand Down Expand Up @@ -132,9 +133,22 @@ pub enum InteriorKind {
InteriorElement(InteriorOffsetKind),
}

// FIXME: Use actual index instead of `ast::Name` with questionable hygiene
#[derive(Clone, Copy, PartialEq, Eq, Hash)]
pub struct FieldIndex(pub ast::Name);
// Contains index of a field that is actually used for loan path comparisons and
// string representation of the field that should be used only for diagnostics.
#[derive(Clone, Copy, Eq)]
pub struct FieldIndex(pub usize, pub Name);

impl PartialEq for FieldIndex {
fn eq(&self, rhs: &Self) -> bool {
self.0 == rhs.0
}
}

impl Hash for FieldIndex {
fn hash<H: Hasher>(&self, h: &mut H) {
self.0.hash(h)
}
}

#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug)]
pub enum InteriorOffsetKind {
Expand Down Expand Up @@ -195,7 +209,7 @@ pub enum ImmutabilityBlame<'tcx> {
}

impl<'tcx> cmt_<'tcx> {
fn resolve_field(&self, field_name: Name) -> Option<(&'tcx ty::AdtDef, &'tcx ty::FieldDef)>
fn resolve_field(&self, field_index: usize) -> Option<(&'tcx ty::AdtDef, &'tcx ty::FieldDef)>
{
let adt_def = match self.ty.sty {
ty::TyAdt(def, _) => def,
Expand All @@ -212,7 +226,7 @@ impl<'tcx> cmt_<'tcx> {
&adt_def.variants[0]
}
};
Some((adt_def, variant_def.field_named(field_name)))
Some((adt_def, &variant_def.fields[field_index]))
}

pub fn immutability_blame(&self) -> Option<ImmutabilityBlame<'tcx>> {
Expand Down Expand Up @@ -639,7 +653,8 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
expr.id,
expr,
base_cmt);
Ok(self.cat_field(expr, base_cmt, f_name.node, expr_ty))
let f_index = self.tcx.field_index(expr.id, self.tables);
Ok(self.cat_field(expr, base_cmt, f_index, f_name.node, expr_ty))
}

hir::ExprIndex(ref base, _) => {
Expand Down Expand Up @@ -967,14 +982,15 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
pub fn cat_field<N:ast_node>(&self,
node: &N,
base_cmt: cmt<'tcx>,
f_index: usize,
f_name: Name,
f_ty: Ty<'tcx>)
-> cmt<'tcx> {
let ret = Rc::new(cmt_ {
id: node.id(),
span: node.span(),
mutbl: base_cmt.mutbl.inherit(),
cat: Categorization::Interior(base_cmt, InteriorField(FieldIndex(f_name))),
cat: Categorization::Interior(base_cmt, InteriorField(FieldIndex(f_index, f_name))),
ty: f_ty,
note: NoteNone
});
Expand Down Expand Up @@ -1262,7 +1278,7 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {

for (i, subpat) in subpats.iter().enumerate_and_adjust(expected_len, ddpos) {
let subpat_ty = self.pat_ty(&subpat)?; // see (*2)
let interior = InteriorField(FieldIndex(Name::intern(&i.to_string())));
let interior = InteriorField(FieldIndex(i, Name::intern(&i.to_string())));
let subcmt = self.cat_imm_interior(pat, cmt.clone(), subpat_ty, interior);
self.cat_pattern_(subcmt, &subpat, op)?;
}
Expand All @@ -1285,7 +1301,8 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {

for fp in field_pats {
let field_ty = self.pat_ty(&fp.node.pat)?; // see (*2)
let cmt_field = self.cat_field(pat, cmt.clone(), fp.node.name, field_ty);
let f_index = self.tcx.field_index(fp.node.id, self.tables);
let cmt_field = self.cat_field(pat, cmt.clone(), f_index, fp.node.name, field_ty);
self.cat_pattern_(cmt_field, &fp.node.pat, op)?;
}
}
Expand All @@ -1302,7 +1319,7 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
};
for (i, subpat) in subpats.iter().enumerate_and_adjust(expected_len, ddpos) {
let subpat_ty = self.pat_ty(&subpat)?; // see (*2)
let interior = InteriorField(FieldIndex(Name::intern(&i.to_string())));
let interior = InteriorField(FieldIndex(i, Name::intern(&i.to_string())));
let subcmt = self.cat_imm_interior(pat, cmt.clone(), subpat_ty, interior);
self.cat_pattern_(subcmt, &subpat, op)?;
}
Expand Down Expand Up @@ -1521,7 +1538,7 @@ pub fn ptr_sigil(ptr: PointerKind) -> &'static str {
impl fmt::Debug for InteriorKind {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match *self {
InteriorField(FieldIndex(name)) => write!(f, "{}", name),
InteriorField(FieldIndex(_, info)) => write!(f, "{}", info),
InteriorElement(..) => write!(f, "[]"),
}
}
Expand Down
23 changes: 23 additions & 0 deletions src/librustc/ty/context.rs
Expand Up @@ -346,6 +346,12 @@ pub struct TypeckTables<'tcx> {
/// method calls, including those of overloaded operators.
type_dependent_defs: ItemLocalMap<Def>,

/// Resolved field indices for field accesses in expressions (`S { field }`, `obj.field`)
/// or patterns (`S { field }`). The index is often useful by itself, but to learn more
/// about the field you also need definition of the variant to which the field
/// belongs, but it may not exist if it's a tuple field (`tuple.0`).
field_indices: ItemLocalMap<usize>,

/// Stores the canonicalized types provided by the user. See also `UserAssertTy` statement in
/// MIR.
user_provided_tys: ItemLocalMap<CanonicalTy<'tcx>>,
Expand Down Expand Up @@ -426,6 +432,7 @@ impl<'tcx> TypeckTables<'tcx> {
TypeckTables {
local_id_root,
type_dependent_defs: ItemLocalMap(),
field_indices: ItemLocalMap(),
user_provided_tys: ItemLocalMap(),
node_types: ItemLocalMap(),
node_substs: ItemLocalMap(),
Expand Down Expand Up @@ -468,6 +475,20 @@ impl<'tcx> TypeckTables<'tcx> {
}
}

pub fn field_indices(&self) -> LocalTableInContext<usize> {
LocalTableInContext {
local_id_root: self.local_id_root,
data: &self.field_indices
}
}

pub fn field_indices_mut(&mut self) -> LocalTableInContextMut<usize> {
LocalTableInContextMut {
local_id_root: self.local_id_root,
data: &mut self.field_indices
}
}

pub fn user_provided_tys(&self) -> LocalTableInContext<CanonicalTy<'tcx>> {
LocalTableInContext {
local_id_root: self.local_id_root,
Expand Down Expand Up @@ -706,6 +727,7 @@ impl<'a, 'gcx> HashStable<StableHashingContext<'a>> for TypeckTables<'gcx> {
let ty::TypeckTables {
local_id_root,
ref type_dependent_defs,
ref field_indices,
ref user_provided_tys,
ref node_types,
ref node_substs,
Expand All @@ -726,6 +748,7 @@ impl<'a, 'gcx> HashStable<StableHashingContext<'a>> for TypeckTables<'gcx> {

hcx.with_node_id_hashing_mode(NodeIdHashingMode::HashDefPath, |hcx| {
type_dependent_defs.hash_stable(hcx, hasher);
field_indices.hash_stable(hcx, hasher);
user_provided_tys.hash_stable(hcx, hasher);
node_types.hash_stable(hcx, hasher);
node_substs.hash_stable(hcx, hasher);
Expand Down

0 comments on commit 4f69b7f

Please sign in to comment.