Auto merge of rust-lang#109765 - petrochenkov:encodeless, r=cjgillot

bors · bors · commit dfe024e1041d · 2023-04-11T10:32:41.000Z
rustc_metadata: Filter encoded data more aggressively using `DefKind`

I focused on data that contains spans, because spans are expensive to encode/decode/hash, but also touched `should_encode_visibility` too.

One incorrect use of impl visibility in diagnostics is also replaced with trait visibility.
diff --git a/compiler/rustc_hir_analysis/src/astconv/mod.rs b/compiler/rustc_hir_analysis/src/astconv/mod.rs
@@ -2580,7 +2580,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
                 tcx.all_impls(trait_def_id)
                     .filter(|impl_def_id| {
                         // Consider only accessible traits
-                        tcx.visibility(*impl_def_id).is_accessible_from(self.item_def_id(), tcx)
+                        tcx.visibility(trait_def_id).is_accessible_from(self.item_def_id(), tcx)
                             && tcx.impl_polarity(impl_def_id) != ty::ImplPolarity::Negative
                     })
                     .filter_map(|impl_def_id| tcx.impl_trait_ref(impl_def_id))
diff --git a/compiler/rustc_metadata/src/rmeta/decoder.rs b/compiler/rustc_metadata/src/rmeta/decoder.rs
@@ -749,6 +749,10 @@ impl CrateRoot {
 }
 
 impl<'a, 'tcx> CrateMetadataRef<'a> {
+    fn missing(self, descr: &str, id: DefIndex) -> ! {
+        bug!("missing `{descr}` for {:?}", self.local_def_id(id))
+    }
+
     fn raw_proc_macro(self, id: DefIndex) -> &'a ProcMacro {
         // DefIndex's in root.proc_macro_data have a one-to-one correspondence
         // with items in 'raw_proc_macros'.
@@ -782,8 +786,13 @@ 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 =
-            self.root.tables.def_ident_span.get(self, item_index).unwrap().decode((self, sess));
+        let span = self
+            .root
+            .tables
+            .def_ident_span
+            .get(self, item_index)
+            .unwrap_or_else(|| self.missing("def_ident_span", item_index))
+            .decode((self, sess));
         Some(Ident::new(name, span))
     }
 
@@ -812,7 +821,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
             .tables
             .def_span
             .get(self, index)
-            .unwrap_or_else(|| panic!("Missing span for {index:?}"))
+            .unwrap_or_else(|| self.missing("def_span", index))
             .decode((self, sess))
     }
 
@@ -924,7 +933,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
             .tables
             .visibility
             .get(self, id)
-            .unwrap()
+            .unwrap_or_else(|| self.missing("visibility", id))
             .decode(self)
             .map_id(|index| self.local_def_id(index))
     }
