Skip to content
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.

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

Support for Generative Type Providers when targeting .NET Core 2.0 #2406

Closed
dbettin opened this issue Feb 10, 2017 · 52 comments
Closed

Support for Generative Type Providers when targeting .NET Core 2.0 #2406

dbettin opened this issue Feb 10, 2017 · 52 comments

Comments

@dbettin
Copy link

dbettin commented Feb 10, 2017

From #2400:

This also takes a dependency on RefEmit. We’ll need to investigate this further, since it’s currently not on the .NET Core 2.0 roadmap

@dbettin
Copy link
Author

dbettin commented Feb 10, 2017

Also would like to add this comment here from @KevinRansom on #2400:

The problem with generative TypeProviders is that ref emit on coreclr doesn't implement Save(). Ref emit is not actually a part of .Net Standard 2.0 the System.Reflection.Emit package will work with fine with .Net Standard 2.0, it's just those Apis are not part of the .Net Standard set of Apis and so ... there is no urgency to improve it's desktop compatibility. The problem for F# is that generative type providers need to produce a P.E file and since the the Save() API doesn't exist, we need a clever solution to that problem. There are in fact plenty of approaches we could take ... we just haven't settled on one that is the obvious winner.

I cornered Jan Kotas in the corridor the other day and tried to push him to commit to getting us Save, he urged us to seek other solutions ...

So ... we need an alternate solution, regardless until .Net Standard 2.0 we can't do much with type providers given that Type has a private constructor on the coreclr, at least in the reference assemblies it is, the implementation still has a protected constructor. So I suppose we could do type reference magic to get something working.

@dbettin
Copy link
Author

dbettin commented Feb 10, 2017

and from @cartermp

Erasing Type Providers (which are more common) should not require System.Reflection.Emit, but Generative Type Providers will. We'll first need to ensure that with the .NET Standard 2.0 APIs coming along, that we can at least support Erasing Type Providers. If that's the case, then many of the type providers in the wild could conceivably be used, assuming there are no other issues we run into.

For Quotations, it's a somewhat similar story. They don't require System.Reflection.Emit, but they are needed for fully-fledged quotation evaluators/compilers, and are needed if they are used as input to a Generative Type Provider.

What I think may be likely is that we'll have support for the "95% case" in these areas, but we could still be blocking key libraries and library authors if they are building full-on quotation compilers and Generative Type Providers.

All of the above is assuming that we don't do any work in those areas ourselves. @KevinRansom can speak more towards the feasibility of that, but given the amount of work we're doing right now, I don't find it likely that rewrites of those areas of TPs and Quotations will come from someone in Redmond between now and .NET Standard/.NET Core 2.0.

@sergey-tihon
Copy link
Contributor

Do we know why Save() from System.Reflection.Emit is not part of .NET Standard 2.0? Explanation page or some discussion that may be linked? // cc @terrajobst

What alternatives we have if Save() will not be part of .NET Standard 2.0?

I think that I have a TP that should be generative - SwaggerProvider.
In we could keep this TP generative and port it to .NET Standard 2.0, it will allow mobile developers to consume generated types in C# / Xamarin.Forms projects. If generative TP is not supported, I will have to convert this TP to erasing and discontinue C# support. 😢

@matthid
Copy link
Contributor

matthid commented Mar 13, 2017

The issue is tracked here
https://github.com/dotnet/corefx/issues/4491
and there is a linked issue created by Kevin explaining in-depth what this means for F#...

@sergey-tihon
Copy link
Contributor

Ok, found issue created by Kevin - https://github.com/dotnet/coreclr/issues/1709

@7sharp9
Copy link
Contributor

7sharp9 commented Mar 21, 2017

Duplicating my comments from the other issue this is linked from:

One of the big things is that generative type providers service the wider ecosystem not just F#, they are the most important aspect in that respect.

Unchecked quotations for generative providers is also something that sorely needed to avoid having to reflectively call quotation builders to build the type provider.

@zpodlovics
Copy link

How about the successor(?) of IKVM.Reflection.Emit from Jeroen Frijters as his name already mentioned in the System.Reflection.Emit.AssemblyBuilder.Save corefx issue [1]? A preview release are available from NuGet.

"Managed.Reflection is a fully managed replacement of System.Reflection and System.Reflection.Emit. Unlike System.Reflection, it is not tied to the runtime it is running on.

It targets .NET Standard 1.3, which means it runs on .NET Core 1.0 and .NET Framework 4.6 and up."

