Emit attribute changes for parameters

[davidwengier]

Fixes #54777

This was a pain! In a way I'm glad I didn't do this while doing custom attributes because I probably would have gone crosseyed.

[davidwengier]

Oh, as painful as some of this is, most of it is necessary for parameter renames anyway, so we get bang for our buck!

[davidwengier]

/azp run

[davidwengier]

The build errors don't match the file contents ¯\_(ツ)_/¯

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[tmat]

    protected override void PopulateEncLogTableRows(ImmutableArray<int> rowCounts)

Let's make PopulateEncTables virtual and PopulateEncLogTableRows and PopulateEncMapTableRows private non-virtual.

Then _customAttributeEncMapRows and _paramEncMapRows can be passed between these methods isntead of being fields defined on the writer.

---

Refers to: src/Compilers/Core/Portable/Emit/EditAndContinue/DeltaMetadataWriter.cs:782 in c13f258. [](commit_id = c13f258, deletion_comment = False)

[tmat]

🕐

[davidwengier]

Tomas and I discussed offline and we decided to just have the compiler emit Param rows whenever a method is updated, rather than try to be smart about it. That simplifies this PR a lot, but unfortunately, and I normally hate to do this after a review has started, it did mean I had to rebase and force push to drop the relevant commits.

[tmat]

[davidwengier]

@cston Care to have another look?
@RikkiGibson Can you review?

Would love to get this in before snap tomorrow.

[RikkiGibson]

I'm assuming the behavior of type parameters is unchanged here, i.e. adding an attribute to a type parameter during an EnC session is not supported.

[tmat]

Yes, editing generic types is not supported.

[RikkiGibson]

It feels like it might be reasonable to fail in retail builds here if the handle isn't present, e.g. by changing the code to var firstRowId = _previousGeneration.FirstParamRowMap[handle]. However, I don't know exactly what the failure mode is here if we proceed into the loop with a firstRowId which is just 0 instead of being something meaningful for the given handle.

[davidwengier]

This would still fail later, when trying to emit an EncLog row for Param row 0, and I think that would give a more descriptive exception that "element not found".

Realistically this can only fail if someone changes how EnC works in a way that would break a lot of things, like perhaps allowing direct creation of a delta without a baseline, so a debug assert feels appropriate to me.

[davidwengier]

Thanks all!