Auto merge of #49718 - petrochenkov:fieldcmp, r=eddyb

Hygiene 2.0: Avoid comparing fields by name

There are two separate commits here (not counting tests):
- The first one unifies named (`obj.name`) and numeric (`obj.0`) field access expressions in AST and HIR. Before field references in these expressions are resolved it doesn't matter whether the field is named or numeric (it's just a symbol) and 99% of code is common. After field references are resolved we work with
them by index for all fields (see the second commit), so it's again not important whether the field was named or numeric (this includes MIR where all fields were already by index).
(This refactoring actually fixed some bugs in HIR-based borrow checker where borrows through names (`S {
0: ref x }`) and indices (`&s.0`) weren't considered overlapping.)
- The second commit removes all by-name field comparison and instead resolves field references to their indices  once, and then uses those resolutions. (There are still a few name comparisons in save-analysis, because save-analysis is weird, but they are made correctly hygienic).
Thus we are fixing a bunch of "secondary" field hygiene bugs (in borrow checker, lints).

Fixes #46314

Author: bors

src/librustc/cfg/construct.rs

@@ -389,7 +389,6 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
             hir::ExprType(ref e, _) |
             hir::ExprUnary(_, ref e) |
             hir::ExprField(ref e, _) |
-            hir::ExprTupField(ref e, _) |
             hir::ExprYield(ref e) |
             hir::ExprRepeat(ref e, _) => {
                 self.straightline(expr, pred, Some(&**e).into_iter())

src/librustc/hir/intravisit.rs

@@ -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)
             }
@@ -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)
             }
@@ -1025,9 +1027,6 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr) {
             visitor.visit_expr(subexpression);
             visitor.visit_name(name.span, name.node);
         }
-        ExprTupField(ref subexpression, _) => {
-            visitor.visit_expr(subexpression);
-        }
         ExprIndex(ref main_expression, ref index_expression) => {
             visitor.visit_expr(main_expression);
             visitor.visit_expr(index_expression)

src/librustc/hir/lowering.rs

@@ -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,
@@ -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,
@@ -3095,7 +3097,6 @@ impl<'a> LoweringContext<'a> {
                 P(self.lower_expr(el)),
                 respan(ident.span, self.lower_ident(ident)),
             ),
-            ExprKind::TupField(ref el, ident) => hir::ExprTupField(P(self.lower_expr(el)), ident),
             ExprKind::Index(ref el, ref er) => {
                 hir::ExprIndex(P(self.lower_expr(el)), P(self.lower_expr(er)))
             }
@@ -3742,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,

src/librustc/hir/mod.rs

@@ -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
@@ -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,
@@ -1276,7 +1278,6 @@ impl Expr {
             ExprAssign(..) => ExprPrecedence::Assign,
             ExprAssignOp(..) => ExprPrecedence::AssignOp,
             ExprField(..) => ExprPrecedence::Field,
-            ExprTupField(..) => ExprPrecedence::TupField,
             ExprIndex(..) => ExprPrecedence::Index,
             ExprPath(..) => ExprPrecedence::Path,
             ExprAddrOf(..) => ExprPrecedence::AddrOf,
@@ -1363,12 +1364,8 @@ pub enum Expr_ {
     ///
     /// For example, `a += 1`.
     ExprAssignOp(BinOp, P<Expr>, P<Expr>),
-    /// Access of a named struct field (`obj.foo`)
+    /// Access of a named (`obj.foo`) or unnamed (`obj.0`) struct or tuple field
     ExprField(P<Expr>, Spanned<Name>),
-    /// Access of an unnamed field of a struct or tuple-struct
-    ///
-    /// For example, `foo.0`.
-    ExprTupField(P<Expr>, Spanned<usize>),
     /// An indexing operation (`foo[2]`)
     ExprIndex(P<Expr>, P<Expr>),
 

src/librustc/hir/print.rs

@@ -1201,8 +1201,7 @@ impl<'a> State<'a> {
     fn print_expr_call(&mut self, func: &hir::Expr, args: &[hir::Expr]) -> io::Result<()> {
         let prec =
             match func.node {
-                hir::ExprField(..) |
-                hir::ExprTupField(..) => parser::PREC_FORCE_PAREN,
+                hir::ExprField(..) => parser::PREC_FORCE_PAREN,
                 _ => parser::PREC_POSTFIX,
             };
 
@@ -1405,11 +1404,6 @@ impl<'a> State<'a> {
                 self.s.word(".")?;
                 self.print_name(name.node)?;
             }
-            hir::ExprTupField(ref expr, id) => {
-                self.print_expr_maybe_paren(&expr, parser::PREC_POSTFIX)?;
-                self.s.word(".")?;
-                self.print_usize(id.node)?;
-            }
             hir::ExprIndex(ref expr, ref index) => {
                 self.print_expr_maybe_paren(&expr, parser::PREC_POSTFIX)?;
                 self.s.word("[")?;
@@ -2376,7 +2370,6 @@ fn contains_exterior_struct_lit(value: &hir::Expr) -> bool {
         hir::ExprCast(ref x, _) |
         hir::ExprType(ref x, _) |
         hir::ExprField(ref x, _) |
-        hir::ExprTupField(ref x, _) |
         hir::ExprIndex(ref x, _) => {
             // &X { y: 1 }, X { y: 1 }.y
             contains_exterior_struct_lit(&x)

src/librustc/ich/impls_hir.rs

@@ -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,
@@ -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);
 
@@ -569,7 +593,6 @@ impl_stable_hash_for!(enum hir::Expr_ {
     ExprAssign(lhs, rhs),
     ExprAssignOp(op, lhs, rhs),
     ExprField(owner, field_name),
-    ExprTupField(owner, idx),
     ExprIndex(lhs, rhs),
     ExprPath(path),
     ExprAddrOf(mutability, sub),

src/librustc/middle/dead.rs

@@ -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;
 
@@ -99,22 +99,14 @@ 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);
-            }
-            _ => span_bug!(lhs.span, "named field access on non-ADT"),
-        }
-    }
-
-    fn handle_tup_field_access(&mut self, lhs: &hir::Expr, idx: usize) {
-        match self.tables.expr_ty_adjusted(lhs).sty {
-            ty::TyAdt(def, _) => {
-                self.insert_def_id(def.non_enum_variant().fields[idx].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, "numeric field access on non-ADT"),
+            _ => span_bug!(lhs.span, "named field access on non-ADT"),
         }
     }
 
@@ -128,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);
         }
     }
 
@@ -191,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);
             }
         }
     }
@@ -242,17 +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::ExprTupField(ref lhs, idx) => {
-                self.handle_tup_field_access(&lhs, idx.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);
                 }
             }
             _ => ()

src/librustc/middle/expr_use_visitor.rs

@@ -404,10 +404,6 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
                 self.select_from_expr(&base);
             }
 
-            hir::ExprTupField(ref base, _) => {         // base.<n>
-                self.select_from_expr(&base);
-            }
-
             hir::ExprIndex(ref lhs, ref rhs) => {       // lhs[rhs]
                 self.select_from_expr(&lhs);
                 self.consume_expr(&rhs);
@@ -663,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)
                         );
@@ -691,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

src/librustc/middle/liveness.rs

@@ -476,7 +476,7 @@ fn visit_expr<'a, 'tcx>(ir: &mut IrMaps<'a, 'tcx>, expr: &'tcx Expr) {
       }
 
       // otherwise, live nodes are not required:
-      hir::ExprIndex(..) | hir::ExprField(..) | hir::ExprTupField(..) |
+      hir::ExprIndex(..) | hir::ExprField(..) |
       hir::ExprArray(..) | hir::ExprCall(..) | hir::ExprMethodCall(..) |
       hir::ExprTup(..) | hir::ExprBinary(..) | hir::ExprAddrOf(..) |
       hir::ExprCast(..) | hir::ExprUnary(..) | hir::ExprBreak(..) |
@@ -912,10 +912,6 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
               self.propagate_through_expr(&e, succ)
           }
 
-          hir::ExprTupField(ref e, _) => {
-              self.propagate_through_expr(&e, succ)
-          }
-
           hir::ExprClosure(.., blk_id, _, _) => {
               debug!("{} is an ExprClosure", self.ir.tcx.hir.node_to_pretty_string(expr.id));
 
@@ -1226,7 +1222,6 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
         match expr.node {
             hir::ExprPath(_) => succ,
             hir::ExprField(ref e, _) => self.propagate_through_expr(&e, succ),
-            hir::ExprTupField(ref e, _) => self.propagate_through_expr(&e, succ),
             _ => self.propagate_through_expr(expr, succ)
         }
     }
@@ -1419,7 +1414,7 @@ fn check_expr<'a, 'tcx>(this: &mut Liveness<'a, 'tcx>, expr: &'tcx Expr) {
       // no correctness conditions related to liveness
       hir::ExprCall(..) | hir::ExprMethodCall(..) | hir::ExprIf(..) |
       hir::ExprMatch(..) | hir::ExprWhile(..) | hir::ExprLoop(..) |
-      hir::ExprIndex(..) | hir::ExprField(..) | hir::ExprTupField(..) |
+      hir::ExprIndex(..) | hir::ExprField(..) |
       hir::ExprArray(..) | hir::ExprTup(..) | hir::ExprBinary(..) |
       hir::ExprCast(..) | hir::ExprUnary(..) | hir::ExprRet(..) |
       hir::ExprBreak(..) | hir::ExprAgain(..) | hir::ExprLit(_) |