Repository:
https://github.com/jfrijters/Managed.Reflection

[1] https://github.com/dotnet/corefx/issues/4491#issuecomment-189756092

@matthid
Copy link
Contributor

matthid commented Mar 21, 2017

@zpodlovics considering the comments I think (after discussing with the maintainers of course) such a PR making it work would be welcome... Its just nothing they have time for right now

@7sharp9
Copy link
Contributor

7sharp9 commented Mar 22, 2017

Ive partially patched reflection stuff with IKVM.Reflection when I was working on an iOS feature for xamarin so it could be worth investigating.

@7sharp9
Copy link
Contributor

7sharp9 commented Apr 13, 2017

@dsyme / etc Would a PR that used a dotnetcore version of ilvm reflection be accepted here?

@sergey-tihon
Copy link
Contributor

@7sharp9 You mean Managed.Reflection?

@Thorium
Copy link
Contributor

Thorium commented May 14, 2017

Could Erasing Type Providers already work (in theory)? IDE (e.g. Visual Studio) is run on non-dotnet-core mode and the compiled code should be run on dotnet core / standard?

@cartermp
Copy link
Contributor

cartermp commented May 14, 2017

@Thorium Yes, in theory Erasing TPs should work on .NET Core 2.0, and thus could target .NET Standard 2.0. That assessment still has to be done. Just last week, support for .NET Core 2.0 has been merged as in-band for the .NET Core SDK, so the assessment and proper work here can now be done.

@7sharp9
Copy link
Contributor

7sharp9 commented May 20, 2017

@sergey-tihon Yeas I meant Managed.Reflection, with that project hanging in the balance Im in two minds whether to use it as a replacement.

@7sharp9
Copy link
Contributor

7sharp9 commented May 20, 2017

To get generatives working do we just have to re-write providedtypes.fs to use an alternative @cartermp / @sergey-tihon

@7sharp9
Copy link
Contributor

7sharp9 commented May 20, 2017

The problem with generative TypeProviders is that ref emit on coreclr doesn't implement Save().
Save is used unless the FX_NO_LOCAL_FILESYSTEM define is there.

@realvictorprm
Copy link
Contributor

Well, can I help here somehow?

@dsyme
Copy link
Contributor

dsyme commented May 26, 2017

I believe we are blocked on having the compiler target .NET Core 2.0

@gsomix
Copy link
Contributor

gsomix commented Aug 8, 2017

What's current status of the issue? There are too many issues related to TP support in .Net Core/Standard, not easy to pick needed info.

Thanks!

@cartermp
Copy link
Contributor

cartermp commented Aug 8, 2017

No update on status at the moment. We're looking at this once .NET Core 2.0 is released. #3303 is a workaround for now.

@matthid
Copy link
Contributor

matthid commented Aug 30, 2017

/cc @alfonsogarciacaro

@alfonsogarciacaro
Copy link
Contributor

At some point there was quotation support in Fable but I removed it because it was flaky and I didn't find many practical uses for it. About System.Type and reflection in general, there's some limited support in Fable.

However, I don't think this is an issue. Fable 0.7 was compatible with TPs (I'm only talking about erasure TPs here) and FCS already gave you the erasure code in the AST, so I didn't have to worry about quotations, etc (FCS did crash sometimes when accessing the AST generated by TPs but most of the cases were fixed).

The main issue with TPs and Fable was that most of the TPs generated code that used BCL APIs not supported by Fable (IO, networking). That's the reason Fable and TPs haven't had a nice story together so far.

Example of a type provider that was compatible with Fable at the time.

@dsyme
Copy link
Contributor

dsyme commented Aug 30, 2017

However, I don't think this is an issue. Fable 0.7 was compatible with TPs

Hmmm... that's probably with the F# compiler running as a .NET Core or .NET Framework component. I think the Fable-compiled FSharp.CompilerService was compiled without TP support, cc @ncave

@alfonsogarciacaro
Copy link
Contributor

Yeah, sorry. I was talking about running Fable on .NET. When compiling FCS to JS I'm not sure, maybe it makes sense to have a specific mechanism for Fable that emits JS directly, similar to how the Emit attribute works.

@matthid
Copy link
Contributor

matthid commented Aug 30, 2017

I'm more interested if we could fundamentally improve the situation with different APIs. For example does fable even have to know about type providers or could that be an implementation detail?
Also should TP resolve/introduce their own dependencies or use an API provided by the compiler for that?

@ForNeVeR
Copy link
Contributor

