Skip to content
This repository has been archived by the owner on Nov 1, 2020. It is now read-only.

Add F# support #2165

Merged
merged 7 commits into from Nov 8, 2016
Merged

Add F# support #2165

merged 7 commits into from Nov 8, 2016

Conversation

tonerdo
Copy link
Contributor

@tonerdo tonerdo commented Nov 8, 2016

This PR lays the groundwork for native compilation of F# programs with CoreRT.
Successful compilation and execution of a simple "Hello World!" F# program was achieved

#2057

This enables F# apps to be built against CoreRT
until System.Diagnostics.Tracing is ported to Unix
- This fixes "unimplemented runtime method" runtime error
for GetNativeSystemInfo method.
- Achieves successful run of a simple "Hello Wolrd!"
F# console app
Fix failed Windows CI build
@@ -59,6 +59,7 @@ public static void FailFast(String message, Exception exception)
RuntimeExceptionHelpers.FailFast(message, exception);
}

#if !PLATFORM_UNIX
Copy link
Member

Choose a reason for hiding this comment

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

Could you please move this to Environment.Windows.cs instead of adding the ifdef?

(BTW: We are using corefx coding guidelines in CoreRT - https://github.com/dotnet/corefx/tree/master/Documentation/coding-guidelines.)

_SC_NPROCESSORS_ONLN = 3,
}

[DllImport(Interop.Libraries.CoreLibNative, EntryPoint = "CoreLibNative_SysConf", SetLastError = true)]
Copy link
Member

@jkotas jkotas Nov 8, 2016

Choose a reason for hiding this comment

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

It would be better to just reference the implementation in Libraries.SystemNative (just like in corefx), and avoid adding the C++ impl.

Are there problems you are running into if you do this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I do use Libraries.SystemNative the F# app builds fine but then produces a 42097 segmentation fault when executed. Still in the process of debugging it though

Copy link
Member

Choose a reason for hiding this comment

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

You can trying merging with #2170 - it may fix the segmentation faults you are seeing.

@@ -104,4 +104,9 @@ extern "C"
{
Copy link
Member

Choose a reason for hiding this comment

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

You may be able to delete GetNativeSystemInfo from this file - it should not be needed anymore.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

GetNativeSystemInfo has been removed

Move Windows specific ProcessorCount method
to Environment.Windows.cs
The GetNativeSystemInfo method is no longer called
on unix so can be removed
System.Native native lib already includes
a SysConf method
@tonerdo
Copy link
Contributor Author

tonerdo commented Nov 8, 2016

@jkotas having an updated System.Native did the trick. We're good 👍

@jkotas jkotas merged commit 75f57dd into dotnet:master Nov 8, 2016
@jkotas
Copy link
Member

jkotas commented Nov 8, 2016

Thank you!

@mfalade
Copy link

mfalade commented Nov 28, 2016

Awesome dude! 👍

@tonerdo tonerdo mentioned this pull request Mar 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants