Skip to content

Simple changes to make it compatible with WASM #8722

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
KevinRansom merged 3 commits into from
May 28, 2020
Merged

Simple changes to make it compatible with WASM #8722

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0d311d8
ifEmptyUse fSharpCompilerLocation
amieres Mar 15, 2020
83387f4
Check GetEnvironmentVariable("FSHARP_COMPILER_BIN") before tryAppConfig
amieres Mar 15, 2020
87d6163
ifEmptyUse with IsNullOrWhiteSpace
amieres Mar 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/fsharp/DotNetFrameworkDependencies.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,13 @@ module internal FSharp.Compiler.DotNetFrameworkDependencies
// Use the location of this dll
location

let inline ifEmptyUse alternative filename = if String.IsNullOrWhiteSpace filename then alternative else filename

let getFSharpCoreLibraryName = "FSharp.Core"
let getFsiLibraryName = "FSharp.Compiler.Interactive.Settings"
let getDefaultFSharpCoreLocation = Path.Combine(fSharpCompilerLocation, getFSharpCoreLibraryName + ".dll")
let getDefaultFsiLibraryLocation = Path.Combine(fSharpCompilerLocation, getFsiLibraryName + ".dll")
let implementationAssemblyDir = Path.GetDirectoryName(typeof<obj>.Assembly.Location)
let implementationAssemblyDir = Path.GetDirectoryName(typeof<obj>.Assembly.Location) |> ifEmptyUse fSharpCompilerLocation
Copy link
Contributor

@realvictorprm realvictorprm Mar 15, 2020

Choose a reason for hiding this comment

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

Could you add additional explanation for that change?

Copy link
Contributor Author

@amieres amieres Mar 15, 2020

Choose a reason for hiding this comment

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

in Mono-Wasm typeof<obj>.Assembly.Location = "mscorlib.dll" with no path and so Path.GetDirectoryName(typeof<obj>.Assembly.Location) = "" which causes an exception later on. In this case I am using fSharpCompilerLocation which was resolved in the prior point.

Copy link
Contributor

@KevinRansom KevinRansom Mar 16, 2020

Choose a reason for hiding this comment

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

@amieres can you put a repro of something that fails on a GitHub repo somewhere. I certainly want to think about this.

The notion that assemblies are loaded from streams, plays havoc with the a couple of our commonly held invariants.

I think it is a good scenario for us to handle, but I would rather we think about what the solution looks like rather than a few targeted bug fixes. Although in the end it may just turn out to be soluble that way.

I think probably we might want to look at what fable does too since they may encounter and have solved similar issues.

Copy link
Contributor Author

@amieres amieres Mar 18, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@KevinRansom
I created a website where different Assemblies for FCS can be tested:
https://amieres.github.io/WASM.html
After the initial load there is an option to select the set of Assemblies to use
with WASM:

  • Interpreted FCS 25.0 modified
  • Interpreted FCS 33.0
  • Interpreted FCS 34.1
  • Interpreted FCS 34.1 fmwk

The first one is the only that works. It is actually a fork of FCS 25.0 created for this purpose:
https://github.com/fsbolero/FSharp.Compiler.Service

Version 33.0 would work if the 2 issues in this PR were solved (plus a third one, that I don't quite recall right now).

Version 34.0 introduced a new issue by using AssemblyLoadContext which is not supported by Mono WASM because according to the documentation it is not supposed to be NetStandard, but only NetCore. OTOH, the package System.Runtime.Loader is published in Nuget.org as .NETStandard 1.5. I do not know who is right (the documentation or the nuget package), but the end result is that version 34 wont work with WASM until WASM goes to NetCore.

Let me know if this helps you in any way.
When you try to compile the snippets you will see the exceptions produced in each case.


// Use the ValueTuple that is executing with the compiler if it is from System.ValueTuple
// or the System.ValueTuple.dll that sits alongside the compiler. (Note we always ship one with the compiler)
Expand Down
9 changes: 4 additions & 5 deletions src/utils/CompilerLocationUtils.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,10 @@ module internal FSharpEnvironment =
// Check for an app.config setting to redirect the default compiler location
// Like fsharp-compiler-location
try
// We let you set FSHARP_COMPILER_BIN. I've rarely seen this used and its not documented in the install instructions.
match Environment.GetEnvironmentVariable("FSHARP_COMPILER_BIN") with
| result when not (String.IsNullOrWhiteSpace result) -> Some result
|_->
Copy link
Contributor

@realvictorprm realvictorprm Mar 15, 2020

Choose a reason for hiding this comment

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

I would prefer if you intend the following code block to prevent any confusion.

Copy link
Contributor Author

@amieres amieres Mar 15, 2020

Choose a reason for hiding this comment

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

I was following the style already in use there which followed the pattern

match ... with
| Some .. -> ....
| _ ->
...
match ... with
| Some .. -> ....
| _ ->

Although admittedly in the end it broke it by indenting the final if then else

If you still prefer multiple indents I will change that section so its consistent.

// FSharp.Compiler support setting an appKey for compiler location. I've never seen this used.
let result = tryAppConfig "fsharp-compiler-location"
match result with
Expand All @@ -201,11 +205,6 @@ module internal FSharpEnvironment =
match probePoint with
| Some p when safeExists (Path.Combine(p,"FSharp.Core.dll")) -> Some p
| _ ->
// We let you set FSHARP_COMPILER_BIN. I've rarely seen this used and its not documented in the install instructions.
let result = Environment.GetEnvironmentVariable("FSHARP_COMPILER_BIN")
if not (String.IsNullOrEmpty(result)) then
Some result
else
// For the prototype compiler, we can just use the current domain
tryCurrentDomain()
with e -> None
Expand Down