Repository navigation
Referencing multiple assemblies can trigger TypeError when calling an extension method #810
Description
Running the following in .NET (Core):
import clr
clr.AddReference("System.Core")
clr.AddReference("System.Linq")
import System.Linq
clr.ImportExtensions(System.Linq)
range(1,10).ElementAt(5)leads to the following TypeError:
TypeError: Multiple targets could match: ElementAt(IEnumerable[Int32], Int32), ElementAt(IEnumerable[Int32], Int32)
Reported by @Simon900225 on gitter.
Activity
BCSharp commentedon Mar 2, 2022
Interestingly, it works fine in ipy3.
BCSharp commentedon Mar 2, 2022
Sorry, false alarm , I see label "core" now.
BCSharp commentedon Mar 3, 2022
It seems that on .NET Core,
Enumerableextensions are exported bySystem.Core.dll, and bySystem.Linq.dll. Therefore there are two identical ambiguous targets, each one comes from a different assembly. The above example works on .NET Core if one of the DLL references is removed. It also keeps working on .NET Framework if the reference toSystem.Linq(but notSystem.Core) is removed.I am not sure what should be a desired behaviour here; should overloads from one DLL hide extension methods of matching signature defined in another DLL? Any order of priority?
slozier commentedon Mar 3, 2022
My initial reaction to the issue is that if the user was loading two different versions of an assembly I would just say fix your code. But in this case they're loading
System.Corewhich is just a facade assembly that's type-forwardingEnumerableto theSystem.Linqassembly so it's actually the same type? Feels like we should be able to avoid the ambiguity in this case.BCSharp commentedon Mar 4, 2022
@slozier I think I agree with you: if two or more assemblies export extension methods with the same signature, then it is a legitimate "ambiguous call" case and the error message is warranted (I wish it were more informative, though). However, if it just a case of a simple type forwarding, then
ImportExtensionsshould avoid importing from the same type twice.Simon900225 commentedon Mar 7, 2022
A sidenote is that it worked up until 2.7.9 and then stopped working from 2.7.10 and forward.
slozier commentedon Mar 7, 2022
@Simon900225 Were you using 2.7.9 with .NET Core?
Simon900225 commentedon Mar 7, 2022
Yes, with .NET 6. But I guess that it's running with help of the "compatibility layer" then as 2.7.9 is not built with .NET core as a target.
slozier commentedon Mar 7, 2022
Ok, 2.7.9 has a .NET Core 2.1 target so I assume .NET 6 will use those assemblies. I'll have to see what changed between 2.7.9 and 2.7.10.
slozier commentedon Mar 12, 2022
Looks like it started failing on #619 (which makes sense).
slozier commentedon Mar 12, 2022
Similar issue hitting slightly different code paths:
In this case the 2nd
ImportExtensionsis loading theSystem.Linqassembly (because that's the assembly of the forwarded type) and we then get the same failure.slozier commentedon Apr 15, 2022
Looks like
test_cliclass.test_extension_methodsis hitting this issue.Simon900225 commentedon Dec 13, 2022
Hi. I'd like to get this issue resolved in some way. Is there a way for my company to sponsor this? Or could I get some input on how I can troubleshoot, find and fix this issue myself?
slozier commentedon Dec 14, 2022
I had looked at this a while back, you could deduplicate the methods in
ExtensionMethodSet.GetExtensionMethods. Something like IronLanguages/ironpython3#1620. Not sure why I had not gone ahead with this fix, maybe I was worried about theHashSetallocations on every call...BCSharp commentedon Dec 14, 2022
I think the concern about creating and populating a
HashSeton every call is a valid one.Another idea: what if methods are being resolved in in a way that those defined in types with
TypeForwardedToAttributehave a lower priority than regular type methods? If there are two "regular" methods it is an ambiguity, but with one, this one gets resolved regardless of how may other methods on forwarded types are defined. With no "regular" methods but several forwarded ones, pick the first on the list, which is then functionally equivalent to deduplication.slozier commentedon Dec 15, 2022
I guess we'd need to add some API surface for this since the
PythonExtensionBinderonly getsMethodInfoobjects and there is no distinction between the forwarded and "regular" methods. Although now that I'm looking atPythonExtensionBinder.GetMember(the only consumer ofExtensionMethodSet.GetExtensionMethods) it's already allocating aList<MemberTracker>. This could easily be switched to aHashSet<MemberTracker>and this would eliminate duplicates, although then we wouldn't preserve insertion order?BCSharp commentedon Dec 16, 2022
After giving it some thought today I think that replacing
ListwithHashSetis probably a good compromise. I do not see a problem with not preserving insertion order since any set larger than one leads to ambiguity hence an error.I have seen those lists and arrays created in several places and thought that they can be replaced with arrays allocated from a memory pool (for performance reasons). This will not be possible with a hash set. However, at this stage, correctness and usability trumps hypothetical performance gain.
Simon900225 commentedon Feb 21, 2023
Hi. Any progress on this? I see that there is a PR in progress.
slozier commentedon Feb 26, 2023
Re-opening as the fix is only in ipy3...