F# 3.1 / Profile 259: <@ System.Exception() @> causes AmbiguousMatchException at runtime about fsharp HOT 12 CLOSED

Hi @stephen-swensen - do you know why this is? Here is the code for ReflectionAdapters - any idea why multiple constructors are being returned here?

I suppose the problem goes away when you add a binding redirect for FSharp.Core from the profile 259 FSharp.Core to 4.3.1.0.

It would be good to know if/why our existing unit tests don't catch this when run on profile259.

let commit (results : _[]) = 
    match results with
    | [||] -> null
    | [| m |] -> m
    | _ -> raise (AmbiguousMatchException())

...

    member this.GetConstructor(parameterTypes : Type[]) = 
        this.GetTypeInfo().DeclaredConstructors
        |> Seq.filter (fun ci ->
            let parameters = ci.GetParameters()
            (parameters.Length = parameterTypes.Length) &&
            (parameterTypes, parameters) ||> Array.forall2 (fun ty pi -> pi.ParameterType.Equals ty) 
        )
        |> Seq.toArray
        |> commit

from fsharp.

@stephen-swensen - please attach a link to repro solution as well, thanks.

from fsharp.

Hi @dsyme -

I created a repo solution at http://swensen.googlecode.com/svn/AmbiguousMatchRepo/. Run the Repo.repo xUnit test using VS2013 Test Explorer (I am using xUnit pre-release NuGet packages that support testing portable profiles and Test Explorer integration).

I did some further digging: evidently System.Exception in profile 259 has a static no-arg class constructor in addition to the expected instance no-arg constructor whereas the full .net 45 only has the instance no-arg constructor.

In the following screen-shot you can see the two ConstructorInfo matches being returned: .ctor() and .cctor() where the later is the unexpected static no-arg class constructor.

I think the fix for this issue is just to filter out static class constructors.

As to why your unit tests don't catch this issue, possibly you simply don't have a test case that covers this scenario, or else possibly your test runner is not actually bound to the profile 259 framework.

from fsharp.

I'd be willing to attempt a pull-request fixing this bug if that sounds alright with you.

from fsharp.

Ah!!! Yes, that's the problem. Great work to find the root cause.

Re PR - yes please, that would be great. Let us know if you need a hand. You can just add this exact quotation as a unit test here: https://github.com/Microsoft/visualfsharp/blob/fsharp4/src/fsharp/FSharp.Core.Unittests/FSharp.Core/Microsoft.FSharp.Quotations/FSharpQuotations.fs. (We will do a separate check to see if we are running the unit tests bound to the profile259 framework - you don't have to fix that if we're not)

from fsharp.

I am fairly confident that the unit tests (and the core portable tests) bind to the correct portable runtimes. This is indeed fraught with peril, but it seems stable for now. There is similar danger of tests accidentally binding to shipped F# bits on your machine, rather than private-built open source bits, but again, I believe I got this all rooted out.

e.g. running 'coreunitportable259' with fusion logging on reveals proper binding:

Similarly, logging all ReadFile operations on anything called FSharp.Core.dll using ProcMon shows that the right guy is being used:

from fsharp.

@latkin This is great - something that might be a useful extension to our bug filing requirements/background information and/or repro.

from fsharp.

@stephen-swensen - any progress on a PR for this? Thanks

from fsharp.

@dsyme per the guidelines at https://github.com/Microsoft/visualfsharp/blob/fsharp4/CONTRIBUTING.md, I am currently in the process of seeking appropriate signatures from my employer for the CLA so that I can contribute. I hope to know the outcome within a week or so, but no definite timeline. Once that is all sorted out, I will get started on the PR right away.

from fsharp.

OK, thanks!

from fsharp.

@dsyme good news - both myself and my employer have signed the CLA so I am now free to contribute! I plan to submit a pull request for this issue some time next week, thanks.

from fsharp.

Great news! :)

from fsharp.