fsprojects / FSharp.TypeProviders.SDK

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

StackOverflow with TryBindType - GetType loop #182

Closed Thorium closed 6 years ago

Thorium commented 6 years ago

ProvidedTypesContext class has a function of tryGetTypeFromAssembly That calls asm.GetType fullName |> function null -> None | x -> Some (x, true) which calls TargetAssembly's method:

        override x.GetType (nm:string) =
            if nm.Contains("+") then
                let i = nm.LastIndexOf("+")
                let enc,nm2 = nm.[0..i-1], nm.[i+1..]
                match x.GetType(enc) with
                | null -> null
                | t -> t.GetNestedType(nm2,bindAll)
            elif nm.Contains(".") then
                let i = nm.LastIndexOf(".")
                let nsp,nm2 = nm.[0..i-1], nm.[i+1..]
                x.TryBindType(USome nsp, nm2) |> Option.toObj
            else
                x.TryBindType(UNone, nm) |> Option.toObj

which calls TryBindType which contains call back to GetType and the loop is ready

        member __.TryBindType(nsp:string uoption, nm:string): Type option =
            match getReader().ILModuleDef.TypeDefs.TryFindByName(nsp, nm) with
            | Some td -> asm.TxILTypeDef None td |> Some
            | None ->
            match getReader().ILModuleDef.ManifestOfAssembly.ExportedTypes.TryFindByName(nsp, nm) with
            | Some tref ->
                match tref.ScopeRef with
                | ILScopeRef.Assembly aref2 ->
                    let ass2opt = tryBindAssembly(aref2)
                    match ass2opt with
                    | Choice1Of2 ass2 -> 
                        match ass2.GetType(joinILTypeName nsp nm)  with 
                        | null -> None
                        | ty -> Some ty
                    | Choice2Of2 _err -> None
                | _ ->
                    printfn "unexpected non-forwarder during binding"
                    None
            | None -> None

GetType "System.Object" -> TryBindType Some("System") "Object" -> GetType "System.Object" -> TryBindType Some("System") "Object" -> ...

Thorium commented 6 years ago

This is mostly valid for .NET Standard libraries using type-providers: It's not clear if the base types come from mscorlib.dll or netstandard.dll.

Thorium commented 6 years ago

Adding just

if nm.StartsWith("System.") then
    Type.GetType nm
else

to the override x.GetType (nm:string) of ProvidedTypes.fs will get rid of stackoverflow, but the .NET Standard build still fails:

error FS1109: A reference to the type 'System.Numerics.BigInteger' in assembly 'netstandard' was found, but the type could not be found in that assembly [C:\myproj\myproj.fsproj]
FSC : error FS1109: A reference to the type 'System.Numerics.BigInteger' in assembly 'System.Numerics' was found, but the type could not be found in that assembly [C:\myproj\myproj.fsproj]
FSC : error FS1109: A reference to the type 'System.Numerics.BigInteger' in assembly 'netstandard' was found, but the type could not be found in that assembly [C:\myproj\myproj.fsproj]
C:\Users\tuomas\AppData\Local\Temp\.NETStandard,Version=v2.0.AssemblyAttributes.fs(2,34): error FS0039: The type 'Versioning' is not defined. [C:\myproj\myproj.fsproj]
C:\myproj\Program.fs(17,3): error FS0039: The type 'Literal' is not defined.
Thorium commented 6 years ago

System.Numerics.BigInteger problem is a bit different: fsc.exe compiler tries to evaluate auto-opens of the reference assemblies and FSharp.Core.dll has some references like Microsoft.FSharp.Core.bigint What is the library for these in .NET Standard? There is none?

dsyme commented 6 years ago

@Thorium The library is System.Numerics.dll

dsyme commented 6 years ago

@Thorium Am I correct that you are seeing this bug when using a type provider to generate code for .NET Standard 2.0? We need to add testing for that scenario, thanks

Thorium commented 6 years ago

Correct. I added a PR #185 to test that. There are not many changes. Don't merge the PR (as it will just break unit tests) but check the changes.

dsyme commented 6 years ago

@Thorium That's not quite what's need to test targeting .NET Standard 2.0 in code generation. I'll work on testing this today, will send a link

dsyme commented 6 years ago

@Thorium I haven't been able to reproduce this bug. Could you write out step-by-step instructions for reproducing it? I don't want to pull #185 until I understand what's going on

Thorium commented 6 years ago

With the latest changes, targeting to .NET Standard, ProvidedTypes.fs dllInfos is null here:

 [ for dllInfo in dllInfos.GetElements() -> (dllInfo.GetProperty("FileName") :?> string)
   if not (isNull baseObj) then
     for dllInfo in baseObj.GetProperty("Value").GetField("dllInfos").GetElements() -> (dllInfo.GetProperty("FileName") :?> string) ]

