Skip to content

Commit 9adda8f

Browse files
committed
Fix overload oscillation for generic type arguments, tuples, and hashes
Extend the uninformative-args bail-out in resolve_overloads to recursively check type parameter vertices (e.g., Array[T], Hash[K,V], tuples). Previously only top-level empty vertices and splat array element vertices were checked, missing cases like Foo.f([@x]) where @x is empty and overloads differ in the element type. To avoid false bail-outs for legitimate empty containers (e.g., {} passed to Hash#merge!), the recursive check is only applied when overloads actually differ in their positional parameter types. Overloads that differ only in blocks or keywords use the simpler top-level empty check.
1 parent 463e359 commit 9adda8f

4 files changed

Lines changed: 134 additions & 22 deletions

File tree

lib/typeprof/core/graph/box.rb

Lines changed: 69 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -255,29 +255,21 @@ def resolve_overloads(changes, genv, node, param_map, a_args, ret, &blk)
255255
# If any positional argument has no type information, we cannot
256256
# determine which overload to select. Return silently (untyped)
257257
# rather than attempting to match. This prevents oscillation in
258-
# cyclic cases like @x = Foo.transform(@x), and avoids false
259-
# "failed to resolve overloads" diagnostics for untyped arguments.
260-
# We still set up dependency edges so the box re-runs when the
261-
# empty arguments later receive types.
258+
# cyclic cases and avoids false "failed to resolve overloads"
259+
# diagnostics for untyped arguments.
262260
#
263-
# For splat arguments, the positional vertex itself holds Array
264-
# types (non-empty), but the array *element* vertex may be empty.
265-
# The same oscillation occurs when match_arguments? extracts
266-
# elements via get_rest_args and the universal typecheck on the
267-
# flattened element list fails due to conflicting array sources.
268-
# We detect this by checking element vertices of splatted arrays.
269-
has_uninformative_args = a_args.positionals.any? {|vtx| vtx.types.empty? }
270-
unless has_uninformative_args
271-
a_args.positionals.each_with_index do |vtx, i|
272-
next unless a_args.splat_flags[i]
273-
vtx.each_type do |ty|
274-
base = ty.base_type(genv)
275-
if base.is_a?(Type::Instance) && base.mod == genv.mod_ary && base.args[0]
276-
has_uninformative_args = true if base.args[0].types.empty?
277-
end
278-
end
279-
break if has_uninformative_args
280-
end
261+
# Top-level empty vertices are always uninformative. For type
262+
# parameter vertices (e.g., Array[T], Hash[K,V], tuples), we
263+
# only recurse when overloads differ in their positional parameter
264+
# types -- otherwise empty type params (like those of `{}`) cannot
265+
# cause oscillation and should not trigger bail-out.
266+
overloads_differ_in_positionals = !@method_types.each_cons(2).all? {|mt1, mt2|
267+
positionals_match?(mt1, mt2)
268+
}
269+
has_uninformative_args = if overloads_differ_in_positionals
270+
a_args.positionals.any? {|vtx| vertex_uninformative?(genv, vtx) }
271+
else
272+
a_args.positionals.any? {|vtx| vtx.types.empty? }
281273
end
282274
if has_uninformative_args
283275
a_args.positionals.each do |vtx|
@@ -298,6 +290,61 @@ def resolve_overloads(changes, genv, node, param_map, a_args, ret, &blk)
298290
end
299291
end
300292

293+
def vertex_uninformative?(genv, vtx, depth = 0)
294+
return true if vtx.types.empty?
295+
return false if depth > 3
296+
vtx.each_type do |ty|
297+
base = ty.base_type(genv)
298+
next unless base.is_a?(Type::Instance) && !base.args.empty?
299+
base.args.each do |arg_vtx|
300+
return true if arg_vtx && vertex_uninformative?(genv, arg_vtx, depth + 1)
301+
end
302+
end
303+
false
304+
end
305+
306+
# Check if two method types have structurally identical positional
307+
# parameter types (req, opt, rest).
308+
def positionals_match?(mt1, mt2)
309+
return false unless mt1.req_positionals.size == mt2.req_positionals.size
310+
return false unless mt1.opt_positionals.size == mt2.opt_positionals.size
311+
return false unless mt1.rest_positionals.nil? == mt2.rest_positionals.nil?
312+
mt1.req_positionals.zip(mt2.req_positionals).all? {|a, b| sig_types_match?(a, b) } &&
313+
mt1.opt_positionals.zip(mt2.opt_positionals).all? {|a, b| sig_types_match?(a, b) } &&
314+
(mt1.rest_positionals.nil? || sig_types_match?(mt1.rest_positionals, mt2.rest_positionals))
315+
end
316+
317+
# Structural equality check for two SigTyNode objects.
318+
def sig_types_match?(a, b)
319+
return false unless a.class == b.class
320+
case a
321+
when AST::SigTyInstanceNode, AST::SigTyInterfaceNode
322+
a.cpath == b.cpath &&
323+
a.args.size == b.args.size &&
324+
a.args.zip(b.args).all? {|x, y| sig_types_match?(x, y) }
325+
when AST::SigTySingletonNode
326+
a.cpath == b.cpath
327+
when AST::SigTyTupleNode, AST::SigTyUnionNode, AST::SigTyIntersectionNode
328+
a.types.size == b.types.size &&
329+
a.types.zip(b.types).all? {|x, y| sig_types_match?(x, y) }
330+
when AST::SigTyRecordNode
331+
a.fields.size == b.fields.size &&
332+
a.fields.all? {|k, v| b.fields[k] && sig_types_match?(v, b.fields[k]) }
333+
when AST::SigTyOptionalNode, AST::SigTyProcNode
334+
sig_types_match?(a.type, b.type)
335+
when AST::SigTyVarNode
336+
a.var == b.var
337+
when AST::SigTyLiteralNode
338+
a.lit == b.lit
339+
when AST::SigTyAliasNode
340+
a.cpath == b.cpath && a.name == b.name &&
341+
a.args.size == b.args.size &&
342+
a.args.zip(b.args).all? {|x, y| sig_types_match?(x, y) }
343+
else
344+
true # Leaf types (bool, nil, self, void, untyped, etc.)
345+
end
346+
end
347+
301348
def show
302349
@method_types.map do |method_type|
303350
args = []
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
## update: test.rbs
2+
class Foo
3+
def self.f: (Array[Integer]) -> String | (Array[String]) -> Symbol
4+
end
5+
6+
## update: test.rb
7+
# Generic type arguments in overload selection cause oscillation.
8+
#
9+
# typecheck_for_module (sig_type.rb) recursively checks type parameter
10+
# vertices. When an element vertex is empty, typecheck returns true
11+
# (via !found_any), making all overloads match. The resulting disjoint
12+
# return types feed back and cause the element types to oscillate.
13+
#
14+
# Variants that exhibit the same issue:
15+
# - Hash[Symbol, Integer] vs Hash[Symbol, String]
16+
# - Array[Array[Integer]] vs Array[Array[String]]
17+
# - Custom generic: Box[Integer] vs Box[String]
18+
# - Tuple: [Integer] vs [String]
19+
def check
20+
@x = Foo.f([@x])
21+
end
22+
23+
## assert
24+
class Object
25+
def check: -> untyped
26+
end
27+
28+
## diagnostics
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
## update: test.rbs
2+
class Foo
3+
def self.f: (Hash[Symbol, Integer]) -> String | (Hash[Symbol, String]) -> Symbol
4+
end
5+
6+
## update: test.rb
7+
# Hash value type argument causes overload oscillation via the same
8+
# mechanism: empty value type vertex in typecheck_for_module.
9+
def check
10+
@x = Foo.f({ a: @x })
11+
end
12+
13+
## assert
14+
class Object
15+
def check: -> untyped
16+
end
17+
18+
## diagnostics
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
## update: test.rbs
2+
class Foo
3+
def self.f: ([Integer]) -> String | ([String]) -> Symbol
4+
end
5+
6+
## update: test.rb
7+
# Tuple element typecheck causes oscillation via the same mechanism
8+
# as generic type argument oscillation: empty element vertex makes
9+
# typecheck return true for all overloads.
10+
def check
11+
@x = Foo.f([@x])
12+
end
13+
14+
## assert
15+
class Object
16+
def check: -> untyped
17+
end
18+
19+
## diagnostics

0 commit comments

Comments
 (0)