Fix name-demangleling of provided types #102
Closed
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
|
I also wonder if this is the right place for the tests. |
|
I still think it would be better to not let the strange logical names happen in the first place. |
| @@ -65,6 +65,7 @@ | |||
| </ItemGroup> | |||
| <ItemGroup> | |||
| <Compile Include="NUnitFrameworkShims.fs" Condition="'$(TargetFramework)' == 'sl3-wp'" /> | |||
| <Compile Include="ManglelingNameOfProvidedTypes.fs" /> | |||
There was a problem hiding this comment.
Proper spelling is "mangling"
|
Looks nice. Just a couple cleanup suggestions. |
|
There is still the question if this is the right fix. |
|
see #203 for the second approach. |
This was referenced Feb 3, 2015
|
closing this in favor of #203 |
latkin
pushed a commit
that referenced
this pull request
Feb 5, 2015
fixes #102 closes #203 commit 6cc3227 Author: Steffen Forkmann <[email protected]> Date: Tue Feb 3 15:30:23 2015 +0100 Do not generate logical type names like "MyNamespace.Test," commit b40d937 Author: Steffen Forkmann <[email protected]> Date: Sun Jan 25 15:08:36 2015 +0100 Fix demangling of provided types with only default values - fixes #98 commit 515501a Author: Steffen Forkmann <[email protected]> Date: Sun Jan 25 14:56:45 2015 +0100 Create test case for #98 commit 5569580 Author: Steffen Forkmann <[email protected]> Date: Sun Jan 25 14:50:04 2015 +0100 Added some tests to describe current behaviour of NameMangling of provided types commit be2d157 Author: Steffen Forkmann <[email protected]> Date: Sun Jan 25 14:22:49 2015 +0100 Extract computeMangledNameWithoutDefaultArgValues to make it testable commit bbdd2a3 Author: Steffen Forkmann <[email protected]> Date: Sun Jan 25 13:13:00 2015 +0100 Cleanup: removed commented code which uses deleted functions
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.
This fixes #98
As reported in #98 the mangleling routine removes parameters which match the default value from the
typeLogicalName. If all parameters match their default value we have a mangled name liketypeName,which is also reported:I created a couple of unittests which pin the current behaviour. In order to do so I extracted the mangleling routine to make it testable.
I then added a testcase which tries to demangle
"MyNamespace.Test,"and fails.The last commit uses the strategy @vladima suggested in #98 and fixes the test case.
I think this will work fine, but I wonder if we should fix https://github.com/Microsoft/visualfsharp/blob/fsharp4/src/fsharp/PrettyNaming.fs#L466 instead and just don't generate logical type names with trailing
,.