Skip to content

Conversation

@dsyme
Copy link
Contributor

@dsyme dsyme commented Nov 21, 2016

The compiler has some logic to search for known types amongst available DLLs.

With this PR we get to enable several tests that uses F# Interactive on .NET Core and more tests for the F# compiler on .NET Core

Some of the known-type-search logic is OK, but swathes of it are not so good, e.g. this stuff

This improves the search by

  1. just having one function to do the search

  2. moving many "known types" out of Abstract IL (ILGlobals) and into the F# Compiler TcGlobals. THe advantages of this is that getting started with ILGlobals is much simpler if the set of referenced types is simpler - in particular if the small set of types known to Abstract IL are all always in the primary assembly (Int32, String, Array etc.).

This is particularly important for .NET Core. I don't trust the current code to correctly find some of the more obscure types for .NET Core, since the searching was based around various assumptions for Profile 7, 78, 259.

Overall this removes a fair chunk of fragile code from the compiler.

Separately this refactors TcGlobals to be a class instead of a record (which requires fewer lines of code)

src/absil/il.fs Outdated
static member IsSomePrimaryAssembly n =
n = PrimaryAssembly.Mscorlib.Name ||
n = PrimaryAssembly.DotNetCore.Name ||
n = PrimaryAssembly.PrivateCoreLib.Name
Copy link
Contributor

Choose a reason for hiding this comment

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

No ... please not this ... its bad enough that the compiler knows about mscorlib and System.Runtime ... let's not do propagate that error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@KevinRansom Will work on it

@dsyme
Copy link
Contributor Author

dsyme commented Nov 22, 2016

@KevinRansom I have removed the knowledge of Private.CoreLib. F# Interactive still runs correctly. I've also enabled lots more testing for .NET Core F# Compiler and F# Interactive in tests.fs

The .NET Core compiler and F# Interactive still have some problems on a number of the FSharpSuite tests. Most of the FSC_BASIC and FSI_BASIC tests that have not been enabled fail in one way or another, either failing to compile (usually the test needs to be adjusted) or failing to run through F# Interactive (this seems to indicate some limitation or difference in dynamic code generation on .NET Core).

@dsyme
Copy link
Contributor Author

dsyme commented Nov 22, 2016

@KevinRansom This is ready to go in (assuming the CI stays green).

I think it's important to get this in to lock down the status of the F# compiler and F# Interactive on .NET Core, at least for the FSharpSuite tests we have under CI (in addition to the FSharp.Core.Unittests, which we've always had under CI).

@dsyme
Copy link
Contributor Author

dsyme commented Nov 22, 2016

@dotnet-bot test this please

@dsyme dsyme closed this Nov 22, 2016
@dsyme dsyme reopened this Nov 22, 2016
@dsyme
Copy link
Contributor Author

dsyme commented Nov 22, 2016

@KevinRansom I'll merge this now. I had to disable the added CoreCLR testing since it hit a testing timeout in CI of some kind, but it runs successfully by hand. I'll work that out tomorrow.

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.

3 participants