Skip to content

Conversation

@dsyme
Copy link
Contributor

@dsyme dsyme commented May 31, 2015

I've been looking for what we can do to help with type-provider multi-targeting as described in #23. This is a major issue for F# type providers and, as NETCore-on-Linux comes around, we will really need to be looking at this closely. Anything we can do in F# 4.0 to help with this will be important.

As mentioned in #23 (comment), the ReferencedAssemblies field of the type provider config is missing entries for basic assemblies such as FSharp.Core. This is a mistake in the implementation of F#, and is bad, as this data is crucial to help give better multi-targeting type providers.

The underlying bug is that the ReferencedAssemblies property only includes the "non-framework" assemblies, and not "framework" assemblies such as FSharp.Core and mscorlib. (The framework v. non-frameworks distinction being referred to here is a somewhat artificial distinction made in the F# compiler to share resources between projects in the IDE in some situations

This corrects this problem by adding the framework references to the ReferencedAssemblies property inn the type provider config.

Unfortunately this fix will only be available to Visual Studio 2015+ and other implementations of F# 4.0. I'm uncertain what should be done to allow simply multi-targeting in other situations, but this does at least correct the ReferencedAssemblies property to be as intended.

More testing for this property and the rest of the values in the type provider config should be added as there doesn't seem to be much there.

I'll leave it to @latkin and @KevinRansom to decide if this makes the cut for Visual Studio 2015 and F# 4.0.

@latkin
Copy link
Contributor

latkin commented Jun 1, 2015

I see that this adds additional entries to ReferencedAssemblies, but how does one use those to prevent the problems described in #23? Or is this just meant as a generally good change to make, without purporting to completely solve any particular problems?

Minor bug - I am seeing duplicate entries now in ReferencedAssemblies when building a small repro TP.

Before:

C:\Users\latkin\Desktop\ConsoleApplication1\TPRepro\bin\Debug\TPRepro.dll

With this change (note duplicate FSharp.Core and mscorlib):

C:\Program Files (x86)\Reference Assemblies\Microsoft\FSharp\.NETFramework\v4.0\4.3.1.0\FSharp.Core.dll
C:\Program Files (x86)\Reference Assemblies\Microsoft\FSharp\.NETFramework\v4.0\4.3.1.0\FSharp.Core.dll
C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.5\mscorlib.dll
C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.5\mscorlib.dll
C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.5\System.Core.dll
C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.5\System.dll
C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.5\System.Numerics.dll
C:\Users\latkin\Desktop\ConsoleApplication1\TPRepro\bin\Debug\TPRepro.dll

@dsyme
Copy link
Contributor Author

dsyme commented Jun 2, 2015

I wasn't expecting the duplicate entries. I'll take a look.

ReferencedAssemblies was added to support the scenario of having a type provider know exactly what platform and binaries are being targeted, and adjusting the target code/quotation generation to match those assemblies. So its an enabling change that makes ReferencedAssemblies meet its specification

But first I need to understand why duplicates are now appearing (and double check that mscorlib and FSharp.Core are indeed missing without this fix).

@latkin latkin removed the bar-check label Jun 2, 2015
@KevinRansom
Copy link
Contributor

We need the list of assemblies that the compiler knows about. So that the type provider author has a chance at limiting the types used in code/quotation generation to the set specified to the compiler. Today that is just not possible.

It is not really the complete solution, however, since the Type Provider author still has to Load the assembly and get the types, with all of the binding difficulties that causes, and on coreclr it is going to be so much worse. I think we can either take this or not take this. Although I doubt if the ReferencedAssemblies property is the long term solution.

@latkin
Copy link
Contributor

latkin commented Aug 4, 2015

This still has the known issue of duplicate list entries, do you have an ETA for a fix?

@dsyme
Copy link
Contributor Author

dsyme commented Aug 4, 2015

Thanks for the prompt - looking into it.

@latkin
Copy link
Contributor

latkin commented Aug 13, 2015

Coming back to this -- the list of assemblies is useful, but in and of itself isn't a big problem today as I understand it. As a consumer if I reference a TP, but fail to reference some assembly it needs, the tools give me a nice friendly error "a reference to Foo.dll is required".

The real problem is reconciling the versions of consumer-referenced assemblies with those of TP-referenced assemblies. If the TP has access only to the raw file path, then is the expectation that they do a ReflectionOnlyLoadFrom or the like, in order to extract the version?

Then once the version is in hand, how exactly do they act on it to "adjust the target code/quotation generation"? Say I authored my TP to depend on Foo.dll 1.0. A year later a consumer references my TP and Foo.dll 2.0. What mechanism is available to TP authors to do codegen/quotation adjustments such that they are future-proof against as-yet-unreleased versions of their dependencies? Is that doc'ed somewhere?

@KevinRansom
Copy link
Contributor

@dsyme, if I understand this PR correctly you are just looking to fix the list of assemblies passed to providers to include a more accurate list of assemblies. Which would make this a bug fix, additional parts of making it easier to write a multi-targeting type provider is still to be designed.

Is that the intent of this PR?

@KevinRansom KevinRansom closed this Oct 1, 2015
dsyme and others added 17 commits December 31, 2015 11:10
*copied from fsharp/fsharp#533

There may be a missing length check on exists, as without hte length check the cost of a comparison and a `call` is incurred.

Adding the simple len > 0 check before the loop appears (in the basic cases I have tested) to near triple performance.
```
let f arr = 
    Array.exists (fun e -> e = true) arr

for i in 0..60000000 do f ([||]:bool []) ;; // 1.8 seconds

let f2 arr = 
    if Array.length arr > 0 then
        Array.exists (fun e -> e = true) arr
    else 
        false

for i in 0..60000000 do f2 ([||]:bool []) ;; // 0.430 seconds
```

Note: this is something I have found an isolated instance of, it may be appropriate to ensure that such checks (if they are required, I can only speak for my use case) are consistant accross other modules, such as Seq\List.
[Jenkins] Added NUnit Test Results/Running Smokes for now
removed the grouping by file because it's not used
latkin and others added 23 commits February 28, 2016 16:14
Fix Out/Error streams chunking in Fsi tool window
Implement allPairs on List, Seq and Array modules
Microbuild support for F# codebase
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.