Avoid inst.body() duplicate call which reallocates AllocIds

[dkcumming]

This PR corrects a bug in generating the Stable MIR JSON artefacts. Prior to this fix allocations were being collected twice (calling Instance::body twice), which resulted in certain allocs (slices) having multiple allocation ids. The fix is to refer to previously collected items so that the collector does not assign allocation ids again.

Problem

There has been a disconnection between Ty and AllocId in the .smir.json artefacts (for slices). A constant should be able to be traced to an allocation by the Ty that will yield an AllocId. However attempting to do this lead to a case where the AllocId from the Ty was not in the Allocs map, and allocations in the map had AllocIds unseen in the rest of the .smir.json.

Cause

The disconnection betwen the Tys and AllocIds listed above came from there being a second collector pass calling Instance::body again that assigned new AllocIds for slices that were not connected to the Tys (still referencing the originals). The code that initiates the second collector pass is from stable-mir-json, while the code that re-allocates fresh ids is in rustc itself.

A rough trace of the pipeline goes like this:

Details

1. First collection initiated in collect_smir by let items = collect_items(tcx);
2. Each MonoItem is mapped to an Item by mk_item(tcx, item.clone(), name)
3. In mk_item if the MonoItem is MonoItemFn(inst: Instance) call Instance::body(&self) on inst by inst.body()
4. Instance::Body calls the rustc_smir Context::instance_body (for TablesWrapper that impl Context)
5. Context::instance_body calls BodyBuilder::new(tables.tcx, instance) to create a new BodyBuilder which then calls the BodyBuilder::build method
6. BodyBuilder::build creates mono_body which is of type rustc_middle::mir::Body (not stable_mir yet), which is then is converted to a stable_mir::mir::Body by mono_body.stable(tables)
7. Converting to a stable_mir::mir::Body through stable will convert all the BasicBlocks, Statements, Terminators, LocalDecls, and importantly rustc_middle::mir::Const into a stable_mir::ty::{TyConst,MirConst}
8. If the Const to covert is a rustc_middle::mir::Const::Val then it is converted with a new stable_mir::ty::Allocation as argument to the stable_mir::ty::ConstantKind::Allocated by ConstantKind::Allocated(alloc::new_allocation(ty, val, tables));
9. The creation of the new Allocation will match the ConstValue, and if it is ConstValue::Slice then the AllocId produced will be from let alloc_id = tables.tcx.reserve_and_set_memory_alloc(data); which will call TyCtxt::reserve_alloc_id and which will create a new allocation id by calling AllocMap::reserve <- IMPORTANT
10. Given 9. explains how the AllocId was created from calling inst.body(), let's skip over the remaining details in executing let items = collect_items(tcx); to collect_interned_values(tcx, items.iter().map(|i| &i.mono_item).collect::<Vec<_>>());
11. collect_interned_values iterates through items and if it matches a MonoItem::Fn(inst) then inst.body() is called again - PROBLEM. This repeats steps 4. - 9. again and at step 9. will allocate another AllocId for the InternedValueCollector that does not match what was originally allocated.

Solution

Once the Items are collected we can just thread a slice through and use the Body that is already collected which means inst.body() does not need to be called again and will not reallocate ids.

[cds-amal]

@dkcumming , your PR investigation/summary hints at a deeper issue: the pipeline is doing three conceptually related things as separate steps, each destroying information that the next step needs. I'll have to give this a think 🤔 🧐

[dkcumming]

@cds-amal I will circle back to #121 after some downstream work this fix opens up