fsprojects / FSharp.TypeProviders.SDK

The SDK for creating F# type providers
https://fsprojects.github.io/FSharp.TypeProviders.SDK/
MIT License
298 stars 94 forks source link

Generated type broken when switching to this version of providedtypes #22

Closed ReedCopsey closed 10 years ago

ReedCopsey commented 10 years ago

It appears that the merge of PR #16 causes a problem with some generated type providers, due to the inclusion of the change from here:

https://fsharp3sample.codeplex.com/SourceControl/changeset/24356

The removal of ilg.Emit(OpCodes.Ldarg_0) when a BaseConstructorCall exists is causing my type provider to create invalid CLR types.

The fix (for my case) is simple - merely moving this up where it was previously "solves" the issue, though I am unsure if this is due to a problem in the ProvidedTypes code or how I'm creating and using the constructor in the type provider.

My code using this is currently:

let ctor = ProvidedConstructor([])
ctor.BaseConstructorCall <- fun _ -> factoryType.GetConstructors().[0], [Expr.Value(resourcePath)]
ctor.InvokeCode <- fun _ -> <@@ () @@>
outertype.AddMember ctor

Based on the comments, I'm unsure if that change was just problematic, or if I should be doing something different to provide "this" somehow.

mavnn commented 10 years ago

I've not used generated type providers at all yet, so I'm not going to try and make a call on this one!

@7sharp9 / @dsyme / @tpetricek - any comment on whether it's ProvidedTypes that needs the change here?

ovatsus commented 10 years ago

cc @v2m

v2m commented 10 years ago

Return value of BaseConstructorCall function should include 'this' argument (that can be obtained from arguments)

ReedCopsey commented 10 years ago

@desco I don't see how to change the above code to make this work. BaseConstructorCall is just constructor info + args... How would I correct the listed code?

Sent from my phone

On Jun 20, 2014, at 8:50 AM, desco notifications@github.com wrote:

Return value of BaseConstructorCall function should include 'this' argument (that can be obtained from arguments)

— Reply to this email directly or view it on GitHub.

dsyme commented 10 years ago

I think (but am not certain) that @v2m means that the "this" parameter should be included as an additional first parameter in the arguments list here https://github.com/fsprojects/FsXaml/blob/master/src/FsXaml.Wpf.TypeProvider/XamlTypeProvider.fs#L275

v2m commented 10 years ago

Don is right

-----Original Message----- From: "Don Syme" notifications@github.com Sent: ‎6/‎20/‎2014 9:19 AM To: "fsprojects/FSharp.TypeProviders.StarterPack" FSharp.TypeProviders.StarterPack@noreply.github.com Cc: "desco" desco.by@gmail.com Subject: Re: [FSharp.TypeProviders.StarterPack] Generated type broken whenswitching to this version of providedtypes (#22)

I think (but am not certain) that @v2m means that the "this" parameter should be included as an additional first parameter in the arguments list here https://github.com/fsprojects/FsXaml/blob/master/src/FsXaml.Wpf.TypeProvider/XamlTypeProvider.fs#L275 — Reply to this email directly or view it on GitHub.

ReedCopsey commented 10 years ago

Thanks guys - I think I've got it now. It's tough, since (as far as I can tell) most of this is undocumented. Figured there was a good chance it was an issue on my end ;)

7sharp9 commented 10 years ago

For non statics, 'this' is always the first param, I've not had any trouble with generative providers.

On 20 Jun 2014, at 18:32, "Reed Copsey, Jr." notifications@github.com wrote:

Thanks guys - I think I've got it now. It's tough, since (as far as I can tell) most of this is undocumented. Figured there was a good chance it was an issue on my end ;)

— Reply to this email directly or view it on GitHub.