ForNeVeR commented Oct 7, 2017

@dsyme @7sharp9 @matthid I'd like to discuss the current difficulties in generative type provider support for .NET Core.

I am still new in the land of type providers, so I could ask really silly things, but could you please give me the directions? Please correct me if I'm saying anything wrong.

We've done a small research with @Dolfik1 on that matter, and have came to the following conclusions.

  1. The main (or at least the first) issue is the usage of AssemblyBuilderAccess.Save in the type provider template:
    • This isn't possible on .NET Core (AssemblyBuilderAccess.Save doesn't even exists).
    • There's the supporting code under the FX_NO_LOCAL_FILESYSTEM flag, but I highly doubt it does the right thing.
    • Compiler really need the bytes from the generated assembly (it will load them and use the code from that assembly).
  2. That means that we need to somehow generate the bytes from the dynamic assembly generated by the type provider, no matter what.
  3. We cannot change the public API of ProvidedTypes.fs (because that will break compatibility), but we could change its' implementation if we really need to.

If these conclusions are correct (and saving the generated assembly is really the biggest problem), then we could really come up with some hacky (hopefully not very intrusive) solution to make it work. Currently I think about using Mono.Reflection to parse the dynamic assembly generated by the type provider (✓ have verified that it works on .NET Core 2.0) and then Mono.Cecil to reconstruct and save the assembly into byte array.

I have some questions regarding that.

  1. Are my conclusions right? Or is there something I haven't noticed yet?
  2. Is that okay to change internals of ProvidedTypes.fs without touching the public API?
  3. Will it work in the long term? Or do we have some bigger plans for the generative type providers (migrating them to the new API with or without compat. layer, obsoleting them entirely etc)?
  4. Could such a (hacky?) contribution into FSharp.TypeProviders.SDK be accepted?

@Dolfik1
Copy link

Dolfik1 commented Oct 7, 2017

I found the System.Reflection.Metadata library, it seems that we can generate assemblies using this library only.

@abelbraaksma
Copy link
Contributor

abelbraaksma commented Oct 8, 2017

While I can't answer your questions (@dsyme is much better suited for that) i doubt a dependency on third party libraries would be accepted, as that would mean that FSharp.Core will get a dependency outside of BCL. Though I'm glad someone is looking into this, it's a much needed requirement ;).

@ForNeVeR
Copy link
Contributor

ForNeVeR commented Oct 9, 2017

@abelbraaksma if that's the biggest issue, we could solve that in one way or another.

One option is to either port or rewrite in F# the corresponding parts of Mono.Cecil/Reflection and/or System.Reflection.Metadata.

But, if I'm following Don correctly, in this comment he suggests us to use the ILReader / ILWriter abstractions from the compiler code. Probably that's the best option for our situation?

PS: please note that we're mostly talking about Type Provider SDK and F# compiler code, and not the FSharp.Core. Any provider-related changes will reside in either the compiler, the TP SDK, or both. I can't see how they could leak to FSharp.Core.

@realvictorprm
Copy link
Contributor

@ForNeVeR so far I see the situation we should pick everything existing up. Still though, dependencies are only being added to the F # compiler, not F# Core (how did this come up?).

Your conclusion make sense to me. We should focus on fixing type providers...

I hope to see a response soon from @dsyme (as soon as he can spend time for our questions :) )

@dsyme
Copy link
Contributor

dsyme commented Oct 9, 2017

@ForNeVeR Yes, your analysis is correct

But, if I'm following Don correctly, in this comment he suggests us to use the ILReader / ILWriter abstractions from the compiler code. Probably that's the best option for our situation?

Yes, I'm wondering if we should just go down that route. There are pros and cons but it seems like a good option to be free of dependencies.

...as that would mean that FSharp.Core will get a dependency outside of BCL....

That's not the case: it is the FSharp.TypeProviders.SDK bits we are talking about, not FSharp.Core

@ForNeVeR
Copy link
Contributor

ForNeVeR commented Oct 9, 2017

Alright, then. We have the following plan.

@Dolfik1 is now implementing a prototype of a Assembly -> byte[] function that'll be able to turn any Assembly into a byte array using common reflection and System.Reflection.Metadata package. I'll work with him to turn that prototype into something useful for type providers.

I propose we first try to convert any simple type provider with our prototype. If that will work (and we won't encounter any blockers), then we could proceed with reimplementation of the prototype in F# with ILWriter or whatever we'll consider useful and @dsyme will approve for use in the Type Providers SDK.

Is that okay?

@abelbraaksma
Copy link
Contributor

abelbraaksma commented Oct 9, 2017

not F# Core (how did this come up?).

My bad, sorry for the fog. As @dsyme stated:

That's not the case: it is the FSharp.TypeProviders.SDK bits we are talking about, not FSharp.Core

Which moots my comment ;) (and luckily so).

@KevinRansom
Copy link
Member

@Dolfik1 can you keep me in the loop on the results of your prototype ... it is an interesting proposal.

@Dolfik1
Copy link

Dolfik1 commented Oct 9, 2017

@KevinRansom Of course. At the moment we have been able to generate a basic assembly, it is not yet valid, but at least dotPeek shows that all the metadata and the IL code are in place.
Right now, me and @ForNeVeR trying to fix validation errors.

default

@dsyme
Copy link
Contributor

dsyme commented Oct 9, 2017

@ForNeVeR I'm not sure it's necessary to do a prototype via System.Reflection.Metadata. I think we should just add the appropriate IL generator and binary writer to ProvidedTypes.fs https://github.com/fsprojects/FSharp.TypeProviders.SDK/tree/master/src

@dsyme
Copy link
Contributor

dsyme commented Oct 10, 2017

@KevinRansom asked offline

"We have a compiler, why don’t we use it to emit the il? Is the issue the transformation between refemit types and il?"

Yeah it’s a good question.

First the usual background…