Now, this can be skipped with adding a corresponding isNull for dllInfos. But if we skip these dlls the compiler will be in the loop of stack-overflow described in this bug. By adding a check in #183 the stackoverflow is avoided.

Thorium commented 6 years ago

Maybe the problem is that the tcImports is not working correctly on .NET Standard target and trying to fix the side-effects won't help.

dsyme commented 6 years ago

@Thorium I see. dllInfos should not be null there. I might need more details of exactly which F# compiler you are using and how you are running it

dsyme commented 6 years ago

@Thorium https://github.com/fsprojects/FSharp.TypeProviders.SDK/pull/194 adds improved diagnostics if dllInfos is null, thanks

Thorium commented 6 years ago

Getting the latest #194 and trying to build the library using the type provider, compiling with .NET Standard 2.0 still gives:

C:\Program Files\dotnet\sdk\2.1.2\FSharp\Microsoft.FSharp.Targets(263,9): error MSB6006: "fsc.exe" exited with code -1073741571.

And with more diagnostics:

         Microsoft (R) F# Compiler version 4.1
         Copyright (c) Microsoft Corporation. All Rights Reserved.

         Process is terminated due to StackOverflowException.

I have the definition in the fsproj file:

  <PropertyGroup Condition="'$(IsWindows)' == 'true'">
    <FscToolPath>C:\Program Files (x86)\Microsoft SDKs\F#\4.1\Framework\v4.0</FscToolPath>
    <FscToolExe>fsc.exe</FscToolExe>
  </PropertyGroup>

fsc.exe file version is 2017.11.4.1, product version 4.4.1.0 I have cloned the visualfsharp repo so if I try version 2017.12.11.0 (changeset d19fb00f95c183a4d4ed3f45c7ca74b48a485e13) the result is the same.

I expect that again this is the loop mentioned in the title of this bug.

Thorium commented 6 years ago

Why is ProvidedTypes.fs line override x.GetType (nm:string) = parameter nm getting values with starting "System." (system types) also?

dsyme commented 6 years ago

Getting the latest #194 and trying to build the library using the type provider, compiling with .NET Standard 2.0 still gives:

When you say "compiling with .NET Standard 2.0" what do you mean? Compiling SqlProvider? Or one of the samples? Or putting netstandard2.0 in FSharp.TypeProviders.SDK.Tests.fsproj? Or something else? Thanks

dsyme commented 6 years ago

Why is ProvidedTypes.fs line override x.GetType (nm:string) = parameter nm getting values with starting "System." (system types) also?

That is correct for TargetTypeDefinition. The SDK builds a reflection model of types in the target assemblies. Also other assemblies will be interrogated for these types, returning null if they are not present.

Thorium commented 6 years ago

When you say "compiling with .NET Standard 2.0" what do you mean? Compiling SqlProvider? Or one of the samples? Or putting netstandard2.0 in FSharp.TypeProviders.SDK.Tests.fsproj? Or something else?

I mean I'm compiling a project that is using a type provider. In my case any of SQLProvider samples at https://github.com/fsprojects/SQLProvider/tree/master/tests/SqlProvider.Core.Tests in a slight modification that their target is netstandard2.0: <TargetFrameworks>netcoreapp2.0;netstandard2.0</TargetFrameworks>

The type provider, SQLProvider, itself compiles nicely. But users of that cannot target to nestandard2.0

So I expected that adding netsrandard2.0 target to FSharp.TypeProviders.SDK.Tests.fsproj would also replicate the same problem, if that project is returning anything from any reference-dll like FSharp.Core. The only modification needed is that .NET Standard is a library, not executable, so there shouldn't be EntryPointAttribute in NET Standard compilation.

dsyme commented 6 years ago

@Thorium Ah I see, thanks. FSharp.TypeProjects.SDK.Tests.fsproj is not actually using a type provider (i.e. as a compiler addin), but rather directly unit-testing the type provider SDK at the level of provided types and their properties. But perhaps now I can repro the problem by compiling SqlProvider.

Thorium commented 6 years ago

Those tests do use NuGet reference of SQLProvider (as referencing direct files was broken when I started with .NET Core). I just pushed the latest updated ProvidedTypes.fs version as SQLProvider 1.1.26-alpha1 to NuGet.

dsyme commented 6 years ago

