dotnet / fsharp

The F# compiler, F# core library, F# language service, and F# tooling integration for Visual Studio
https://dotnet.microsoft.com/languages/fsharp
MIT License
3.94k stars 789 forks source link

In a generative type provider adding an Intrinsic type extension causes a compile time exception #49

Open KevinRansom opened 9 years ago

KevinRansom commented 9 years ago

Opened by corado at codeplex

Given a generative type provider. Take the example from the sample pack: GeneratedTypeProvider And the following usage of it:

module FSharpLib

type Test = Samples.ShareInfo.TPTest.TPTestType

type Test with
    member x.Extension = 42

let t = Test()
printfn "%i" (t.F1(1))

The type Test should be Intrinsically extended with the Extension property as the type is defined in the same file but this will fail to build with the error:

/SampleProviders/GeneratedTypeProvider/ConsoleApplication2: Error FS0192: internal error: FindTypeDefBuilder: Test not found (FS0192) (ConsoleApplication2)

comments ReedCopsey wrote Apr 25, 2014 at 12:03 PM [x] Using VS 2013, I get the same issue with my XAML Type provider. However, the error message is slightly different. Given:

 type App = XAML<"App.xaml">

 type App with
member self.Foo = 42

[<STAThread>]
 [<EntryPoint>]
 let main argv = 
let app = App()
printfn "%d" app.Foo
app.CreateRoot().Run()

I get: Error 3 The field, constructor or member 'Foo' is not defined

7sharp9 wrote Apr 25, 2014 at 1:12 PM [x] Is that error given on build?

ReedCopsey wrote Apr 25, 2014 at 1:50 PM [x] Yes. I get that at compile time.

chkn commented 9 years ago

Any word on this? An intrinsic extension to a provided type still gives an internal compiler error. It looks like it comes down to TypeDefsBuilder.FindTypeDefBuilder where the tdefs collection doesn't contain the type definition. I'd try to fix this myself, but I'm a bit lost

chkn commented 9 years ago

I did a little more poking around. It looks like the provided types are only merged into the compiled module at the very end, after the sources are already compiled (around https://github.com/Microsoft/visualfsharp/blob/fsharp4/src/fsharp/fsc.fs#L1628). This means the provided type defs are not yet around when the source code is being compiled, so we get the error when we try to look one up to extend it.

Now this is my first time working with the code base, so I might be shooting in the dark here, but it seems like this can be fixed by building the ILTypeDefs for the generated types and adding them to the TypeDefsBuilder before the main compilation so they are around to be extended. I cooked up a preliminary patch that seems to work for my little test case (I'm working on Mac so this is forked from the open repo): https://github.com/chkn/fsharp/commit/type-extensions-for-provided-types

Any tips or guidance would be awesome :)

chkn commented 9 years ago

Current status of the patch:

latkin commented 9 years ago

I'm removing the VS 2015 milestone on this guy as I don't think we will realistically have a fully-tested fix in time, assuming it even gets shiproom approval (which is unlikely at this point for VS 2015).

latkin commented 9 years ago

@chkn were does this stand? Do you think you will have a fix in the short-term?

chkn commented 9 years ago

@latkin Apologies for my delayed response..

I've squashed the original patch and rebased it on the tip of fsharp/fsharp. However the last 2 points of my last comment still stand. The fsi issue is probably related to this snippet from https://msdn.microsoft.com/en-us/library/hh361034.aspx:

When you compile a .dll or .exe file, the backing .dll file for generated types is statically linked into the resulting assembly. This link is created by copying the Intermediate Language (IL) type definitions and any managed resources from the backing assembly into the final assembly. When you use F# Interactive, the backing .dll file isn't copied and is instead loaded directly into the F# Interactive process.

This patch basically generates the ILTypeDefs used by that static linking step earlier in the process. But fsi doesn't do that linking at all, and I'm not sure what's the "right" thing to do about that.

For the type checker issue, that code just goes right over my head. I'll stare at it some more if I find some spare time.

So TL;DR, the answer to your question is "no" ... Unless somebody can help :)

7sharp9 commented 9 years ago

This would be really nice to have, intrinsic type extensions to generative types makes them magnitudes more appealing as well as working with other code bases easier. i.e. C# API that generate types such as Xamarin.Forms, monotouch, Android code generation etc.

7sharp9 commented 8 years ago

@dsyme I can confirm that this is working for generative providers in both the raw compiler (fsc.exe) and IDE integration via cherry-picked commits to FCS. All that remains is the patching of fsi to work in the same way.

forki commented 8 years ago

There is fix suggested at https://github.com/Microsoft/visualfsharp/pull/882

7sharp9 commented 8 years ago

Yes #882 is a PR for this, Ive been working with @chkn on this.

dsyme commented 2 years ago

Reopening as the PR was not merged. Note this scenario is simply not supported today

7sharp9 commented 2 years ago

From memory, from a non-fsi point of view, there was a working prototype, fsi was broken for the intrinsic extension, but the same error was reported before the patch, so no new errors in that instance.

I think work mainly stopped because no one was able to dig into the fsi issues, or understand what that part was doing.

7sharp9 commented 2 years ago

Oh, the other big issue is overloads were using the depreciated style:

type t = BasicProvider.Generative.MyType

type t with 
    member x.Foo = 42
    override x.Rotate _ = ()

This would result in the following warning:

[FS0060] Override implementations in augmentations are now deprecated. Override implementations should be given as part of the initial declaration of a type.

There's no way around that unless the spec for overrides are changed or some type of new syntax.

chkn commented 2 years ago

That matches my recollection as well. I believe this comment is still an accurate summary of the working prototype status.