Historically generative type providers were only introduced to allow other code generators (e.g. C# compiler, or svcgen.exe etc) to be hosted as TPs, with a thin erasing wrapper. In that scenario “some other tool” did the codegen.

Over time the community (I think it was @v2m acting in community mode initially) added an ability to take this way of specifying erasing type providers (the TP SDK) and repurpose it as a way to author generative TPs, basically the ProvidedAssembly type and the Generate method in the TPSDK. That uses reflection emit but ended up duplicating some fragments of compiler logic. (It also has some oddities like allowing DateTime and TimeSpan in the emittable Value constants rather than requiring the user to generate the appropriate quotation for the code to resurrect these.)

On the positive side, the code was outside our repo, and we didn’t need to know anything about it - if people wanted a new feature or to expand the range of quotations allowed then they just sent a simple PR, and all the QA was on the TP SDK. On the negative side it used Reflection Emit so can’t do cross-targeting. But the approach basically worked.

So to your question – yes, in theory it would be possible to abandon this approach and instead have the compiler give each TP a utility method in the TypeProviderConfig, e.g.

member GenerateAssembly: (ProvidedAssembly -> bytes) 

which would internally to the compiler be implemented as either

GenerateAssembly == ProvidedAssembly--> Abstract IL --> ilwrite.fs --> bytes

Or

GenerateAssembly == ProvidedAssembly--> TAST --> Abstract IL --> ilwrite.fs --> bytes

Either way this would not use reflection emit and would reuse the ilwrite in the compiler.

However, this still means implementing either ProvidedAssembly--> TAST or ProvidedAssembly--> Abstract IL. That’s largely new logic either way.

Now, for technical reasons ProvidedAssembly--> TAST is hard. Not impossible, but it’s hard. Some fragments are already present. It can only be done inside the compiler for sure and would make use of the “full” compiler stack – IlxGen, state machine generation, optimizer etc. etc. So reusing the compiler in its full glory is not impossible, but hard. (As an aside, somehow it also feels against the original goals of type providers, which was to leverage external code generators in a fairly lightweight way - I'm not saying we achieved that, but it's on my mind)

However, in reality the code generated by TPs tends to be very simple, and TPs tend to call into runtime library code for anything complex. That’s why @v2m's “do assembly generation in the TP SDK” approach worked well enough. After all, the generated code doesn’t really require full optimization, nor tailcalls, nor any magic done in IlxGen.fs. And with the TP SDK the community could iterate fast. And there are some advantages to disincentivizing people from generating complex code using TPs anyway - e.g. there's no debug story for that code.

Anyway, all these are reasons as to why redoing some compilation logic in the TP SDK has historically made sense.

I’ll think it over more, but it still feels to me that the most simple path to unblock is to prototype the Assembly --> Abstract IL --> copy-of-ilwrite.fs in the TP SDK – it feels like just a day’s work to be honest. It’s much easier to get this stuff up and running outside the compiler (but writing it in such a way that we can bring it in later). If we decide to later reuse what’s in the compiler then we can do that for both ilread.fs and ilwrite.fs together.

p.s. One thing that could force us to go down the Assembly --> TAST route is if generative TPs ever wanted to refer to types in the assembly actually being compiled by the F# compiler, e.g. see fsharp/fslang-suggestions#450 and fsharp/fslang-design#125

@ForNeVeR
Copy link
Contributor

@dsyme thanks so much for your comment. It's both interesting and valuable.

Regarding to the "prototype" based on System.Reflection.Metadata: @Dolfik1 just was too enthusiastic about that approach, so he's already done a large part of work, and I feel bad about abandoning it. We could eventually even make this prototype an externally-useful (e.g. not tied to F# type providers) artifact of the research. Also, it seems that we're almost done with the prototype, and it could give us an insight on any further issues we'll encounter while integrating binary writer with the Type Provider SDK.

I agree with your rationale, and we'll try to follow that path soon (probably in a week or two). If anybody wants to begin integrating ilwriter.fs with the Type Provider SDK right now — you're welcome, we'll try to provide any possible help then. If not — then I hope we'll eventually do it.

@AviAvni
Copy link
Contributor

AviAvni commented Oct 10, 2017

@dsyme please also consider fsharp/fslang-suggestions#154
Also maybe it's a silly suggestion but maybe we can make the GenerateAssembly little shorter
And start from TAST it means that type provider produce TAST instead of ProvidedAssembly

@dsyme
Copy link
Contributor

dsyme commented Oct 10, 2017

@ForNeVeR I'm trying to understand what the role of the AssemblyGenerator would be. It seems it assumes the assembly coming in has code backed by IL, e.g. here. But in that case the Assembly would already normally have corresponding bytes in its existing on-disk image.

For generative F# type providers authored using the TPSDK, the situation is that we have an artificial Assembly (i.e. a ProvidedAssembly from the TPSDK) with code given by F# quotations (e.g. InvokerCode in ProvidedMethod). It is this data that needs to be used to generate the assembly.

@dsyme
Copy link
Contributor

dsyme commented Oct 10, 2017

@AviAvni

@dsyme please also consider fsharp/fslang-suggestions#154

Also maybe it's a silly suggestion but maybe we can make the GenerateAssembly little shorter. And start from TAST it means that type provider produce TAST instead of ProvidedAssembly

I see what you're asking, but the TAST is not a public data structure.

Currently, generative type providers authored using the TPSDK specify the code contents of a generated assembly using quotations, e.g. InvokerCode in ProvidedMethod, in the context of an overall ProvidedAssembly. That's the input IR we're dealing with at this point.

@ForNeVeR
Copy link
Contributor

@dsyme at the same time, to me it looked like a generative TP would emit the code into the dynamically generated assembly: https://github.com/fsprojects/FSharp.TypeProviders.SDK/blob/36c892674c276265f2c028b8a2e36a99445acb3e/src/ProvidedTypes.fs#L7495-L7496

...and then get its' bytes using the standard assembly.Save call: https://github.com/fsprojects/FSharp.TypeProviders.SDK/blob/36c892674c276265f2c028b8a2e36a99445acb3e/src/ProvidedTypes.fs#L7856

Initially I thought that's the essential part of type provider functioning, but now I'm just a bit confused.

@dsyme
Copy link
Contributor

dsyme commented Oct 10, 2017

@ForNeVeR Yes, that's correct - though at the start of the process the code for each method is an F# quotation - see that same file

@dsyme
Copy link
Contributor

dsyme commented Oct 12, 2017

@ForNeVeR The TPSDK has now been updated with support for cross-targeting generative type providers by integrating the binary writer.

You should now be able to build generative type providers for .NET Core though that has not yet been specifically tested (and might require an updated F# compiler for .NET Core).

@dsyme
Copy link
Contributor

dsyme commented Nov 2, 2017

I'm closing this as the TPSDK has now been updated to support generative type providers when targeting .NET Core

If targeting .NET Standard or .NET Core and using type providers you still need to apply the workaround specified here: #3303. (That is a blunt workaround for issue #3736 which needs to be resolved (and type providers re-published) before type providers can be created which will load into .NET Core-based tooling)

@dsyme dsyme closed this as completed Nov 2, 2017
@dsyme dsyme changed the title Full Support for Type Providers on .NET Core 2.0 Support for Generative Type Providers on .NET Core 2.0 Nov 2, 2017
@dsyme dsyme changed the title Support for Generative Type Providers on .NET Core 2.0 Support for Generative Type Providers when targeting .NET Core 2.0 Nov 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests