You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
A Universal App or unit test executing code like <@ System.Exception() @> in an F# 3.1 / Profile 259 portable class library will result in an AmbiguousMatchException at runtime. Stacktrace looks something like
at Microsoft.FSharp.Core.ReflectionAdapters.commit[a](a[] results)
at Microsoft.FSharp.Core.ReflectionAdapters.Type.GetConstructor(Type this, Type[] parameterTypes)
at Microsoft.FSharp.Quotations.PatternsModule.bindCtor(Type tc, FSharpFunc2 argTypes, FSharpList1 tyargs)
at Microsoft.FSharp.Quotations.PatternsModule.u_constSpec@1424-40.Invoke(FSharpList1 tyargs) at Microsoft.FSharp.Quotations.PatternsModule.u_Expr@1305-2.Invoke(BindingEnv env) at Microsoft.FSharp.Quotations.PatternsModule.u_Expr@1327-7.Invoke(BindingEnv env) at Microsoft.FSharp.Quotations.PatternsModule.deserialize(Type localAssembly, FSharpList1 types, FSharpList1 splices, Byte[] bytes) at Microsoft.FSharp.Quotations.FSharpExpr.Deserialize(Type qualifyingType, FSharpList1 spliceTypes, FSharpList`1 spliceExprs, Byte[] bytes)
at ...
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.
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:
@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.
@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.
Activity
dsyme commentedon Jan 20, 2015
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.
...
dsyme commentedon Jan 20, 2015
@stephen-swensen - please attach a link to repro solution as well, thanks.
stephen-swensen commentedon Jan 20, 2015
Hi @dsyme -
I created a repo solution at http://swensen.googlecode.com/svn/AmbiguousMatchRepo/. Run the
Repo.repoxUnit 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
ConstructorInfomatches 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.
stephen-swensen commentedon Jan 21, 2015
I'd be willing to attempt a pull-request fixing this bug if that sounds alright with you.
dsyme commentedon Jan 21, 2015
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)
latkin commentedon Jan 21, 2015
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:
brodyberg commentedon Jan 21, 2015
@latkin This is great - something that might be a useful extension to our bug filing requirements/background information and/or repro.
dsyme commentedon Jan 29, 2015
@stephen-swensen - any progress on a PR for this? Thanks
stephen-swensen commentedon Jan 29, 2015
@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.
dsyme commentedon Jan 29, 2015
OK, thanks!
stephen-swensen commentedon Feb 5, 2015
@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.
dsyme commentedon Feb 5, 2015
Great news! :)
Fix dotnet#68 - filter out type initializer from Type.GetConstructor …
work around F# bug constructing System.Exception() inside quotation i…
work around F# bug constructing System.Exception() inside quotation i…