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 PCL 259 #672

Closed
wants to merge 4 commits into from
Closed

Support for PCL 259 #672

wants to merge 4 commits into from

Conversation

ovatsus
Copy link

@ovatsus ovatsus commented Aug 30, 2014

Work in progress

@ovatsus
Copy link
Author

ovatsus commented Aug 30, 2014

PCL 259 has the same problems as PCL 7 had (#521), and I still has no clue how to fix it, but as PCL7 was basically redundant compared to PCL47 for most purposes, I dropped it.

PCL 259 is more interesting, though, and I'd really like to support it

I'm already remapping mscorlib types to System.Runtime.* types since the last iteration on this problem, but I'm still getting these errors:

error FS3034: The type provider 'CsvProvider' used an invalid parameter in the ParameterExpression: row
error FS3034: The type provider 'XmlProvider' used an invalid parameter in the ParameterExpression: t
error FS3034: The type provider 'JsonProvider @' used an invalid parameter in the ParameterExpression: t

@v2m @dsyme @KevinRansom @CodeCutter @latkin: Help please!

@ghost
Copy link

ghost commented Aug 31, 2014

I sort of think that the type providers should not be doing any type mapping, I think the compiler should, which is probably a different position than we have talked about before, however, this is entirely the compilers Job and if the TypeProvider has code duplicating that, it is bound to come up with different answers from time to time. Also, I don't really see why the TP should have to be aware of each profile and how to do the mapping. It seems to me that the problem is that we don't expose any APIs that type provider authors can call to do this work. I will have a chat with Vlad next week to see if there is anything we can do to make consuming the .NET profiles from TypeProviders less error prone at least in respect to Type Mapping.

In the mean time could you put together a tiny repro so that we can talk about the issue with a concrete example. Is there an issue on CodePlex for discussion?

BTW. Profile 47 contains legacy APIs, code using profiles 7, 78 and 259 are less likely to require rewriting moving forward.

Thanks

Kevin

Date: Sat, 30 Aug 2014 06:14:35 -0700
From: notifications@github.com
To: FSharp.Data@noreply.github.com
CC: codecutter@hotmail.com
Subject: Re: [FSharp.Data] Support for PCL 259 (#672)

PCL 259 has the same problems as PCL 7 had (#521), and I still has no clue how to fix it, but as PCL7 was basically redundant compared to PCL47 for most purposes, I dropped it.

PCL 259 is more interesting, though, and I'd really like to support it

I'm already remapping mscorlib types to System.Runtime.* types since the last iteration on this problem, but I'm still getting these errors:

error FS3034: The type provider 'CsvProvider' used an invalid parameter in the ParameterExpression: row

error FS3034: The type provider 'XmlProvider' used an invalid parameter in the ParameterExpression: t

error FS3034: The type provider 'JsonProvider @' used an invalid parameter in the ParameterExpression: t

@v2m @dsyme @KevinRansom @CodeCutter @latkin: Help please!


Reply to this email directly or view it on GitHub.

@ovatsus
Copy link
Author

ovatsus commented Aug 31, 2014

What do you mean by "Profile 47 contains legacy APIs"?

I was thinking about getting rid of type mapping altogether. One option would be to only have one portable profile and not the full .net version, and make FSharp.Data.DesignTime depend on FSharp.Data. That would cause some assembly loading problems, but should be fixable. The only problem is that I would have to stick with 47 only to be able to support Vs2012, and I would have to use reflection to be able to load files from the disk when using the full .net.

Another option would be to have a FSharp.Data.DesignTime version for each FSharp.Data version, but I think I tried that before. I still have remapping issues when VS2013 is being used which forces loading of FSharp.Core 4.3.1.0 while FSharp.Data used 4.3.0.0... I can give it another try

@ovatsus
Copy link
Author

ovatsus commented Aug 31, 2014

Actually, the problem appears in a much simpler case. If I create a ProvidedProperty of type typeof it doesn't work for the PCL, as it will be the mscorlib type instead of the System.Runtime type

@tpetricek
Copy link
Member

53687218 1

@ovatsus
Copy link
Author

ovatsus commented Sep 10, 2014

@tpetricek LOL

@dsyme
Copy link
Contributor

dsyme commented Oct 18, 2014

I'd really like to get this going - I'm hoping to help look at the problem next week.

@ovatsus - Profile 47 (mscorlib-based) is sort of regarded as "legacy" now, and the System.Runtime profiles (Profile7, Profile 78, Profile259) seem to be the way of the future for PCL. That's one of the reasons why this is a priority for me - I'd like to understand the issues at a deeper level.

@ovatsus
Copy link
Author

ovatsus commented Oct 23, 2014

Why is 47 considered "legacy"? It's the closest one to full .Net.

The root of the problem is incompatibility between types in mscorlib and system.runtime, and trying to look them up directly fails. I was thinking about this problem, and I could try to always get the types indirectly to see if it solves the problem:

  • Instead of including some files both in FSharp.Data.dll and FSharp.Data.DesignTime.dll, reference FSharp.Data.dll from FSharp.Data.DesignTime.dll
  • Have a designTime dll for each runtime dll version, and do not look at TypeProviderConfig.RuntimeAssembly at all
  • Add a dummy static class to the runtime with a member for each primitive type, including nullables and options, and always reference that instead of using typeof or similar
  • With this, AssemblyReplacer and AssemblyLoader are no longer needed, neither is using reflectionOnlyLoad

@KevinRansom
Copy link

It’s the furthest away from NetCore, except desktop.

From: Gustavo Guerra [mailto:notifications@github.com]
Sent: Wednesday, October 22, 2014 5:07 PM
To: fsharp/FSharp.Data
Cc: Kevin Ransom
Subject: Re: [FSharp.Data] Support for PCL 259 (#672)

Why is 47 considered "legacy"? It's the closest one to full .Net.

The root of the problem is incompatibility between types in mscorlib and system.runtime, and trying to look them up directly fails. I was thinking about this problem, and I could try to always get the types indirectly to see if it solves the problem:

  • Instead of including some files both in FSharp.Data.dll and FSharp.Data.DesignTime.dll, reference FSharp.Data.dll from FSharp.Data.DesignTime.dll
  • Have a designTime dll for each runtime dll version, and do not look at TypeProviderConfig.RuntimeAssembly at all
  • Add a dummy static class to the runtime with a member for each primitive type, including nullables and options, and always reference that instead of using typeof or similar
  • With this, AssemblyReplacer and AssemblyLoader are no longer needed, neither is using reflectionOnlyLoad


Reply to this email directly or view it on GitHubhttps://github.com//pull/672#issuecomment-60175287.

@ovatsus
Copy link
Author

ovatsus commented Oct 23, 2014

BTW, only having PCL versions of NetCore is also not ideal, as you can't use the bait-and-switch (http://log.paulbetts.org/the-bait-and-switch-pcl-trick/) approach to nuget packages and are forced to use reflection like we do in FSharp.Data. There's lot of places where all the final .Net profiles being run have one method, but none of the PCL's has it. There should be FSharp.Core versions targeting the full phone/windows profiles.

@dsyme
Copy link
Contributor

dsyme commented Oct 23, 2014

@ovatsus Presumably in many cases those FSharp.Core DLLs would be identical to the ones for the PCL Profiles, apart from the attribute attached to the DLL that indicates which profile it is assocaited with.

@ovatsus Re "legacy" - whether we like it or not, the .NET Core programming model (centred on System.Runtime, typified by the new reflection API) seems to be the future heart of the PCL and .NET. It seems reasonable to infer (though I'm just guessing) that in the future there may be configurations of .NET that don't support the "full .NET 4.x" model - whether .NET configurations for cloud, desktop or device. That said for the moment such configurations are primarily relevant to devices and don't intrude in typical desktop or cloud enterprise programming much.

@ovatsus
Copy link
Author

ovatsus commented Oct 23, 2014

Well, at least we need a full .Net version, a pcl47 version, and one of the NetCore versions, but because of strong naming and different version numbers, we probably need all

@ghost
Copy link

ghost commented Oct 24, 2014

I can appreciate your concern, however without specific scenarios, it's a little tricky to figure out what we need. With the current set of profiles it is possible to create libraries that target the most common scenarios, and a single compiler codebase that supports them all.

I think we need to add some Apis that enable type provider authors to more easily cope with the complexity of multiple profile support. We should probably do stuff that supports our multi-targeting support as well.

Regardless ... that is not work we currently have planned for #fsharp 4.0, I think we should try to put our heads together and identify a solution to the problem and add it into our planning process.

It would help if you could identify the type provider scenarios around versioning and profiles that you want to enable and then ewe can identify the roadblocks and address those.

I hope this makes sense and helps,

Kevin

Date: Thu, 23 Oct 2014 02:55:23 -0700
From: notifications@github.com
To: FSharp.Data@noreply.github.com
CC: codecutter@hotmail.com
Subject: Re: [FSharp.Data] Support for PCL 259 (#672)

Well, at least we need a full .Net version, a pcl47 version, and one of the NetCore versions, but because of strong naming and different version numbers, we probably need all


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

@ovatsus
Copy link
Author

ovatsus commented Nov 15, 2014

I was trying to do this:

  • Instead of including some files both in FSharp.Data.dll and FSharp.Data.DesignTime.dll, reference FSharp.Data.dll from FSharp.Data.DesignTime.dll
  • Have a designTime dll for each runtime dll version, and do not look at TypeProviderConfig.RuntimeAssembly at all
  • Add a dummy static class to the runtime with a member for each primitive type, including nullables and options, and always reference that instead of using typeof or similar
  • With this, AssemblyReplacer and AssemblyLoader are no longer needed, neither is using reflectionOnlyLoad

But I fail to even get the first point working, as when I reference FSharp.Data from a project that wants to use the TP, I get errors like these all over the place:

Error 1 The type provider 'ProviderImplementation.CsvProvider' reported an error: Could not load file or assembly 'FSharp.Data, Version=2.1.1.0, Culture=neutral, PublicKeyToken=null' or one of its dependencies. The system cannot find the file specified. C:\Git\FSharp.Data\tests\FSharp.Data.Tests\CsvProvider.fs 21 18 FSharp.Data.Tests

And I can't even put a breakpoint and attach a debugger, it seems the exception is very early on the loading process. Can anyone have a look?

@ovatsus
Copy link
Author

ovatsus commented Feb 15, 2015

Any pointers? @vladima @dsyme

@SabotageAndi
Copy link

Any idea/timeframe when this is completed?

@paulyoung
Copy link

I'd love to know this too.

@KevinRansom
Copy link

There are no plans to add additional profiles at this time.

There are no plans to update the Type Providers to support the portable class libraries at this time. I think before we do that we need to do some work on the compiler / typeprovider interface to make type resolution more deterministic and predictable, effectively the type provider is clueless about reference assemblies, type forwarders the equivalence of System.Runtime and mscorlib blah ... blah ... blah ...

As you can probably tell from the threads linked above Don and I differ on what is probably necessary, Don is more of a minimalist than I am, we haven't yet coalesced on a design for what this looks like. I'm thinking that we will probably tackle that as part of the cross platform coreclr work. Writing a cross platform type provider is going to require a much more rigorous approach to type binding and so we will need it there for sure. Once we have that in place if we end up where I want to be, a type provider should not really need to know much about profiles, at least as far as imported types go.

However ... that is a bit down the road. I'm sorry I can't be more concrete than that, the problem is difficult, if it weren't it would be fixed already. Fortunately the cross plat work is going to force us to provide a strong solution to it, so eventually we will be in a good place.

Does this help?

Kevin

@dsyme
Copy link
Contributor

dsyme commented Aug 27, 2015

Hi Kevin,

I'm all for fixing this by providing further helpers in the fundamental compiler interface as necessary (and then presumably mapping those up through the ProvidedTypes API which all type providers are authored against). Do you have any rough proposal for what this might look like?

I find it hard to imagine why any reverse-binding logic would be better placed in the compiler rather than in the ProvidedTypes implementation. But that might be because I'm more comfortable making changes to the ProvidedTypes API and we already implement a number of fixups there. In any case you're right that the CoreCLR work may flush this out, at least for CoreCLR targets.

I still do plan to take a look at solving this for F# 4.0 and .NET 4.x in ProvidedTyoes, I've had this red-flagged for ages and I really should have done this, my apologies. I expect a solution there would be useful to CoreCLR as well, even if the compiler is able to provide additional helpers.

@KevinRansom
Copy link

Hey Don,
On CoreCLR Assembly.Load, Assembly.LoadFrom and Assembly.ReflectOnlyLoad do not work the same, we would have to expose a LoadContext on the API that providedtypes would then need to use to reify reflect only types to import for code generation.

There are a number of reasons why the compiler should control the types that the type provider can use in generated code:

1.In the future FSC and FSI may use AOT (Ahead of Time) compilation the type provider will not be able to grab the fsharp.core that the compiler is using.
2.TypeProviders should be able to support multi-targeting – to do that they need to use the version of fsharp.core that was provided using –r: not typeof.Assembly.Location
3.TypeProviders should be able to support profiles – to do that they need to use the fsharp.core that was provided using –r: not typeof.Assembly.Location, similarly they need int from System.Runtime or mscorlib and so on for every other runtime type that exist in profiles.
4.We want an abstraction that will work well across platforms, IDEs and build environments.

In short the compiler command line needs to define the complete set of input files required to successfully complete the compilation, the type provider should not need to helpfully hunt around for assemblies to satisfy it's compilation requirements. The provider should definitely not need to look at configuration files, the registry or environment variables to know what to do.

It seems profoundly wrong to me that a type provider would need code like this, I can’t imagine why a compiler add-in should know so much about the compilation environment.

let private referenceAssembliesPath =  
    if runningOnMono 
    then "/Library/Frameworks/Mono.framework/Versions/CurrentVersion/lib/mono/" 
    else Environment.GetFolderPath Environment.SpecialFolder.ProgramFilesX86  
    ++ "Reference Assemblies"  
    ++ "Microsoft"  

This won’t work on coreclr:

let private designTimeAssemblies =  
    AppDomain.CurrentDomain.GetAssemblies() 
    |> Seq.map (fun asm -> asm.GetName().Name, asm) 
    // If there are dups, Map.ofSeq will take the last one. When the portable version 
    // is already loaded, it will be the last one and replace the full version on the 
    // map. We don't want that, so we use distinct to only keep the first version of 
    // each assembly (assumes CurrentDomain.GetAssemblies() returns assemblies in 
    // load order, must check if that's also true for Mono) 
    |> Seq.distinctBy fst  
    |> Map.ofSeq

In short we need an abstraction that enables a TypeProvider Author to do his work without worrying about how to access the types and that makes it difficult to do the wrong thing. I don't yet know what it looks like, since I haven't examined enough TPs to figure what would be useful APIs.

My guess is that whatever we do in ProvidedTypes.fs will be throw-away code, because coreclr has such a different reflection and assembly loading environment. We can be sure that existing TP's will need a lot of rewriting for coreclr, so many APIs have changed especially since they spend so much code on resolving types and assemblies wrongly.

I hope this makes sense, and I'm sorry I haven't yet a design for you to review

Kevin

@dsyme
Copy link
Contributor

dsyme commented Aug 27, 2015

Hi Kevin,

The LoadContext would be immensely useful, that's a fine direction for the core API.

It seems profoundly wrong to me that a type provider would need code like this...

I agree with this - the compiler should manage the set of bindable types. And in any solution, logic like the above gets taken out if all type providers and into the compiler and/or ProvidedTypes API

...throwaway code...

Yes eventually the portion of code used for type binding could be replaced, which would be great. But additional code in ProvidedTypes looks needed to rewrite quotations and provided type definitions to target bindable types Also any solution for .NET 4.x or F# 4.x is going to be used for a long time - realistically these will dominate F# usage for a good time.

@ovatsus ovatsus closed this Oct 11, 2015
@ovatsus ovatsus deleted the PCL259 branch October 11, 2015 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants