Auto merge of #30145 - petrochenkov:hyg, r=nrc

Instead of `ast::Ident`, bindings, paths and labels in HIR now keep a new structure called `hir::Ident` containing mtwt-renamed `name` and the original not-renamed `unhygienic_name`. `name` is supposed to be used by default, `unhygienic_name` is rarely used.

This is not ideal, but better than the status quo for two reasons:
- MTWT tables can be cleared immediately after lowering to HIR
- This is less bug-prone, because it is impossible now to forget applying `mtwt::resolve` to a name. It is still possible to use `name` instead of `unhygienic_name` by mistake, but `unhygienic_name`s are used only in few very special circumstances, so it shouldn't be a problem.

Besides name resolution `unhygienic_name` is used in some lints and debuginfo. `unhygienic_name` can be very well approximated by "reverse renaming" `token::intern(name.as_str())` or even plain string `name.as_str()`, except that it would break gensyms like `iter` in desugared `for` loops. This approximation is likely good enough for lints and debuginfo, but not for name resolution, unfortunately (see #27639), so `unhygienic_name` has to be kept.

cc #29782

r? @nrc

src/librustc/middle/check_match.rs

@@ -247,7 +247,7 @@ fn check_for_bindings_named_the_same_as_variants(cx: &MatchCheckCtxt, pat: &Pat)
                     let def = cx.tcx.def_map.borrow().get(&p.id).map(|d| d.full_def());
                     if let Some(DefLocal(..)) = def {
                         if edef.variants.iter().any(|variant|
-                            variant.name == ident.node.name
+                            variant.name == ident.node.unhygienic_name
                                 && variant.kind() == VariantKind::Unit
                         ) {
                             span_warn!(cx.tcx.sess, p.span, E0170,

src/librustc/middle/liveness.rs

@@ -1596,7 +1596,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
                     self.ir.tcx.sess.add_lint(lint::builtin::UNUSED_VARIABLES, id, sp,
                         format!("variable `{}` is assigned to, but never used",
                                 name));
-                } else {
+                } else if name != "self" {
                     self.ir.tcx.sess.add_lint(lint::builtin::UNUSED_VARIABLES, id, sp,
                         format!("unused variable: `{}`", name));
                 }

src/librustc/middle/pat_util.rs

@@ -14,7 +14,6 @@ use middle::ty;
 use util::nodemap::FnvHashMap;
 
 use syntax::ast;
-use syntax::ext::mtwt;
 use rustc_front::hir;
 use rustc_front::util::walk_pat;
 use syntax::codemap::{respan, Span, Spanned, DUMMY_SP};
@@ -27,8 +26,8 @@ pub type PatIdMap = FnvHashMap<ast::Name, ast::NodeId>;
 // use the NodeId of their namesake in the first pattern.
 pub fn pat_id_map(dm: &RefCell<DefMap>, pat: &hir::Pat) -> PatIdMap {
     let mut map = FnvHashMap();
-    pat_bindings_hygienic(dm, pat, |_bm, p_id, _s, path1| {
-        map.insert(mtwt::resolve(path1.node), p_id);
+    pat_bindings(dm, pat, |_bm, p_id, _s, path1| {
+        map.insert(path1.node, p_id);
     });
     map
 }
@@ -124,9 +123,8 @@ pub fn pat_bindings<I>(dm: &RefCell<DefMap>, pat: &hir::Pat, mut it: I) where
         true
     });
 }
-
-pub fn pat_bindings_hygienic<I>(dm: &RefCell<DefMap>, pat: &hir::Pat, mut it: I) where
-    I: FnMut(hir::BindingMode, ast::NodeId, Span, &Spanned<ast::Ident>),
+pub fn pat_bindings_ident<I>(dm: &RefCell<DefMap>, pat: &hir::Pat, mut it: I) where
+    I: FnMut(hir::BindingMode, ast::NodeId, Span, &Spanned<hir::Ident>),
 {
     walk_pat(pat, |p| {
         match p.node {
@@ -214,7 +212,7 @@ pub fn def_to_path(tcx: &ty::ctxt, id: DefId) -> hir::Path {
     tcx.with_path(id, |path| hir::Path {
         global: false,
         segments: path.last().map(|elem| hir::PathSegment {
-            identifier: ast::Ident::with_empty_ctxt(elem.name()),
+            identifier: hir::Ident::from_name(elem.name()),
             parameters: hir::PathParameters::none(),
         }).into_iter().collect(),
         span: DUMMY_SP,

src/librustc/middle/resolve_lifetime.rs

@@ -426,7 +426,7 @@ fn extract_labels<'v, 'a>(ctxt: &mut LifetimeContext<'a>, b: &'v hir::Block) {
     fn expression_label(ex: &hir::Expr) -> Option<ast::Name> {
         match ex.node {
             hir::ExprWhile(_, _, Some(label)) |
-            hir::ExprLoop(_, Some(label)) => Some(label.name),
+            hir::ExprLoop(_, Some(label)) => Some(label.unhygienic_name),
             _ => None,
         }
     }

src/librustc_driver/driver.rs

@@ -112,6 +112,13 @@ pub fn compile_input(sess: Session,
         let mut hir_forest = time(sess.time_passes(),
                                   "lowering ast -> hir",
                                   || hir_map::Forest::new(lower_crate(&lcx, &expanded_crate)));
+
+        // Discard MTWT tables that aren't required past lowering to HIR.
+        if !sess.opts.debugging_opts.keep_mtwt_tables &&
+           !sess.opts.debugging_opts.save_analysis {
+            syntax::ext::mtwt::clear_tables();
+        }
+
         let arenas = ty::CtxtArenas::new();
         let ast_map = make_map(&sess, &mut hir_forest);
 
@@ -704,12 +711,6 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session,
              "resolution",
              || resolve::resolve_crate(sess, &ast_map, make_glob_map));
 
-    // Discard MTWT tables that aren't required past resolution.
-    // FIXME: get rid of uses of MTWT tables in typeck, mir and trans and clear them
-    if !sess.opts.debugging_opts.keep_mtwt_tables {
-        // syntax::ext::mtwt::clear_tables();
-    }
-
     let named_region_map = time(time_passes,
                                 "lifetime resolution",
                                 || middle::resolve_lifetime::krate(sess, krate, &def_map.borrow()));

src/librustc_front/fold.rs

@@ -12,7 +12,7 @@
 //! and returns a piece of the same type.
 
 use hir::*;
-use syntax::ast::{Ident, Name, NodeId, DUMMY_NODE_ID, Attribute, Attribute_, MetaItem};
+use syntax::ast::{Name, NodeId, DUMMY_NODE_ID, Attribute, Attribute_, MetaItem};
 use syntax::ast::{MetaWord, MetaList, MetaNameValue};
 use syntax::attr::ThinAttributesExt;
 use hir;

src/librustc_front/hir.rs

@@ -39,7 +39,7 @@ use intravisit::Visitor;
 use std::collections::BTreeMap;
 use syntax::codemap::{self, Span, Spanned, DUMMY_SP, ExpnId};
 use syntax::abi::Abi;
-use syntax::ast::{Name, Ident, NodeId, DUMMY_NODE_ID, TokenTree, AsmDialect};
+use syntax::ast::{Name, NodeId, DUMMY_NODE_ID, TokenTree, AsmDialect};
 use syntax::ast::{Attribute, Lit, StrStyle, FloatTy, IntTy, UintTy, CrateConfig};
 use syntax::attr::ThinAttributes;
 use syntax::owned_slice::OwnedSlice;
@@ -50,7 +50,65 @@ use print::pprust;
 use util;
 
 use std::fmt;
-use serialize::{Encodable, Encoder, Decoder};
+use std::hash::{Hash, Hasher};
+use serialize::{Encodable, Decodable, Encoder, Decoder};
+
+/// Identifier in HIR
+#[derive(Clone, Copy, Eq)]
+pub struct Ident {
+    /// Hygienic name (renamed), should be used by default
+    pub name: Name,
+    /// Unhygienic name (original, not renamed), needed in few places in name resolution
+    pub unhygienic_name: Name,
+}
+
+impl Ident {
+    /// Creates a HIR identifier with both `name` and `unhygienic_name` initialized with
+    /// the argument. Hygiene properties of the created identifier depend entirely on this
+    /// argument. If the argument is a plain interned string `intern("iter")`, then the result
+    /// is unhygienic and can interfere with other entities named "iter". If the argument is
+    /// a "fresh" name created with `gensym("iter")`, then the result is hygienic and can't
+    /// interfere with other entities having the same string as a name.
+    pub fn from_name(name: Name) -> Ident {
+        Ident { name: name, unhygienic_name: name }
+    }
+}
+
+impl PartialEq for Ident {
+    fn eq(&self, other: &Ident) -> bool {
+        self.name == other.name
+    }
+}
+
+impl Hash for Ident {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        self.name.hash(state)
+    }
+}
+
+impl fmt::Debug for Ident {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        fmt::Debug::fmt(&self.name, f)
+    }
+}
+
+impl fmt::Display for Ident {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        fmt::Display::fmt(&self.name, f)
+    }
+}
+
+impl Encodable for Ident {
+    fn encode<S: Encoder>(&self, s: &mut S) -> Result<(), S::Error> {
+        self.name.encode(s)
+    }
+}
+
+impl Decodable for Ident {
+    fn decode<D: Decoder>(d: &mut D) -> Result<Ident, D::Error> {
+        Ok(Ident::from_name(try!(Name::decode(d))))
+    }
+}
 
 #[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Copy)]
 pub struct Lifetime {
@@ -105,6 +163,14 @@ impl fmt::Display for Path {
 #[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
 pub struct PathSegment {
     /// The identifier portion of this path segment.
+    ///
+    /// Hygiene properties of this identifier are worth noting.
+    /// Most path segments are not hygienic and they are not renamed during
+    /// lowering from AST to HIR (see comments to `fn lower_path`). However segments from
+    /// unqualified paths with one segment originating from `ExprPath` (local-variable-like paths)
+    /// can be hygienic, so they are renamed. You should not normally care about this peculiarity
+    /// and just use `identifier.name` unless you modify identifier resolution code
+    /// (`fn resolve_identifier` and other functions called by it in `rustc_resolve`).
     pub identifier: Ident,
 
     /// Type/lifetime parameters attached to this path. They come in

src/librustc_front/intravisit.rs

@@ -26,7 +26,7 @@
 //! property.
 
 use syntax::abi::Abi;
-use syntax::ast::{Ident, NodeId, CRATE_NODE_ID, Name, Attribute};
+use syntax::ast::{NodeId, CRATE_NODE_ID, Name, Attribute};
 use syntax::codemap::Span;
 use hir::*;