Auto merge of rust-lang#95880 - cjgillot:def-ident-span, r=petrochenkov

Handle `def_ident_span` like `def_span`.

`def_ident_span` had an ad-hoc status in the compiler.

This PR refactors it to be a first-class citizen like `def_span`:
- it gets encoded in the main metadata loop, instead of the visitor;
- its implementation is updated to mirror the one of `def_span`.

We do not remove the `Option` in the return type, since some items do not have an ident, AnonConsts for instance.

Author: bors

compiler/rustc_metadata/src/rmeta/decoder.rs

@@ -774,17 +774,8 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
 
     fn opt_item_ident(self, item_index: DefIndex, sess: &Session) -> Option<Ident> {
         let name = self.opt_item_name(item_index)?;
-        let span = match self.root.tables.def_ident_span.get(self, item_index) {
-            Some(lazy_span) => lazy_span.decode((self, sess)),
-            None => {
-                // FIXME: this weird case of a name with no span is specific to `extern crate`
-                // items, which are supposed to be treated like `use` items and only be encoded
-                // to metadata as `Export`s, return `None` because that's what all the callers
-                // expect in this case.
-                assert_eq!(self.def_kind(item_index), DefKind::ExternCrate);
-                return None;
-            }
-        };
+        let span =
+            self.root.tables.def_ident_span.get(self, item_index).unwrap().decode((self, sess));
         Some(Ident::new(name, span))
     }
 

compiler/rustc_metadata/src/rmeta/encoder.rs

@@ -31,7 +31,7 @@ use rustc_serialize::{opaque, Encodable, Encoder};
 use rustc_session::config::CrateType;
 use rustc_session::cstore::{ForeignModule, LinkagePreference, NativeLib};
 use rustc_span::hygiene::{ExpnIndex, HygieneEncodeContext, MacroKind};
-use rustc_span::symbol::{sym, Ident, Symbol};
+use rustc_span::symbol::{sym, Symbol};
 use rustc_span::{
     self, DebuggerVisualizerFile, ExternalSource, FileName, SourceFile, Span, SyntaxContext,
 };
@@ -1007,6 +1007,9 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
             record!(self.tables.def_span[def_id] <- tcx.def_span(def_id));
             self.encode_attrs(local_id);
             record!(self.tables.expn_that_defined[def_id] <- self.tcx.expn_that_defined(def_id));
+            if let Some(ident_span) = tcx.def_ident_span(def_id) {
+                record!(self.tables.def_ident_span[def_id] <- ident_span);
+            }
             if def_kind.has_codegen_attrs() {
                 record!(self.tables.codegen_fn_attrs[def_id] <- self.tcx.codegen_fn_attrs(def_id));
             }
@@ -1071,7 +1074,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
             assert!(f.did.is_local());
             f.did.index
         }));
-        self.encode_ident_span(def_id, variant.ident(tcx));
         self.encode_item_type(def_id);
         if variant.ctor_kind == CtorKind::Fn {
             // FIXME(eddyb) encode signature only in `encode_enum_variant_ctor`.
@@ -1163,7 +1165,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
         debug!("EncodeContext::encode_field({:?})", def_id);
 
         record!(self.tables.kind[def_id] <- EntryKind::Field);
-        self.encode_ident_span(def_id, field.ident(self.tcx));
         self.encode_item_type(def_id);
     }
 
@@ -1242,7 +1243,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
                 record!(self.tables.kind[def_id] <- EntryKind::AssocType(container));
             }
         }
-        self.encode_ident_span(def_id, ast_item.ident);
         match trait_item.kind {
             ty::AssocKind::Const | ty::AssocKind::Fn => {
                 self.encode_item_type(def_id);
@@ -1306,7 +1306,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
                 record!(self.tables.kind[def_id] <- EntryKind::AssocType(container));
             }
         }
-        self.encode_ident_span(def_id, impl_item.ident(self.tcx));
         self.encode_item_type(def_id);
         if let Some(trait_item_def_id) = impl_item.trait_item_def_id {
             self.tables.trait_item_def_id.set(def_id.index, trait_item_def_id.into());
@@ -1408,8 +1407,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
 
         debug!("EncodeContext::encode_info_for_item({:?})", def_id);
 
-        self.encode_ident_span(def_id, item.ident);
-
         let entry_kind = match item.kind {
             hir::ItemKind::Static(..) => EntryKind::Static,
             hir::ItemKind::Const(_, body_id) => {
@@ -1953,7 +1950,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
                 record!(self.tables.kind[def_id] <- EntryKind::ForeignType);
             }
         }
-        self.encode_ident_span(def_id, nitem.ident);
         self.encode_item_type(def_id);
         if let hir::ForeignItemKind::Fn(..) = nitem.kind {
             record!(self.tables.fn_sig[def_id] <- tcx.fn_sig(def_id));
@@ -2035,10 +2031,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
         }
     }
 
-    fn encode_ident_span(&mut self, def_id: DefId, ident: Ident) {
-        record!(self.tables.def_ident_span[def_id] <- ident.span);
-    }
-
     /// In some cases, along with the item itself, we also
     /// encode some sub-items. Usually we want some info from the item
     /// so it's easier to do that here then to wait until we would encounter

compiler/rustc_middle/src/hir/map/mod.rs

@@ -912,27 +912,33 @@ impl<'hir> Map<'hir> {
         }
     }
 
+    #[inline]
+    fn opt_ident(self, id: HirId) -> Option<Ident> {
+        match self.get(id) {
+            Node::Binding(&Pat { kind: PatKind::Binding(_, _, ident, _), .. }) => Some(ident),
+            // A `Ctor` doesn't have an identifier itself, but its parent
+            // struct/variant does. Compare with `hir::Map::opt_span`.
+            Node::Ctor(..) => match self.find(self.get_parent_node(id))? {
+                Node::Item(item) => Some(item.ident),
+                Node::Variant(variant) => Some(variant.ident),
+                _ => unreachable!(),
+            },
+            node => node.ident(),
+        }
+    }
+
+    #[inline]
+    pub(super) fn opt_ident_span(self, id: HirId) -> Option<Span> {
+        self.opt_ident(id).map(|ident| ident.span)
+    }
+
+    #[inline]
     pub fn opt_name(self, id: HirId) -> Option<Symbol> {
-        Some(match self.get(id) {
-            Node::Item(i) => i.ident.name,
-            Node::ForeignItem(fi) => fi.ident.name,
-            Node::ImplItem(ii) => ii.ident.name,
-            Node::TraitItem(ti) => ti.ident.name,
-            Node::Variant(v) => v.ident.name,
-            Node::Field(f) => f.ident.name,
-            Node::Lifetime(lt) => lt.name.ident().name,
-            Node::GenericParam(param) => param.name.ident().name,
-            Node::Binding(&Pat { kind: PatKind::Binding(_, _, l, _), .. }) => l.name,
-            Node::Ctor(..) => self.name(HirId::make_owner(self.get_parent_item(id))),
-            _ => return None,
-        })
+        self.opt_ident(id).map(|ident| ident.name)
     }
 
     pub fn name(self, id: HirId) -> Symbol {
-        match self.opt_name(id) {
-            Some(name) => name,
-            None => bug!("no name for {}", self.node_to_string(id)),
-        }
+        self.opt_name(id).unwrap_or_else(|| bug!("no name for {}", self.node_to_string(id)))
     }
 
     /// Given a node ID, gets a list of attributes associated with the AST
@@ -1008,7 +1014,7 @@ impl<'hir> Map<'hir> {
     }
 
     pub fn span_if_local(self, id: DefId) -> Option<Span> {
-        id.as_local().and_then(|id| self.opt_span(self.local_def_id_to_hir_id(id)))
+        if id.is_local() { Some(self.tcx.def_span(id)) } else { None }
     }
 
     pub fn res_span(self, res: Res) -> Option<Span> {

compiler/rustc_middle/src/hir/mod.rs

@@ -121,7 +121,16 @@ pub fn provide(providers: &mut Providers) {
     providers.hir_attrs =
         |tcx, id| tcx.hir_crate(()).owners[id].as_owner().map_or(AttributeMap::EMPTY, |o| &o.attrs);
     providers.source_span = |tcx, def_id| tcx.resolutions(()).definitions.def_span(def_id);
-    providers.def_span = |tcx, def_id| tcx.hir().span_if_local(def_id).unwrap_or(DUMMY_SP);
+    providers.def_span = |tcx, def_id| {
+        let def_id = def_id.expect_local();
+        let hir_id = tcx.hir().local_def_id_to_hir_id(def_id);
+        tcx.hir().opt_span(hir_id).unwrap_or(DUMMY_SP)
+    };
+    providers.def_ident_span = |tcx, def_id| {
+        let def_id = def_id.expect_local();
+        let hir_id = tcx.hir().local_def_id_to_hir_id(def_id);
+        tcx.hir().opt_ident_span(hir_id)
+    };
     providers.fn_arg_names = |tcx, id| {
         let hir = tcx.hir();
         let hir_id = hir.local_def_id_to_hir_id(id.expect_local());

compiler/rustc_ty_utils/src/ty.rs

@@ -5,7 +5,6 @@ use rustc_middle::ty::subst::Subst;
 use rustc_middle::ty::{
     self, Binder, EarlyBinder, Predicate, PredicateKind, ToPredicate, Ty, TyCtxt,
 };
-use rustc_span::Span;
 use rustc_trait_selection::traits;
 
 fn sized_constraint_for_ty<'tcx>(
@@ -103,21 +102,6 @@ fn adt_sized_constraint(tcx: TyCtxt<'_>, def_id: DefId) -> ty::AdtSizedConstrain
     ty::AdtSizedConstraint(result)
 }
 
-fn def_ident_span(tcx: TyCtxt<'_>, def_id: DefId) -> Option<Span> {
-    tcx.hir()
-        .get_if_local(def_id)
-        .and_then(|node| match node {
-            // A `Ctor` doesn't have an identifier itself, but its parent
-            // struct/variant does. Compare with `hir::Map::opt_span`.
-            hir::Node::Ctor(ctor) => ctor
-                .ctor_hir_id()
-                .and_then(|ctor_id| tcx.hir().find(tcx.hir().get_parent_node(ctor_id)))
-                .and_then(|parent| parent.ident()),
-            _ => node.ident(),
-        })
-        .map(|ident| ident.span)
-}
-
 /// See `ParamEnv` struct definition for details.
 #[instrument(level = "debug", skip(tcx))]
 fn param_env(tcx: TyCtxt<'_>, def_id: DefId) -> ty::ParamEnv<'_> {
@@ -480,7 +464,6 @@ pub fn provide(providers: &mut ty::query::Providers) {
     *providers = ty::query::Providers {
         asyncness,
         adt_sized_constraint,
-        def_ident_span,
         param_env,
         param_env_reveal_all_normalized,
         instance_def_size_estimate,

src/test/ui/closures/issue-87461.stderr

@@ -5,6 +5,12 @@ LL |     Ok(())
    |     -- ^^ expected `u16`, found `()`
    |     |
    |     arguments to this enum variant are incorrect
+   |
+note: tuple variant defined here
+  --> $SRC_DIR/core/src/result.rs:LL:COL
+   |
+LL |     Ok(#[stable(feature = "rust1", since = "1.0.0")] T),
+   |     ^^
 
 error[E0308]: mismatched types
   --> $DIR/issue-87461.rs:17:8
@@ -13,6 +19,12 @@ LL |     Ok(())
    |     -- ^^ expected `u16`, found `()`
    |     |
    |     arguments to this enum variant are incorrect
+   |
+note: tuple variant defined here
+  --> $SRC_DIR/core/src/result.rs:LL:COL
+   |
+LL |     Ok(#[stable(feature = "rust1", since = "1.0.0")] T),
+   |     ^^
 
 error[E0308]: mismatched types
   --> $DIR/issue-87461.rs:26:12
@@ -21,6 +33,12 @@ LL |         Ok(())
    |         -- ^^ expected `u16`, found `()`
    |         |
    |         arguments to this enum variant are incorrect
+   |
+note: tuple variant defined here
+  --> $SRC_DIR/core/src/result.rs:LL:COL
+   |
+LL |     Ok(#[stable(feature = "rust1", since = "1.0.0")] T),
+   |     ^^
 
 error: aborting due to 3 previous errors
 

src/test/ui/const-generics/const-argument-cross-crate-mismatch.stderr

@@ -5,6 +5,12 @@ LL |     let _ = const_generic_lib::function(const_generic_lib::Struct([0u8, 1u8
    |                                         ------------------------- ^^^^^^^^^^ expected an array with a fixed size of 3 elements, found one with 2 elements
    |                                         |
    |                                         arguments to this struct are incorrect
+   |
+note: tuple struct defined here
+  --> $DIR/auxiliary/const_generic_lib.rs:1:12
+   |
+LL | pub struct Struct<const N: usize>(pub [u8; N]);
+   |            ^^^^^^
 
 error[E0308]: mismatched types
   --> $DIR/const-argument-cross-crate-mismatch.rs:8:65
@@ -13,6 +19,12 @@ LL |     let _: const_generic_lib::Alias = const_generic_lib::Struct([0u8, 1u8,
    |                                       ------------------------- ^^^^^^^^^^^^^^^ expected an array with a fixed size of 2 elements, found one with 3 elements
    |                                       |
    |                                       arguments to this struct are incorrect
+   |
+note: tuple struct defined here
+  --> $DIR/auxiliary/const_generic_lib.rs:1:12
+   |
+LL | pub struct Struct<const N: usize>(pub [u8; N]);
+   |            ^^^^^^
 
 error: aborting due to 2 previous errors
 

src/test/ui/mismatched_types/issue-35030.stderr

@@ -11,6 +11,11 @@ LL |         Some(true)
    |
    = note: expected type parameter `bool` (type parameter `bool`)
                         found type `bool` (`bool`)
+note: tuple variant defined here
+  --> $SRC_DIR/core/src/option.rs:LL:COL
+   |
+LL |     Some(#[stable(feature = "rust1", since = "1.0.0")] T),
+   |     ^^^^
 
 error: aborting due to previous error