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

ProvidedConstructors for generative types are not passed "this" parameter #218

Closed 7sharp9 closed 6 years ago

7sharp9 commented 6 years ago

Description

ProvidedConstructors for non-static generative types are not passed "this" parameter in the invokeCode function.

Repro steps

create a ProvidedConstructor thats not static and also generative

   ProvidedConstructor([], invokeCode = fun args ->
        let this = args.[0] //IOOB

This will throw an index out of bounds exception, which did not occur in previous versions of the SDK.

Expected behavior

this parameter is supplied for generative types in ProvidedConstructor so it can be used in the quoted expression.

Actual behavior

No parameter provided

Known workarounds

None

Related information

See the code at: https://github.com/fsprojects/FSharp.TypeProviders.SDK/blob/master/src/ProvidedTypes.fs#L8767-L8770

You can see there is a contradiction in the comment and actual implementation. not isCtor should not be included in the expression.

I think the above may be a red herring but the bug still stands, I never get an instance of this.

sergey-tihon commented 6 years ago

Sorry, but I do not understand the logic behind this statement

not isCtor should not be included in the expression.

This part of the condition is for erased types.

The condition is was recently changed by this PR https://github.com/fsprojects/FSharp.TypeProviders.SDK/pull/202/files

and the code looks correct for me...

if not isStatic && (not isCtor || isGenerated) then yield "this"

1) not isStatic && not isCtor - all not static members of erased types, exclusing ctor 2) not isStatic && isGenerated - all not static members of generated types, including ctor

7sharp9 commented 6 years ago

That doesn't make any sense, the comment states:

// "Generated constructors should always pass the instance as the first argument when calling invokeCode!"

I never get an instance of "this" in a ProvidedConstructor invokeCode

sergey-tihon commented 6 years ago

but the comment is covered by condition not isStatic && isGenerated

Where am I wrong?

7sharp9 commented 6 years ago

I never get an instance of "this" in a ProvidedConstructor invokeCode so something is broken

7sharp9 commented 6 years ago

I think this is the thing that broke it: https://github.com/fsprojects/FSharp.TypeProviders.SDK/blame/ea890016ab764b1d6fc54c9ba245ef97106e414c/src/ProvidedTypes.fs#L1022

Quotations.Var(paramNames.[i] would of created a var "this" previously

dsyme commented 6 years ago

@7sharp9 Could you check this again? I've added test https://github.com/fsprojects/FSharp.TypeProviders.SDK/pull/258 to make sure this is available in generative constructors, see https://github.com/fsprojects/FSharp.TypeProviders.SDK/pull/258/files#diff-433ad7897b64ccaf4d011cddea4f6c3dR108

dsyme commented 6 years ago

@7sharp9 Since I've added the test I will close this. Perhaps something has been fixed - the code you referred to on Apr 19 has had one fix since then

7sharp9 commented 5 years ago

I’ll re raise if I encounter anything else, I forgot the use case that was broken anyway.

On Tue, 11 Sep 2018 at 15:40, Don Syme notifications@github.com wrote:

Closed #218 https://github.com/fsprojects/FSharp.TypeProviders.SDK/issues/218.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/fsprojects/FSharp.TypeProviders.SDK/issues/218#event-1838723284, or mute the thread https://github.com/notifications/unsubscribe-auth/AAj7yu4-PKztM7zZhVw-i4z00mI4LfR_ks5uZ8t1gaJpZM4TcDoA .