Repository navigation
Conversation
…s, as C# does
list.max(System::Func[Integer, Integer].new { -it }) raised "Found multiple
methods for 'max'": LINQ's Max<TSource>(IEnumerable<TSource>,
Func<TSource, int>) and Max<TSource, TResult>(IEnumerable<TSource>,
Func<TSource, TResult>) both become Max(IEnumerable<int>, Func<int, int>)
once instantiated, and the DLR's only rule for two generic methods is that
it has none ("Need to support selecting least generic method here"). Min
had the same problem with Int32, Int64, Double ... selectors; Sum and
Average were fine, as none of their selector overloads returns a type
parameter.
RubyOverloadResolver now overrides CompareEquivalentCandidates, the DLR's
hook for overloads no argument prefers, with C#'s tie-break ("better
function member"): when the parameter types are the same once instantiated,
the overload whose declared parameter types are more specific wins. A type
parameter is less specific than any other type, and a constructed or array
type is more specific than another of the same shape if one of its type
arguments is and none is less. The declared types are those of the generic
method definition in the generic type definition, so a Box<int>'s
Put(int) also wins over its Put(T). Parameters expanded from a params array
or reduced from a by-ref are left alone, as C# compares unexpanded ones.
Overloads whose instantiated parameter types differ are not compared at
all, so a block that fits several selector types (list.sum { it * 2 }) is
still reported as an ambiguity, with the hint to pass the delegate.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…; if all do, a TypeError
obj.Method("hi", Time.now) with C# overloads Method(string, string) and
Method(string, int) raised System::Reflection::AmbiguousMatchException.
On the last narrowing level a CLR method's String, numeric, Array or Hash
parameter takes any object, by Ruby's conversion protocols (to_str,
to_int, to_f, to_ary, to_hash), and whether the object can do them is only
found out by the call - so both overloads fitted equally, although calling
either could only raise. An object with #to_int only was just as
ambiguous, although Method(string, int) is the one it fits.
The resolver now asks the argument's class, as the conversion would:
ProtocolConversionAction.CanConvert answers false when the class has no
to_xxx method for the conversion and leaves respond_to? and method_missing
as Kernel and BasicObject define them - the cases in which BuildConversion
can only build the conversion error. With that, on the last level:
- of two conversions of an argument, one that can only fail is the worst
(SelectBestConversionFor), and of two overloads, one that can take the
arguments wins over one that cannot (CompareEquivalentCandidates), also
where another argument prefers the other: Pair(Int32, Int32) and
Pair(Double, Double) given an object with only #to_f and a 1;
- if every overload left in the ambiguity has an argument whose conversion
can only fail, the call raises the TypeError calling one would: "no
implicit conversion of Time into String or Integer", naming what the
first such argument would have to convert to. Positional, splatted and
keyword arguments alike.
An object that can do both conversions, or might (method_missing,
respond_to?), is an ambiguity as before, and so is any ambiguity on an
earlier level (nil for String/Byte[], ...). Library methods, whose
parameters take protocol conversions only where [DefaultProtocol] says so,
are not affected.
The answer depends on the argument's class, so the rule tests it as a
protocol conversion's rule tests its target (AddTargetTypeTest, a
condition for an argument read from a splatted list), and the lookups are
registered as BuildConversion registers them: defining or removing the
conversion method, in the class, a module it includes or an object's
singleton class, re-binds the call site.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The specificity comparison ran as soon as two generic overloads took the
same instantiated parameter types, so it picked overloads C# rejects:
F<T>(T, T) or F<T>(int, T, int c = 0) called with (1, 2) chose the one
that needs a default value, and Y<T>(T, T) or Y<T>(int, T, params int[])
the one that expands its params array into no items. Master reported
both as ambiguous.
C# ("better function member") applies these rules in order before it
compares parameter types:
- the normal form beats an expanded params array;
- of two expanded forms, the one with more declared parameters wins;
- all parameters passed beats a default value substituted.
Two overloads that both need default values are compared only if they
have as many parameters; Roslyn reports CS0121 otherwise.
The specificity comparison also handles the forms it used to skip.
Expanded params items are compared by their item type, and by-ref
parameters by their declared types. So M<T>(T, params T[]) or
M<T>(int, params T[]), and K<T>(T, ref int) or K<T>(T, ref T), resolve
as C# does. All 20 generic pairs checked, including the four C# finds
ambiguous, give the overload the C# compiler picks.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review of the previous commit found that its ranking made calls bind
that master rejected, and bind them with a loss of data:
- A Float or Single parameter counted any object with #to_f as
convertible, and a String as well, because the conversion parses one.
So System::Math.Abs("0.1"), Math.Max("10", 9) and Math.Max(Time, 1)
bound the Single overload. That rounds silently, and Math.Abs("abc")
became an ArgumentError instead of a TypeError. MRI's implicit Float
conversion (rb_to_float) takes only a Numeric. Ranking now uses that
rule, so these calls raise TypeError again. Passing such an argument
to a lone Float parameter still calls #to_f, as it did before.
- The "an overload that can take the arguments wins" rule in
CompareEquivalentCandidates decided even where another argument
preferred the other overload. Math.Min(only_to_int, 1.5) bound
Min(Byte, Byte) and returned 1, and Max(only_to_int, 300.5) raised an
OverflowException. The rule is gone. What remains is the ranking per
argument: a conversion the argument's class does not implement is the
worst there is. When the arguments disagree, the call stays an
AmbiguousMatchException, or raises the TypeError if no overload can
take the arguments.
The TypeError now names the Ruby classes MRI would name ("no implicit
conversion of Time into Integer or Float") rather than every CLR numeric
type. nil counts as lacking a Float conversion, since NilClass is not a
Numeric and the conversion rejects nil anyway.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 59 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two problems in how a Ruby call chooses between CLR overloads.
Generic overloads that instantiate to the same signature
Max<TSource>(IEnumerable<TSource>, Func<TSource, int>)andMax<TSource, TResult>(IEnumerable<TSource>, Func<TSource, TResult>)become the same signature once instantiated, and the DLR has no rule for two generic methods ("Need to support selecting least generic method here").minhad the same problem.sumandaveragewere fine.Fix.
RubyOverloadResolvernow overrides the DLR'sCompareEquivalentCandidateshook, so no DLR patch is needed. When two overloads take the same parameter types once instantiated, it applies C#'s "better function member" tie-breaks in C#'s order:The normal form beats an expanded params array, including one expanded into no items.
Of two expanded forms, the one with more declared parameters wins.
An overload that gets every parameter passed beats one that needs a default value. If both need defaults, the next rule applies only when they have as many parameters; otherwise the call stays ambiguous, as C# reports CS0121.
The overload whose declared parameter types are more specific wins:
Declared types come from the generic method definition inside the generic type definition, so
Box<int>.Put(int)beatsPut(T).A Ruby block that fits several selector delegates (
l.sum { it * 2 }) is still reported as an ambiguity, with the hint to pass the delegate explicitly.Calls that no overload can take raised AmbiguousMatchException
With C# overloads
Method(string a, string b)andMethod(string a, int b),obj.Method("hi", Time.new)raisedAmbiguousMatchException. On the last narrowing level, a CLR String, numeric, Array or Hash parameter accepts any object through Ruby's conversion protocols (to_str,to_int, ...). That made both overloads look equally good, although calling either one could only fail.Fix. The resolver now ranks each argument by MRI's implicit conversions, asking the argument's class the same way the conversion itself would (
ProtocolConversionAction.CanConvert):to_str,to_int,to_aryorto_hashconversion, an argument counts as convertible if its class defines the method, or overridesrespond_to?ormethod_missing.Numericcounts, as with MRI'srb_to_float. So Strings,Timeandnildo not pull a call onto a lossySystem::Singleoverload.TypeError: no implicit conversion of Time into String or Integer.These cases stay
AmbiguousMatchException:method_missingorrespond_to?;Pair(Int32, Int32)vsPair(Double, Double)called with ato_int-only object and1.5;Library methods are not affected.
The decision depends on the argument's class, so the rule adds a class test for the argument. It is a condition when the argument comes from a splat. The method lookups are registered the same way
BuildConversionregisters them. Defining, removing or mixing in a conversion method, or adding a singleton method, makes the call site re-bind.Behaviour changes
Only calls that raised
AmbiguousMatchExceptionon master behave differently:maxandminwith explicitFuncselectors, and other generic overload pairs that C# resolves, now bind to the overload C# picks.System::Math.Max(Time.at(100), 1)givesno implicit conversion of Time into Integer or Float.StringBuilder.new(obj_with_to_str)builds from the string;StringBuilder.new(Rational(5, 2)), aBigDecimal, or ato_intobject picks the capacity constructor. This matches MRI, whereNumeric#to_intmakes these implicit Integers (Array.new(Rational(5, 2))).Calls that bound on master bind the same way. Single-overload behaviour is unchanged, e.g. a lone Float parameter still calls
to_f.Known limits:
Testing
New
Src/IronRuby.Tests/Runtime/ClrOverloadResolutionTests.cs:ClrOverloadGenericTieBreak1: LINQ-shaped fixtures,TieBreakBox<T>, non-generic beats generic, a cross pair stays ambiguous;ClrOverloadGenericTieBreak2: real LINQ max, min, sum, average, and the block hint;ClrOverloadGenericTieBreak3: params and default-value forms; each expected result is what the C# compiler picks;ClrOverloadProtocolConversions1: the String/Int32 pair, the 3-way overload, Float vs Numeric, positional, splatted and keyword arguments;ClrOverloadProtocolConversions2: the rule cache re-binds on class and singleton changes.All five fail against master's runtime and pass on this branch.
C# suite, clean rebuild of IronRuby.Tests (net8.0): 0 failures (828 tests, 5 new).
spec/language: 2934 examples, 5581 expectations, 0 failures, 0 errors, the same as master.spec dirs, branch vs master, identical and all 0F/0E:
CI steps:
Src/Samples/Linq/101samples.rb) output matches101samples.expected, exit 0;Util/check-no-executables.rb: 10105 files, OK.Generic tie-break checked against Roslyn on 20+ overload pairs, including three that C# rejects with CS0121. IronRuby picks what C# picks, and the CS0121 pairs stay
AmbiguousMatchException.Overload batteries diffed between master and this branch. They cover BCL Math, StringBuilder, Convert, List, Dictionary, TimeSpan, Path and Enum, plus fixture pairs, with protocol objects, Strings, Time, nil, Rational, BigDecimal, Symbols, Arrays and Hashes, passed positionally, splatted and by keyword. Every changed line was an
AmbiguousMatchExceptionon master.Rule-cache batteries (shared vs fresh call sites across class redefinition,
method_missing,respond_to?,extend): 0 mismatches.Not run: the Windows-only Notepad sample, and a net10.0 build (only net8.0 was built).
🤖 Generated with Claude Code