Auto merge of #64469 - matthewjasper:increase-hygiene-use, r=petrochenkov

Cleanup handling of hygiene for built-in macros

This makes most identifiers generated by built-in macros use def-site hygiene, not only the ones that previously used gensyms.

* `ExtCtxt::ident_of` now takes a `Span` and is preferred to `Ident::{from_str, from_str_and_span}`
* Remove `Span::with_legacy_ctxt`
    * `assert` now uses call-site hygiene because it needs to resolve `panic` unhygienically.
    * `concat_idents` now uses call-site hygiene because it wouldn't be very useful with def-site hygiene.
    * everything else is moved to def-site hygiene

r? @petrochenkov

Author: bors

src/librustc/hir/lowering.rs

@@ -1316,7 +1316,7 @@ impl<'a> LoweringContext<'a> {
                             ImplTraitContext::Universal(in_band_ty_params),
                         );
                         // Set the name to `impl Bound1 + Bound2`.
-                        let ident = Ident::from_str(&pprust::ty_to_string(t)).with_span_pos(span);
+                        let ident = Ident::from_str_and_span(&pprust::ty_to_string(t), span);
                         in_band_ty_params.push(hir::GenericParam {
                             hir_id: self.lower_node_id(def_node_id),
                             name: ParamName::Plain(ident),

src/librustc_lint/unused.rs

@@ -618,24 +618,19 @@ impl UnusedImportBraces {
             }
 
             // Trigger the lint if the nested item is a non-self single item
-            let node_ident;
-            match items[0].0.kind {
+            let node_name = match items[0].0.kind {
                 ast::UseTreeKind::Simple(rename, ..) => {
                     let orig_ident = items[0].0.prefix.segments.last().unwrap().ident;
                     if orig_ident.name == kw::SelfLower {
                         return;
                     }
-                    node_ident = rename.unwrap_or(orig_ident);
+                    rename.unwrap_or(orig_ident).name
                 }
-                ast::UseTreeKind::Glob => {
-                    node_ident = ast::Ident::from_str("*");
-                }
-                ast::UseTreeKind::Nested(_) => {
-                    return;
-                }
-            }
+                ast::UseTreeKind::Glob => Symbol::intern("*"),
+                ast::UseTreeKind::Nested(_) => return,
+            };
 
-            let msg = format!("braces around {} is unnecessary", node_ident.name);
+            let msg = format!("braces around {} is unnecessary", node_name);
             cx.span_lint(UNUSED_IMPORT_BRACES, item.span, &msg);
         }
     }

src/librustc_metadata/cstore_impl.rs

@@ -444,7 +444,8 @@ impl cstore::CStore {
             .insert(local_span, (name.to_string(), data.get_span(id.index, sess)));
 
         LoadedMacro::MacroDef(ast::Item {
-            ident: ast::Ident::from_str(&name.as_str()),
+            // FIXME: cross-crate hygiene
+            ident: ast::Ident::with_dummy_span(name.as_symbol()),
             id: ast::DUMMY_NODE_ID,
             span: local_span,
             attrs: attrs.iter().cloned().collect(),

src/librustc_resolve/lib.rs

@@ -40,7 +40,7 @@ use rustc_metadata::cstore::CStore;
 use syntax::ext::hygiene::{ExpnId, Transparency, SyntaxContext};
 use syntax::ast::{self, Name, NodeId, Ident, FloatTy, IntTy, UintTy};
 use syntax::ext::base::{SyntaxExtension, MacroKind, SpecialDerives};
-use syntax::symbol::{Symbol, kw, sym};
+use syntax::symbol::{kw, sym};
 
 use syntax::visit::{self, Visitor};
 use syntax::attr;
@@ -241,7 +241,7 @@ impl Segment {
 
     fn names_to_string(segments: &[Segment]) -> String {
         names_to_string(&segments.iter()
-                            .map(|seg| seg.ident)
+                            .map(|seg| seg.ident.name)
                             .collect::<Vec<_>>())
     }
 }
@@ -951,7 +951,7 @@ pub struct Resolver<'a> {
     struct_constructors: DefIdMap<(Res, ty::Visibility)>,
 
     /// Features enabled for this crate.
-    active_features: FxHashSet<Symbol>,
+    active_features: FxHashSet<Name>,
 
     /// Stores enum visibilities to properly build a reduced graph
     /// when visiting the correspondent variants.
@@ -1018,8 +1018,8 @@ impl<'a> hir::lowering::Resolver for Resolver<'a> {
     fn resolve_str_path(
         &mut self,
         span: Span,
-        crate_root: Option<Symbol>,
-        components: &[Symbol],
+        crate_root: Option<Name>,
+        components: &[Name],
         ns: Namespace,
     ) -> (ast::Path, Res) {
         let root = if crate_root.is_some() {
@@ -2555,7 +2555,7 @@ impl<'a> Resolver<'a> {
     fn add_suggestion_for_rename_of_use(
         &self,
         err: &mut DiagnosticBuilder<'_>,
-        name: Symbol,
+        name: Name,
         directive: &ImportDirective<'_>,
         binding_span: Span,
     ) {
@@ -2770,38 +2770,37 @@ impl<'a> Resolver<'a> {
     }
 }
 
-fn names_to_string(idents: &[Ident]) -> String {
+fn names_to_string(names: &[Name]) -> String {
     let mut result = String::new();
-    for (i, ident) in idents.iter()
-                            .filter(|ident| ident.name != kw::PathRoot)
+    for (i, name) in names.iter()
+                            .filter(|name| **name != kw::PathRoot)
                             .enumerate() {
         if i > 0 {
             result.push_str("::");
         }
-        result.push_str(&ident.as_str());
+        result.push_str(&name.as_str());
     }
     result
 }
 
 fn path_names_to_string(path: &Path) -> String {
     names_to_string(&path.segments.iter()
-                        .map(|seg| seg.ident)
+                        .map(|seg| seg.ident.name)
                         .collect::<Vec<_>>())
 }
 
 /// A somewhat inefficient routine to obtain the name of a module.
 fn module_to_string(module: Module<'_>) -> Option<String> {
     let mut names = Vec::new();
 
-    fn collect_mod(names: &mut Vec<Ident>, module: Module<'_>) {
+    fn collect_mod(names: &mut Vec<Name>, module: Module<'_>) {
         if let ModuleKind::Def(.., name) = module.kind {
             if let Some(parent) = module.parent {
-                names.push(Ident::with_dummy_span(name));
+                names.push(name);
                 collect_mod(names, parent);
             }
         } else {
-            // danger, shouldn't be ident?
-            names.push(Ident::from_str("<opaque>"));
+            names.push(Name::intern("<opaque>"));
             collect_mod(names, module.parent.unwrap());
         }
     }
@@ -2810,9 +2809,8 @@ fn module_to_string(module: Module<'_>) -> Option<String> {
     if names.is_empty() {
         return None;
     }
-    Some(names_to_string(&names.into_iter()
-                        .rev()
-                        .collect::<Vec<_>>()))
+    names.reverse();
+    Some(names_to_string(&names))
 }
 
 #[derive(Copy, Clone, Debug)]

src/librustc_resolve/resolve_imports.rs

@@ -1433,15 +1433,17 @@ fn import_path_to_string(names: &[Ident],
     let global = !names.is_empty() && names[0].name == kw::PathRoot;
     if let Some(pos) = pos {
         let names = if global { &names[1..pos + 1] } else { &names[..pos + 1] };
-        names_to_string(names)
+        names_to_string(&names.iter().map(|ident| ident.name).collect::<Vec<_>>())
     } else {
         let names = if global { &names[1..] } else { names };
         if names.is_empty() {
             import_directive_subclass_to_string(subclass)
         } else {
-            format!("{}::{}",
-                    names_to_string(names),
-                    import_directive_subclass_to_string(subclass))
+            format!(
+                "{}::{}",
+                names_to_string(&names.iter().map(|ident| ident.name).collect::<Vec<_>>()),
+                import_directive_subclass_to_string(subclass),
+            )
         }
     }
 }

src/libsyntax/ext/base.rs

@@ -977,13 +977,6 @@ impl<'a> ExtCtxt<'a> {
         span.with_call_site_ctxt(self.current_expansion.id)
     }
 
-    /// Span with a context reproducing `macro_rules` hygiene (hygienic locals, unhygienic items).
-    /// FIXME: This should be eventually replaced either with `with_def_site_ctxt` (preferably),
-    /// or with `with_call_site_ctxt` (where necessary).
-    pub fn with_legacy_ctxt(&self, span: Span) -> Span {
-        span.with_legacy_ctxt(self.current_expansion.id)
-    }
-
     /// Returns span for the macro which originally caused the current expansion to happen.
     ///
     /// Stops backtracing at include! boundary.
@@ -1081,8 +1074,8 @@ impl<'a> ExtCtxt<'a> {
     pub fn set_trace_macros(&mut self, x: bool) {
         self.ecfg.trace_mac = x
     }
-    pub fn ident_of(&self, st: &str) -> ast::Ident {
-        ast::Ident::from_str(st)
+    pub fn ident_of(&self, st: &str, sp: Span) -> ast::Ident {
+        ast::Ident::from_str_and_span(st, sp)
     }
     pub fn std_path(&self, components: &[Symbol]) -> Vec<ast::Ident> {
         let def_site = self.with_def_site_ctxt(DUMMY_SP);

src/libsyntax/ext/build.rs

@@ -363,7 +363,7 @@ impl<'a> ExtCtxt<'a> {
         self.expr(sp, ast::ExprKind::Field(expr, ident.with_span_pos(sp)))
     }
     pub fn expr_tup_field_access(&self, sp: Span, expr: P<ast::Expr>, idx: usize) -> P<ast::Expr> {
-        let ident = Ident::from_str(&idx.to_string()).with_span_pos(sp);
+        let ident = Ident::new(sym::integer(idx), sp);
         self.expr(sp, ast::ExprKind::Field(expr, ident))
     }
     pub fn expr_addr_of(&self, sp: Span, e: P<ast::Expr>) -> P<ast::Expr> {
@@ -525,7 +525,7 @@ impl<'a> ExtCtxt<'a> {
         let err = self.std_path(&[sym::result, sym::Result, sym::Err]);
         let err_path = self.path_global(sp, err);
 
-        let binding_variable = self.ident_of("__try_var");
+        let binding_variable = self.ident_of("__try_var", sp);
         let binding_pat = self.pat_ident(sp, binding_variable);
         let binding_expr = self.expr_ident(sp, binding_variable);
 

src/libsyntax/parse/parser/item.rs

@@ -1256,7 +1256,7 @@ impl<'a> Parser<'a> {
             for part in idents {
                 fixed_name.push_str(&format!("_{}", part.name));
             }
-            ident = Ident::from_str(&fixed_name).with_span_pos(fixed_name_sp);
+            ident = Ident::from_str_and_span(&fixed_name, fixed_name_sp);
 
             self.struct_span_err(fixed_name_sp, error_msg)
                 .span_label(fixed_name_sp, "dash-separated idents are not valid")

src/libsyntax_ext/asm.rs

@@ -62,7 +62,7 @@ pub fn expand_asm<'cx>(cx: &'cx mut ExtCtxt<'_>,
     MacEager::expr(P(ast::Expr {
         id: ast::DUMMY_NODE_ID,
         node: ast::ExprKind::InlineAsm(P(inline_asm)),
-        span: cx.with_legacy_ctxt(sp),
+        span: cx.with_def_site_ctxt(sp),
         attrs: ThinVec::new(),
     }))
 }

src/libsyntax_ext/assert.rs

@@ -23,7 +23,9 @@ pub fn expand_assert<'cx>(
         }
     };
 
-    let sp = cx.with_legacy_ctxt(sp);
+    // `core::panic` and `std::panic` are different macros, so we use call-site
+    // context to pick up whichever is currently in scope.
+    let sp = cx.with_call_site_ctxt(sp);
     let panic_call = Mac {
         path: Path::from_ident(Ident::new(sym::panic, sp)),
         tts: custom_message.unwrap_or_else(|| {