@Thorium I tried to use the following repro steps and still couldn't repro it:

  1. Install MySql with default settings
  2. git clone https://github.com/dsyme/SQLProvider
  3. msbuild src\SQLProvider\SqlProvider.fsproj
  4. cd C:\GitHub\dsyme\SQLProvider\tests\SqlProvider.Core.Tests\MsSql
  5. Use the freshly-built SQLProvider <Reference Include="FSharp.Data.SqlProvider"><HintPath>../../../bin/net451/FSharp.Data.SqlProvider.dll</HintPath> </Reference> rather than the one from the package
  6. Use a couple of modifications to use a default database that got installed with MySql, notably the code below
  7. .\build.cmd
  8. change to <TargetFramework>netstandard2.0</TargetFramework> in tests\SqlProvider.Core.Tests\MySql\SqlProvider.Core.Tests.fsproj
  9. .\build.cmd

All commands succeeded, when I look at the generated DLL it has generated .NET Standard 2.0 code successfully. This is with the latest TPSDK (which the project uses by default). What am I missing?

image

Thanks! don

Here's the code adjustment to use the default database:

let connStr =  "Server=localhost;Database=sakila;Uid=root;Pwd=PASSWORD;Convert Zero Datetime=true;"

type HR = SqlDataProvider<Common.DatabaseProviderTypes.MYSQL, connStr,  ResolutionPath = msyqlConnectorPath>

...
            for emp in ctx.Sakila.Staff do
...
Thorium commented 6 years ago

Hi, Ok... So the problem was: I used the NuGet package which detected that if my target is .NET Standard 2.0 library, the reference is also netstandard2.0 version rather than net451 version. It seems that you used the net451 compilation of SQLProfiler (src\SQLProvider\SqlProvider.fsproj) rather than the netstandard2.0 (\src\SQLProvider.Standard\SQLProvider.Standard.fsproj), which seems reasonable, now that you again pointed it out, as I know the fsc.exe used by dotnet.exe is the .Net framework version.

Now my question is (documentation related, and maybe stupid, but keep in mind I don't know the details of .NET Core compilation well, I'm just a user): Will we ever need bin\netstandard2.0\FSharp.Data.SqlProvider.dll (e.g. on runtime on .NET Standard library) or is \bin\net451\FSharp.Data.SqlProvider.dll always enough? In other words: does the fsharp compilation totally embed the erasing type provider library to the compiled dll output, or will it have external references still to the type provider dll?

dsyme commented 6 years ago

@Thorium Great, thanks for letting me know!

Will we ever need bin\netstandard2.0\FSharp.Data.SqlProvider.dll (e.g. on runtime on .NET Standard library) or is \bin\net451\FSharp.Data.SqlProvider.dll always enough?

Looking at the way your type provider works, you will indeed need bin\netstandard2.0\FSharp.Data.SqlProvider.dll at runtime when people are generating code for .NET Standard 2.0.

For now I think the nuget package should look like this:

lib\net451\FSharp.Data.SqlProvider.dll

lib\netstandard2.0\FSharp.Data.SqlProvider.dll
lib\netstandard2.0\netstandard.dll
lib\netstandard2.0\System.Console.dll
lib\netstandard2.0\System.IO.dll
lib\netstandard2.0\System.Data.SqlClient.dll
lib\netstandard2.0\System.Reflection.dll
lib\netstandard2.0\System.Runtime.dll

Eventually you will need to also make some changes to use the type provider in conjunction with the F# compiler that comes in the .NET SDK (which runs on .NET Core, as opposed to the one you get via setting FscToolPath here). For now don't worry about it though - the F# compiler in the .NET SDK has not yet been updated and there are still ongoing discussions about how exactly a type provider will be packaged to work with both the C:\Program Files (x86)\Microsoft SDKs\F#\4.1\Framework\v4.0 compiler and the .NET SDK compiler simultaneously. For now people will just continue to have to set FscToolPath, though they should now be able to correctly generate .NET Standard 2.0 code.

In other words: does the fsharp compilation totally embed the erasing type provider library to the compiled dll output, or will it have external references still to the type provider dll?

In your cases it has external references. In TP jargon you have your TPRTC and TPDTC in one combined DLL.

dsyme commented 6 years ago

(@Thorium note I updated the comment above, my first version was inaccurate)

BTW I believe that in the long run you won't need the .NET 4.5.1 version of the type provider at all. The only place I can see where the netstandard2.0 and net451 TPDTCs vary is here:


#if NETSTANDARD
        if conStringName <> "" then
            failwith "ConnectionStringName not supported in .NET Standard. Use a development connection string and then give a runtime connection string as parameter variable to GetDataContext"
#endif

But I suspect this can just be deleted - and that ConnectionStringName might work correctly at design-time and run-time. That would simplify matters - you would then just have a netstandard2.0 TPDTC+TPRTC capable of generating code for either .NET 4.5.1 or .NET Standard 2.0

For now the .NET 4.5.1 TPDTC is still needed for backwards compat, since .NET Standard 2.0 is not supported pre .NET 4.7