Skip to content

Conversation

@KevinRansom
Copy link
Contributor

FSharp.Compiler.dll and the FSharp.LanguageService.Compiler.dll compiled with peverify issues.

new System.String(NativePtr.ofNativeInt (m.Addr i), 0, n, System.Text.Encoding.UTF8)
#endif
System.Runtime.InteropServices.Marshal.PtrToStringAnsi((m.Addr i), n)
//#if FX_RESHAPED_REFLECTION
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove code?

@OmarTawfik
Copy link
Contributor

I can rebase my PR and rerun the build after you merge the existing fix.

@KevinRansom KevinRansom merged commit b877799 into dotnet:master Aug 23, 2016
@KevinRansom KevinRansom deleted the peverify branch August 26, 2016 05:37
@dsyme
Copy link
Contributor

dsyme commented Aug 26, 2016

I don't think this needed a fix - the code used was valid IL and there was no functionality bug here. It's just that the code was not verifiable. Not all valid IL code is verifiable. For example, managed C++ and unsafe C# would have emitted equivalent code AFAIK.

But what I'm most concerned about is a perf regression. If Marshal.ReadByte is slower in any way (how do we know? does it do any kind of check?) then we will likely see a perf regression on binary reading. Best not to take the risk at this stage.

best
don

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants