Changed generation of PrivateImplementationDetails to append module name... #1546
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| /// Given an input string changes it to be acceptable as a part of a type name. | ||
| /// For now we will simply replace '.' with '_'as the most common case. | ||
| /// </summary> | ||
| internal static string MangleForTypeNameIfNeeded(this string original) |
There was a problem hiding this comment.
This should also be used at
There was a problem hiding this comment.
it is there. - GitHub lists this as the first changed file.
|
👍 though please note my comment |
|
|
||
| if (submissionSlotIndex >= 0) | ||
| { | ||
| name += submissionSlotIndex.ToString(); |
There was a problem hiding this comment.
Previously, the slot index was inside the angle brackets and instead of the module name. Does this matter? @tmat ?
|
Is there a test that actually tests the netmodule-merge case? I can't seem to find it. |
|
@agocke - there is a test added by Neal in last change. I did not need to change it. MultipleNetmodulesWithPrivateImplementationDetails |
…ame only when dealing with netmudules. The goal of module name apending is to avoid clashes when combining multiple netmodules into multifile assembly. When building a regular assembly, appending module name is not serving any purpose and just causes unnecessary metadata differences. Also in this change - when we do apend the module name, replace '.' with '_' when that happens. For example when we build a netmodule and its name is Foo.Bar.dll More complicated name mangling schemes were discussed, but at this point we will do a simple '.' --> '_' as the least destabilizing change which is still sufficient in the most common case of having dots in the module name. Fixes dotnet#1430
VSadov
added a commit
that referenced
this pull request
Mar 25, 2015
Changed generation of PrivateImplementationDetails to append module name...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
... only when dealing with netmudules.
The goal of module name apending is to avoid clashes when combining multiple netmodules into multifile assembly. When building a regular assembly, appending module name is not serving any purpose and just causes unnecessary metadata differences.
Also in this change - when we do apend the module name, replace '.' with
'_'when that happens. For example when we build a netmodule and its name is Foo.Bar.dllMore complicated name mangling schemes were discussed, but at this point we will do a simple '.' --> '_' as the least destabilizing change which is still sufficient in the most common case of having dots in the module name.
Fixes #1430