Remove Reflection.Emit APIs

[i3arnon]

Removes all unused leftover Reflection.Emit APIs

Fix #49452

[krwq]

@i3arnon the only thing which is left is to remove src/libraries/System.Reflection.Emit/src/MatchingRefApiCompatBaseline.txt

[krwq]

@i3arnon seems you got some build errors:

/__w/1/s/src/coreclr/vm/ceeload.cpp:12445:37: error: out-of-line definition of 'Create' does not match any declaration in 'ReflectionModule'
ReflectionModule *ReflectionModule::Create(Assembly *pAssembly, PEFile *pFile, AllocMemTracker *pamTracker, LPCWSTR szName, BOOL fIsTransient)
                                    ^~~~~~
/__w/1/s/src/coreclr/vm/ceeload.cpp:12472:14: error: no member named 'SetIsTransient' in 'ReflectionModule'
    pModule->SetIsTransient(fIsTransient ? true : false);
    ~~~~~~~  ^
/__w/1/s/src/coreclr/vm/ceeload.cpp:12501:5: error: use of undeclared identifier 'm_fIsTransient'
    m_fIsTransient = false;

[eerhardt]

What about

• AssemblyBuilder.DefineDynamicModule(System.String, System.Boolean)
• ModuleBuilder.DefineDocument(System.String, System.Guid, System.Guid, System.Guid)

I believe those public methods should be deleted as well

[i3arnon]

What about

• AssemblyBuilder.DefineDynamicModule(System.String, System.Boolean)
• ModuleBuilder.DefineDocument(System.String, System.Guid, System.Guid, System.Guid)

I believe those public methods should be deleted as well

Should I just delete the API or follow through all the way? It goes quite deep.
Removing AssemblyBuilder.DefineDynamicModule(System.String, System.Boolean) obviates emitSymbolInfo:

https://github.com/i3arnon/runtime/blob/6351328858949aed8c1ab452bc93339b822d9ba7/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/AssemblyBuilder.cs#L348-L356

And it continues from there.
I'm willing to cut as much as possible :), just want to make sure it's fine..

[eerhardt]

I think for this PR, let's just remove the public methods. It would definitely go quite deep to start removing the unused emitSymbolInfo parameter. I think that refactoring can be done in a follow up PR.

[i3arnon]

The test failures don't seem to be related.
Could they be transient? Is it possible to rerun them?

[jeffhandley]

I've kicked off a re-run of the failed jobs.

@krwq This PR is assigned to you for follow-up before the RC1 snap.

[krwq]

@i3arnon this looks good to me, are you planning to also remove System.Reflection.Emit.ModuleBuilder.GetModuleHandleImpl? Seems that's the only extra API now. Consider changing it to just internal if implementation is used in many places

[eerhardt]

are you planning to also remove System.Reflection.Emit.ModuleBuilder.GetModuleHandleImpl? Seems that's the only extra API now. Consider changing it to just internal if implementation is used in many places

I don't believe we should change this. See the code comment for an explanation.

fyi @MichalStrehovsky since this references CoreRT.

[eerhardt]

LGTM. Thanks @i3arnon for the contribution!

[i3arnon]

I think for this PR, let's just remove the public methods. It would definitely go quite deep to start removing the unused emitSymbolInfo parameter. I think that refactoring can be done in a follow up PR.

Now that this is merge.. should I do that follow up PR?

[krwq]

@i3arnon yes please, if don't have time please create an issue

[MichalStrehovsky]

fyi @MichalStrehovsky since this references CoreRT.

Yeah, the reflection stack in NativeAOT is not implemented in CoreLib so keeping this protected internal allows the NativeAOT reflection stack to override. If we undo that here, we'll need to undo that change in NativeAOT again. Appreciate that we don't have to :).