lowering: Rename identifiers only when necessary

Do not rename invalid identifiers, they stop being invalid after renaming

Author: petrochenkov

src/librustc/hir/lowering.rs

@@ -44,7 +44,7 @@ use hir;
 use hir::map::Definitions;
 use hir::map::definitions::DefPathData;
 use hir::def_id::{DefIndex, DefId};
-use hir::def::Def;
+use hir::def::{Def, PathResolution};
 
 use std::collections::BTreeMap;
 use std::iter;
@@ -53,7 +53,7 @@ use syntax::attr::{ThinAttributes, ThinAttributesExt};
 use syntax::ext::mtwt;
 use syntax::ptr::P;
 use syntax::codemap::{respan, Spanned, Span};
-use syntax::parse::token;
+use syntax::parse::token::{self, keywords};
 use syntax::std_inject;
 use syntax::visit::{self, Visitor};
 
@@ -72,6 +72,9 @@ pub trait Resolver {
     // Resolve a global hir path generated by the lowerer when expanding `for`, `if let`, etc.
     fn resolve_generated_global_path(&mut self, path: &hir::Path, is_value: bool) -> Def;
 
+    // Obtain the resolution for a node id
+    fn get_resolution(&mut self, id: NodeId) -> Option<PathResolution>;
+
     // Record the resolution of a path or binding generated by the lowerer when expanding.
     fn record_resolution(&mut self, id: NodeId, def: Def);
 
@@ -85,6 +88,9 @@ impl Resolver for DummyResolver {
     fn resolve_generated_global_path(&mut self, _path: &hir::Path, _is_value: bool) -> Def {
         Def::Err
     }
+    fn get_resolution(&mut self, _id: NodeId) -> Option<PathResolution> {
+        None
+    }
     fn record_resolution(&mut self, _id: NodeId, _def: Def) {}
     fn definitions(&mut self) -> Option<&mut Definitions> {
         None
@@ -170,7 +176,11 @@ impl<'a> LoweringContext<'a> {
     }
 
     fn lower_ident(&mut self, ident: Ident) -> Name {
-        mtwt::resolve(ident)
+        if ident.name != keywords::Invalid.name() {
+            mtwt::resolve(ident)
+        } else {
+            ident.name
+        }
     }
 
     fn lower_attrs(&mut self, attrs: &Vec<Attribute>) -> hir::HirVec<Attribute> {
@@ -315,18 +325,14 @@ impl<'a> LoweringContext<'a> {
         }
     }
 
-    // Path segments are usually unhygienic, hygienic path segments can occur only in
-    // identifier-like paths originating from `ExprPath`.
-    // Make life simpler for rustc_resolve by renaming only such segments.
-    fn lower_path_full(&mut self, p: &Path, maybe_hygienic: bool) -> hir::Path {
-        let maybe_hygienic = maybe_hygienic && !p.global && p.segments.len() == 1;
+    fn lower_path_full(&mut self, p: &Path, rename: bool) -> hir::Path {
         hir::Path {
             global: p.global,
             segments: p.segments
                        .iter()
                        .map(|&PathSegment { identifier, ref parameters }| {
                            hir::PathSegment {
-                               name: if maybe_hygienic {
+                               name: if rename {
                                    self.lower_ident(identifier)
                                } else {
                                    identifier.name
@@ -846,9 +852,14 @@ impl<'a> LoweringContext<'a> {
                 PatKind::Wild => hir::PatKind::Wild,
                 PatKind::Ident(ref binding_mode, pth1, ref sub) => {
                     self.with_parent_def(p.id, |this| {
+                        let name = match this.resolver.get_resolution(p.id).map(|d| d.full_def()) {
+                            // Only pattern bindings are renamed
+                            None | Some(Def::Local(..)) => this.lower_ident(pth1.node),
+                            _ => pth1.node.name,
+                        };
                         hir::PatKind::Ident(this.lower_binding_mode(binding_mode),
-                                      respan(pth1.span, this.lower_ident(pth1.node)),
-                                      sub.as_ref().map(|x| this.lower_pat(x)))
+                                            respan(pth1.span, name),
+                                            sub.as_ref().map(|x| this.lower_pat(x)))
                     })
                 }
                 PatKind::Lit(ref e) => hir::PatKind::Lit(self.lower_expr(e)),
@@ -1212,7 +1223,16 @@ impl<'a> LoweringContext<'a> {
                             position: position,
                         }
                     });
-                    hir::ExprPath(hir_qself, self.lower_path_full(path, qself.is_none()))
+                    let rename = if path.segments.len() == 1 {
+                        // Only local variables are renamed
+                        match self.resolver.get_resolution(e.id).map(|d| d.full_def()) {
+                            Some(Def::Local(..)) | Some(Def::Upvar(..)) => true,
+                            _ => false,
+                        }
+                    } else {
+                        false
+                    };
+                    hir::ExprPath(hir_qself, self.lower_path_full(path, rename))
                 }
                 ExprKind::Break(opt_ident) => hir::ExprBreak(opt_ident.map(|sp_ident| {
                     respan(sp_ident.span, self.lower_ident(sp_ident.node))

src/librustc_resolve/lib.rs

@@ -1102,6 +1102,10 @@ impl<'a> hir::lowering::Resolver for Resolver<'a> {
         }
     }
 
+    fn get_resolution(&mut self, id: NodeId) -> Option<PathResolution> {
+        self.def_map.get(&id).cloned()
+    }
+
     fn record_resolution(&mut self, id: NodeId, def: Def) {
         self.def_map.insert(id, PathResolution { base_def: def, depth: 0 });
     }