rustc_metadata: Deduplicate code between encode_info_for_item and encode_info_for_foreign_item

[petrochenkov]

The set of data to record for regular items and foreign items is pretty similar, but slightly different.
When metadata encoding is done independently for them, issues like #77375 arise.

encode_info_for_trait_item and encode_info_for_impl_item are also similar, and also slightly different (all in different directions), so they may be included into the refactoring as well.

[jyn514]

Relevant code:

rust/compiler/rustc_metadata/src/rmeta/encoder.rs

Line 1223 in d902752

fn encode_info_for_item(&mut self, def_id: DefId, item: &'tcx hir::Item<'tcx>) {

rust/compiler/rustc_metadata/src/rmeta/encoder.rs

Line 1725 in d902752

fn encode_info_for_foreign_item(&mut self, def_id: DefId, nitem: &hir::ForeignItem<'_>) {

rust/compiler/rustc_metadata/src/rmeta/encoder.rs

Line 965 in d902752

fn encode_info_for_trait_item(&mut self, def_id: DefId) {

rust/compiler/rustc_metadata/src/rmeta/encoder.rs

Line 1054 in d902752

fn encode_info_for_impl_item(&mut self, def_id: DefId) {

[1c3t3a]

I'd like to give this a try if mentoring is available!

[1c3t3a]

Is it 'just' refactoring the methods all together?

[petrochenkov]

The goal is to make the code nicer and less error-prone, but I'm not sure which exactly refactoring to apply to do that.
As I mentioned above all the methods are slightly different. Perhaps some of the called encode_* functions can be tweaked to accommodate for the differences.
The issue is E-easy in the sense that the changes will certainly be easy to implement, but what those changes should be is a question.
It needs experimentation.

[pitaj]

Hi I'd like to try my hand at this so I made a small change to align the contents of encode_info_for_item and encode_info_for_foreign_item. I created new conditional helper methods and rearranged things to both have the following calls:

        self.encode_ident_span(def_id, ...);
        record!(self.tables.visibility[def_id] <-
            ty::Visibility::from_hir(...));
        record!(self.tables.attributes[def_id] <- ...);
        self.encode_stability(def_id);
        self.encode_const_stability(def_id);
        self.encode_deprecation(def_id);
        self.encode_item_type_when(def_id, ...);
        self.encode_inherent_implementations(def_id);
        self.encode_function_signature_when(
            def_id,
            matches!(...),
        );
        self.encode_generics(def_id);
        self.encode_variances_when(def_id, matches!(...));
        self.encode_explicit_predicates(def_id);
        self.encode_generics_explicit_predicates_inferred_outlives_when(def_id, ...);

I'd then pull those calls out into a single helper method. Am I headed in the right direction? If so I'll keep going. My strategy is to dedupe _item and _foreign_item, then dedupe _trait_item and _impl_item, then dedupe the result of those if it makes sense to.

Couple questions:

• What is the difference between tcx.def_span() and item.span?
• Does the order of record!s matter?

[petrochenkov]

@pitaj

Does the order of record!s matter?

No, it shouldn't matter.
(And if it does you'll get test suit failures anyway.)

What is the difference between tcx.def_span() and item.span?

For functions def_span returns the span of its header, not the whole function with body, for other items it will return item.span.
def_span is preferable by default.

[pitaj]

@rustbot claim

[petrochenkov]

This is no longer relevant with the recent metadata encoding changes in #80347 and related PRs.