-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Metadata writer refactoring (4) #9699
Conversation
@dotnet/roslyn-compiler Please review. Feel free to skip all files in The change was tested thoroughly to avoid functional and perf regressions. Both CoreFX and Roslyn repos were built with a compiler containing these changes and the results were bit-for-bit equal to the corresponding binaries built by a compiler without this change (in deterministic mode). |
|
||
if (signature == 0 || signature == 1) | ||
{ | ||
nameFlags = (System.Reflection.AssemblyNameFlags)value; | ||
nameFlags = (AssemblyFlags)(AssemblyNameFlags)value; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see an explicit conversion operator for these types -- is there a reason why the two conversions are necessary instead of one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Value is object, so the first cast is unbox, which needs to match the type it was boxed with, and the second is cast between enums.
@dotnet/roslyn-compiler Please review. Mac/Linux failures look like an issue in CoreCLR. Investigating. |
The failures on CoreCLR are caused by missing HashAlgorithm.TransformBlock. The fix is to move to netstandard1.3. |
@dotnet/roslyn-compiler Ping. |
|
||
if (configureToExecuteInAppContainer) | ||
{ | ||
result |= DllCharacteristics.AppContainer; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is WindowsRuntime
referred to as AppContainer
in some other context? How did you choose this name?
LGTM to the degree GitHub was able to show the diff. |
retest vsi please |
1 similar comment
retest vsi please |
test determinism please |
[Equivalent to https://github.com//pull/9459, with all commits squashed into a single one for easier review.]
Refactors PeWriter into components independent of Roslyn, so that it can later be moved to System.Reflection.Metadata in CoreFX.
The goal is to provide PE/COFF builder and ECMA-335 metadata builder that can be used broadly to write managed and native PE files. The goal is not to immediately provide the full support for writing all existing sections of the PE/COFF format but rather to enable composing PE/COFF files from section blobs and to provide builders for sections commonly appearing in managed PE files.
Includes implementation of a set of types, blob encoders that can be used for serialization of various blob structures found in the ECMA-335 metadata (especially signatures). The introduced abstraction provides type-system enforced structure to the blobs with minimal runtime overhead.
More work will follow to polish the public surface of the builders and serializers.
Tracked by work item: #6904