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

Add CustomAttribute implementation and open up API slightly for Falanx usage. #291

Closed 7sharp9 closed 4 years ago

7sharp9 commented 5 years ago

This is all the changes that we had to make to Falanx to consume the TP SDK.

The test I added currently fails due to the way the TP SDK wraps imported types compared to Falanx.

Falanx does not use the Assembly reader and TargetContext which means the call:

let attribute = data.Constructor.Invoke(arguments) :?> Attribute

Works in Falanx but fails in the test here. I opened this PR as there was interest in getting CustomAttribute support in the TP sdk although the way we have done this is at odds with the TargetContext mechanism. I opened a question: #290 To see if there would be a way around this, or if there might be a reliable way to get access to a RunTimeType.

Myself and @drvink also discussed this, so I though this might be a good place to discuss with the relevant context of the PR.

drvink commented 5 years ago

if there might be a reliable way to get access to a RunTimeType

For clarification, this is in reference to System.RuntimeType, i.e. what is usually produced by typeof<t> or x.GetType() (and then upcast to System.Type[1] before being returned to the user). The issue is difficult to summarize, but effectively, the .NET reflection machinery permits (by "design") users to extend it in ways that allow returning ersatz reflection data, and the TP SDK takes advantage of this; unfortunately, the implementation is incomplete[2], and thus reflection on TP-generated types produces buggy, incomplete reflection information. One example issue:

1) UnderlyingSystemType is abstract: https://referencesource.microsoft.com/#mscorlib/system/type.cs,9100e9994ceb896c 2) TypeDelegator invokes UnderlyingSystemType on whatever the user has set typeImpl to: https://github.com/Microsoft/referencesource/blob/4251daa76e0aae7330139978648fc04f5c7b8ccb/mscorlib/system/reflection/typedelegator.cs#L240 3) ProvidedTypes sets TargetTypeDefinition.typeImpl (inherited from TypeDelegator) to an instance of itself: https://github.com/fsprojects/FSharp.TypeProviders.SDK/blob/67cd11d0206a75d3bd399537ad7c7c27fb35979d/src/ProvidedTypes.fs#L7696 4) ProvidedTypes overrides UnderlyingSystemType, upcasting this to System.Type, which means when you ask for an UnderlyingSystemType, you get StackOverflowException: https://github.com/fsprojects/FSharp.TypeProviders.SDK/blob/67cd11d0206a75d3bd399537ad7c7c27fb35979d/src/ProvidedTypes.fs#L7829

I believe @7sharp9 was also unable to use Enum.ToObject on an enum generated via a TP, though I don't have a repro handy at the moment.

[1] ...which is actually an abstract base class--what most people think of as values of System.Type are usually upcast instances of System.RuntimeType, the definition of which is actually internal to the runtime. [2] This seems to be intentional, or at least is acknowledged in this comment.

7sharp9 commented 5 years ago

@drvink This is the bit you were referring to, I patched this by trying to recreate a type using the assembly and full name: https://github.com/fsprojects/FSharp.TypeProviders.SDK/pull/291/files#diff-7c540ae6a67f14c48d0403e7b021fc86R340

The thing that fails after that is the reflection.Invoke call on the attributes constructor, which is here: https://github.com/fsprojects/FSharp.TypeProviders.SDK/pull/291/files#diff-7c540ae6a67f14c48d0403e7b021fc86R288

7sharp9 commented 5 years ago

@drvink / @dsyme I mean it is possible to reverse map them with:

TargetContext.ConvertTargetTypeToSource

But the issue is that none of the ProvidedTypes has the TargetContext only TypeProviderForNamespaces has that context.

7sharp9 commented 5 years ago

@dsyme / @drvink Thoughts? Could/should a ProvidedType or ProvidedMembers have access to TargetContext and thus ConvertTargetTypeToSource so thats the potential exists to get back the original RunTimeType etc? That way full reflection could still work in the context of the source, if not I tink we would be limited in creating a type from the .FullName, .DeclaringType.Assembly.Fullname and using that as an invoke target which is much the same...

dsyme commented 5 years ago

@7sharp9 Could you get this green? thanks

7sharp9 commented 5 years ago

Had to mark this ready for review to see the conflicts, I will have a look at the changes soon.