@@ -934,7 +943,12 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
     }
 
     fn get_expn_that_defined(self, id: DefIndex, sess: &Session) -> ExpnId {
-        self.root.tables.expn_that_defined.get(self, id).unwrap().decode((self, sess))
+        self.root
+            .tables
+            .expn_that_defined
+            .get(self, id)
+            .unwrap_or_else(|| self.missing("expn_that_defined", id))
+            .decode((self, sess))
     }
 
     fn get_debugger_visualizers(self) -> Vec<rustc_span::DebuggerVisualizerFile> {
diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs
@@ -811,7 +811,7 @@ fn analyze_attr(attr: &Attribute, state: &mut AnalyzeAttrState) -> bool {
     should_encode
 }
 
-fn should_encode_visibility(def_kind: DefKind) -> bool {
+fn should_encode_span(def_kind: DefKind) -> bool {
     match def_kind {
         DefKind::Mod
         | DefKind::Struct
@@ -823,25 +823,136 @@ fn should_encode_visibility(def_kind: DefKind) -> bool {
         | DefKind::ForeignTy
         | DefKind::TraitAlias
         | DefKind::AssocTy
+        | DefKind::TyParam
         | DefKind::Fn
         | DefKind::Const
-        | DefKind::Static(..)
+        | DefKind::Static(_)
         | DefKind::Ctor(..)
         | DefKind::AssocFn
         | DefKind::AssocConst
-        | DefKind::Macro(..)
+        | DefKind::Macro(_)
+        | DefKind::AnonConst
+        | DefKind::InlineConst
+        | DefKind::OpaqueTy
+        | DefKind::Field
+        | DefKind::Impl { .. }
+        | DefKind::Closure
+        | DefKind::Generator => true,
+        DefKind::ConstParam
+        | DefKind::ExternCrate
+        | DefKind::Use
+        | DefKind::ForeignMod
+        | DefKind::ImplTraitPlaceholder
+        | DefKind::LifetimeParam
+        | DefKind::GlobalAsm => false,
+    }
+}
+
+fn should_encode_attrs(def_kind: DefKind) -> bool {
+    match def_kind {
+        DefKind::Mod
+        | DefKind::Struct
+        | DefKind::Union
+        | DefKind::Enum
+        | DefKind::Variant
+        | DefKind::Trait
+        | DefKind::TyAlias
+        | DefKind::ForeignTy
+        | DefKind::TraitAlias
+        | DefKind::AssocTy
+        | DefKind::Fn
+        | DefKind::Const
+        | DefKind::Static(_)
+        | DefKind::AssocFn
+        | DefKind::AssocConst
+        | DefKind::Macro(_)
+        | DefKind::Field
+        | DefKind::Impl { .. } => true,
+        DefKind::TyParam
+        | DefKind::ConstParam
+        | DefKind::Ctor(..)
+        | DefKind::ExternCrate
         | DefKind::Use
         | DefKind::ForeignMod
+        | DefKind::AnonConst
+        | DefKind::InlineConst
         | DefKind::OpaqueTy
         | DefKind::ImplTraitPlaceholder
-        | DefKind::Impl { .. }
+        | DefKind::LifetimeParam
+        | DefKind::GlobalAsm
+        | DefKind::Closure
+        | DefKind::Generator => false,
+    }
+}
+
+fn should_encode_expn_that_defined(def_kind: DefKind) -> bool {
+    match def_kind {
+        DefKind::Mod
+        | DefKind::Struct
+        | DefKind::Union
+        | DefKind::Enum
+        | DefKind::Variant
+        | DefKind::Trait
+        | DefKind::Impl { .. } => true,
+        DefKind::TyAlias
+        | DefKind::ForeignTy
+        | DefKind::TraitAlias
+        | DefKind::AssocTy
+        | DefKind::TyParam
+        | DefKind::Fn
+        | DefKind::Const
+        | DefKind::ConstParam
+        | DefKind::Static(_)
+        | DefKind::Ctor(..)
+        | DefKind::AssocFn
+        | DefKind::AssocConst
+        | DefKind::Macro(_)
+        | DefKind::ExternCrate
+        | DefKind::Use
+        | DefKind::ForeignMod
+        | DefKind::AnonConst
+        | DefKind::InlineConst
+        | DefKind::OpaqueTy
+        | DefKind::ImplTraitPlaceholder
+        | DefKind::Field
+        | DefKind::LifetimeParam
+        | DefKind::GlobalAsm
+        | DefKind::Closure
+        | DefKind::Generator => false,
+    }
+}
+
+fn should_encode_visibility(def_kind: DefKind) -> bool {
+    match def_kind {
+        DefKind::Mod
+        | DefKind::Struct
+        | DefKind::Union
+        | DefKind::Enum
+        | DefKind::Variant
+        | DefKind::Trait
+        | DefKind::TyAlias
+        | DefKind::ForeignTy
+        | DefKind::TraitAlias
+        | DefKind::AssocTy
+        | DefKind::Fn
+        | DefKind::Const
+        | DefKind::Static(..)
+        | DefKind::Ctor(..)
+        | DefKind::AssocFn
+        | DefKind::AssocConst
+        | DefKind::Macro(..)
         | DefKind::Field => true,
-        DefKind::TyParam
+        DefKind::Use
+        | DefKind::ForeignMod
+        | DefKind::TyParam
         | DefKind::ConstParam
         | DefKind::LifetimeParam
         | DefKind::AnonConst
         | DefKind::InlineConst
+        | DefKind::OpaqueTy
+        | DefKind::ImplTraitPlaceholder
         | DefKind::GlobalAsm
+        | DefKind::Impl { .. }
         | DefKind::Closure
         | DefKind::Generator
         | DefKind::ExternCrate => false,
@@ -1160,11 +1271,17 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
             let def_kind = tcx.opt_def_kind(local_id);
             let Some(def_kind) = def_kind else { continue };
             self.tables.opt_def_kind.set_some(def_id.index, def_kind);
-            let def_span = tcx.def_span(local_id);
-            record!(self.tables.def_span[def_id] <- def_span);
-            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) {
+            if should_encode_span(def_kind) {
+                let def_span = tcx.def_span(local_id);
+                record!(self.tables.def_span[def_id] <- def_span);
+            }
+            if should_encode_attrs(def_kind) {
+                self.encode_attrs(local_id);
+            }
+            if should_encode_expn_that_defined(def_kind) {
+                record!(self.tables.expn_that_defined[def_id] <- self.tcx.expn_that_defined(def_id));
+            }
+            if should_encode_span(def_kind) && 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() {
@@ -1523,23 +1640,32 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
         })
     }
 
-    fn encode_info_for_item(&mut self, def_id: DefId, item: &'tcx hir::Item<'tcx>) {
+    fn encode_info_for_item(&mut self, item: &'tcx hir::Item<'tcx>) {
         let tcx = self.tcx;
-
+        let def_id = item.owner_id.to_def_id();
         debug!("EncodeContext::encode_info_for_item({:?})", def_id);
 
+        let record_associated_item_def_ids = |this: &mut Self, def_ids: &[DefId]| {
+            record_array!(this.tables.children[def_id] <- def_ids.iter().map(|&def_id| {
+                assert!(def_id.is_local());
+                def_id.index
+            }))
+        };
+
         match item.kind {
             hir::ItemKind::Fn(ref sig, .., body) => {
                 self.tables.asyncness.set_some(def_id.index, sig.header.asyncness);
                 record_array!(self.tables.fn_arg_names[def_id] <- self.tcx.hir().body_param_names(body));
                 self.tables.constness.set_some(def_id.index, sig.header.constness);
+                record!(self.tables.fn_sig[def_id] <- tcx.fn_sig(def_id));
+                self.tables.is_intrinsic.set(def_id.index, tcx.is_intrinsic(def_id));
             }
             hir::ItemKind::Macro(ref macro_def, _) => {
                 self.tables.is_macro_rules.set(def_id.index, macro_def.macro_rules);
                 record!(self.tables.macro_definition[def_id] <- &*macro_def.body);
             }
             hir::ItemKind::Mod(ref m) => {
-                return self.encode_info_for_mod(item.owner_id.def_id, m);
+                self.encode_info_for_mod(item.owner_id.def_id, m);
             }
             hir::ItemKind::OpaqueTy(ref opaque) => {
                 self.encode_explicit_item_bounds(def_id);
@@ -1550,9 +1676,11 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
             hir::ItemKind::Impl(hir::Impl { defaultness, constness, .. }) => {
                 self.tables.impl_defaultness.set_some(def_id.index, *defaultness);
                 self.tables.constness.set_some(def_id.index, *constness);
+                self.tables.impl_polarity.set_some(def_id.index, self.tcx.impl_polarity(def_id));
+
+                if let Some(trait_ref) = self.tcx.impl_trait_ref(def_id) {
+                    record!(self.tables.impl_trait_ref[def_id] <- trait_ref);
 
-                let trait_ref = self.tcx.impl_trait_ref(def_id);
-                if let Some(trait_ref) = trait_ref {
                     let trait_ref = trait_ref.skip_binder();
                     let trait_def = self.tcx.trait_def(trait_ref.def_id);
                     if let Ok(mut an) = trait_def.ancestors(self.tcx, def_id) {
@@ -1570,71 +1698,34 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
                     }
                 }
 
-                let polarity = self.tcx.impl_polarity(def_id);
-                self.tables.impl_polarity.set_some(def_id.index, polarity);
+                let associated_item_def_ids = self.tcx.associated_item_def_ids(def_id);
+                record_associated_item_def_ids(self, associated_item_def_ids);
+                for &trait_item_def_id in associated_item_def_ids {
+                    self.encode_info_for_impl_item(trait_item_def_id);
+                }
             }
             hir::ItemKind::Trait(..) => {
-                let trait_def = self.tcx.trait_def(def_id);
-                record!(self.tables.trait_def[def_id] <- trait_def);
+                record!(self.tables.trait_def[def_id] <- self.tcx.trait_def(def_id));
+
+                let associated_item_def_ids = self.tcx.associated_item_def_ids(def_id);
+                record_associated_item_def_ids(self, associated_item_def_ids);
+                for &item_def_id in associated_item_def_ids {
+                    self.encode_info_for_trait_item(item_def_id);
+                }
             }
             hir::ItemKind::TraitAlias(..) => {
-                let trait_def = self.tcx.trait_def(def_id);
-                record!(self.tables.trait_def[def_id] <- trait_def);
-            }
-            hir::ItemKind::ExternCrate(_) | hir::ItemKind::Use(..) => {
-                bug!("cannot encode info for item {:?}", item)
+                record!(self.tables.trait_def[def_id] <- self.tcx.trait_def(def_id));
             }
-            hir::ItemKind::Static(..)
+            hir::ItemKind::ExternCrate(_)
+            | hir::ItemKind::Use(..)
+            | hir::ItemKind::Static(..)
             | hir::ItemKind::Const(..)
             | hir::ItemKind::Enum(..)
             | hir::ItemKind::Struct(..)
             | hir::ItemKind::Union(..)
             | hir::ItemKind::ForeignMod { .. }
             | hir::ItemKind::GlobalAsm(..)
             | hir::ItemKind::TyAlias(..) => {}
-        };
-        // FIXME(eddyb) there should be a nicer way to do this.
-        match item.kind {
-            hir::ItemKind::Impl { .. } | hir::ItemKind::Trait(..) => {
-                let associated_item_def_ids = self.tcx.associated_item_def_ids(def_id);
-                record_array!(self.tables.children[def_id] <-
-                    associated_item_def_ids.iter().map(|&def_id| {
-                        assert!(def_id.is_local());
-                        def_id.index
-                    })
-                );
-            }
-            _ => {}
-        }
-        if let hir::ItemKind::Fn(..) = item.kind {
-            record!(self.tables.fn_sig[def_id] <- tcx.fn_sig(def_id));
-            self.tables.is_intrinsic.set(def_id.index, tcx.is_intrinsic(def_id));
-        }
-        if let hir::ItemKind::Impl { .. } = item.kind {
-            if let Some(trait_ref) = self.tcx.impl_trait_ref(def_id) {
-                record!(self.tables.impl_trait_ref[def_id] <- trait_ref);
-            }
-        }
-        // 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
-        // normally in the visitor walk.
-        match item.kind {
-            hir::ItemKind::Impl { .. } => {
-                for &trait_item_def_id in
-                    self.tcx.associated_item_def_ids(item.owner_id.to_def_id()).iter()
-                {
-                    self.encode_info_for_impl_item(trait_item_def_id);
-                }
-            }
-            hir::ItemKind::Trait(..) => {
-                for &item_def_id in
-                    self.tcx.associated_item_def_ids(item.owner_id.to_def_id()).iter()
-                {
-                    self.encode_info_for_trait_item(item_def_id);
-                }
-            }
-            _ => {}
         }
     }
 
@@ -2020,10 +2111,7 @@ impl<'a, 'tcx> Visitor<'tcx> for EncodeContext<'a, 'tcx> {
     }
     fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) {
         intravisit::walk_item(self, item);
-        match item.kind {
-            hir::ItemKind::ExternCrate(_) | hir::ItemKind::Use(..) => {} // ignore these
-            _ => self.encode_info_for_item(item.owner_id.to_def_id(), item),
-        }
+        self.encode_info_for_item(item);
     }
     fn visit_foreign_item(&mut self, ni: &'tcx hir::ForeignItem<'tcx>) {
         intravisit::walk_foreign_item(self, ni);
diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs
@@ -687,7 +687,7 @@ impl Item {
                 return None;
             }
             // Variants always inherit visibility
-            VariantItem(..) => return None,
+            VariantItem(..) | ImplItem(..) => return None,
             // Trait items inherit the trait's visibility
             AssocConstItem(..) | TyAssocConstItem(..) | AssocTypeItem(..) | TyAssocTypeItem(..)
             | TyMethodItem(..) | MethodItem(..) => {
diff --git a/tests/ui/associated-types/associated-types-in-ambiguous-context.rs b/tests/ui/associated-types/associated-types-in-ambiguous-context.rs
diff --git a/tests/ui/associated-types/associated-types-in-ambiguous-context.stderr b/tests/ui/associated-types/associated-types-in-ambiguous-context.stderr

Original file line number	Diff line number	Diff line change
`@@ -687,7 +687,7 @@ impl Item {`
`687`	`687`	`return None;`
`688`	`688`	`}`
`689`	`689`	`// Variants always inherit visibility`
`690`		`- VariantItem(..) => return None,`
	`690`	`+ VariantItem(..) \| ImplItem(..) => return None,`
`691`	`691`	`// Trait items inherit the trait's visibility`
`692`	`692`	`AssocConstItem(..) \| TyAssocConstItem(..) \| AssocTypeItem(..) \| TyAssocTypeItem(..)`
`693`	`693`	`\| TyMethodItem(..) \| MethodItem(..) => {`