Add the module name to generated PrivateImplementationDetails class - #1257
Conversation
so that there will be no conflict if they are used as netmodules Fixes dotnet#1228
gafter
added
Area-Compilers
4 - In Review
|
@jaredpar @VSadov @AlekseyTs @agocke @VladimirReshetnikov Could a couple of you review this please? |
There was a problem hiding this comment.
I removed some unused type parameters
|
LGTM |
There was a problem hiding this comment.
Please use non-ASCII module name (e.g. "ＭＯＤＵＬＥ") in at least one test.
There was a problem hiding this comment.
Pedantic: could you fix the comment alignment.
There was a problem hiding this comment.
I'd rather remove it, as it is not true. This is the module builder, not the module. This was terribly confusing to me when I was working on this code. I'd love to disentangle the two types.
There was a problem hiding this comment.
Could we add a comment about the necessity of this naming convention?
|
👍 Added some minor comments. |
There was a problem hiding this comment.
You could use EmitToImageReference helper instead.
|
LGTM |
Add a test for multiple modules using anonymous types.
There was a problem hiding this comment.
@tmat Please tell me if this is OK. It was required to get tests to pass, as otherwise we use a random GUID for the module name in E&C, which means that the generated code is not deterministic even for testing.
Add the module name to generated PrivateImplementationDetails class
7 participants
so that there will be no conflict if they are used as netmodules
Fixes #1228