From b1248938bfa51d8524f58e85bd9e61b018342899 Mon Sep 17 00:00:00 2001 From: Stephen von Takach Date: Tue, 19 May 2026 22:43:30 +1000 Subject: [PATCH 1/2] Include uninstantiated generic subclasses in virtual metaclass dispatch and type filtering (#11134, #16947, #10831) --- spec/compiler/codegen/class_spec.cr | 45 +++++++++++++++++++ spec/compiler/semantic/is_a_spec.cr | 12 +++++ .../crystal/semantic/method_lookup.cr | 9 +++- .../crystal/semantic/type_intersect.cr | 7 ++- src/compiler/crystal/types.cr | 39 +++++++++++++++- 5 files changed, 107 insertions(+), 5 deletions(-) diff --git a/spec/compiler/codegen/class_spec.cr b/spec/compiler/codegen/class_spec.cr index 79306acdae7c..4b8d10234075 100644 --- a/spec/compiler/codegen/class_spec.cr +++ b/spec/compiler/codegen/class_spec.cr @@ -1165,6 +1165,51 @@ describe "Code gen: class" do CRYSTAL end + it "dispatches virtual metaclass call to uninstantiated generic subclass (#11134)" do + run(<<-CRYSTAL).to_i.should eq(2) + class Foo + def self.foo + 1 + end + end + + class Bar(T) < Foo + def self.foo + 2 + end + end + + class Baz < Foo + def self.foo + 3 + end + end + + Bar.as(Foo.class).foo + CRYSTAL + end + + it "does not segfault when an Array(Module) holds an instance of an uninstantiated generic subclass (#16947)" do + run(<<-CRYSTAL).to_i.should eq(2) + require "prelude" + + module Foo + end + + class GenericFoo(T) + include Foo + end + + class SubGenericFoo(T) < GenericFoo(T) + end + + array = [] of Foo + array << GenericFoo(String).new + array << SubGenericFoo(String).new + array.size + CRYSTAL + end + it "can assign virtual metaclass to virtual metaclass (#3007)" do run(<<-CRYSTAL).to_i.should eq(2) class Foo diff --git a/spec/compiler/semantic/is_a_spec.cr b/spec/compiler/semantic/is_a_spec.cr index f54126b70625..4f9e88cebfb2 100644 --- a/spec/compiler/semantic/is_a_spec.cr +++ b/spec/compiler/semantic/is_a_spec.cr @@ -258,4 +258,16 @@ describe "Semantic: is_a?" do end CRYSTAL end + + it "preserves type info across `is_a?(GenericA) && is_a?(GenericB)` when GenericB <= GenericA (#10831)" do + assert_type(<<-CRYSTAL) { bool } + class A; end + class B(T) < A; end + class C(T) < B(T); end + + x = C(Int32).new.as(A) + # If the chain still produced NoReturn, the call would fail to type. + (x.is_a?(B) && x.is_a?(C)) ? true : false + CRYSTAL + end end diff --git a/src/compiler/crystal/semantic/method_lookup.cr b/src/compiler/crystal/semantic/method_lookup.cr index a38fe0e280f5..79318d7a2e22 100644 --- a/src/compiler/crystal/semantic/method_lookup.cr +++ b/src/compiler/crystal/semantic/method_lookup.cr @@ -428,8 +428,13 @@ module Crystal matches = base_type_matches.matches changes = nil - # Traverse all subtypes - instance_type.subtypes(base_type).each do |subtype| + # Traverse all subtypes. For virtual metaclasses, include uninstantiated + # generic subclasses too — their metaclasses are valid dispatch targets + # (e.g. `Bar.foo` where `class Bar(T) < Foo`), and without them the + # dispatch table is missing a branch and the call hits the unreachable + # at runtime (#11134). + subtypes = virtual_metaclass? ? instance_type.subtypes_including_generic(base_type) : instance_type.subtypes(base_type) + subtypes.each do |subtype| next if is_new && subtype.is_a?(GenericClassInstanceType) && subtype.abstract? subtype_lookup = virtual_lookup(subtype) diff --git a/src/compiler/crystal/semantic/type_intersect.cr b/src/compiler/crystal/semantic/type_intersect.cr index 490503dbc4c1..e72575dce248 100644 --- a/src/compiler/crystal/semantic/type_intersect.cr +++ b/src/compiler/crystal/semantic/type_intersect.cr @@ -214,7 +214,12 @@ module Crystal def self.common_descendent(type1 : GenericClassType, type2 : GenericClassType) return type1 if type1 == type2 - common_descendent_instance_and_generic(type1, type2) + # Try both directions: e.g. for `class C(T) < B(T)`, `common_descendent(B, C)` + # should resolve to `C` because every C is also a B. Without the symmetric + # check, chained type filters like `x.is_a?(B) && x.is_a?(C)` collapse to + # `NoReturn` (#10831). + common_descendent_instance_and_generic(type1, type2) || + common_descendent_instance_and_generic(type2, type1) end def self.common_descendent(type1 : Type, type2 : AliasType) diff --git a/src/compiler/crystal/types.cr b/src/compiler/crystal/types.cr index fed6f8d68f01..efbfc48baa9f 100644 --- a/src/compiler/crystal/types.cr +++ b/src/compiler/crystal/types.cr @@ -3431,7 +3431,7 @@ module Crystal subtypes end - def subtypes(type) + def subtypes(type : Type) subtypes = [] of Type type.subclasses.each do |subclass| collect_subtypes subclass, subtypes @@ -3439,6 +3439,23 @@ module Crystal subtypes end + # Like `subtypes`, but also yields uninstantiated generic subclasses. + # Used for virtual *metaclass* dispatch, where `Bar.foo` is callable + # even when `class Bar(T)` has no instantiation (#11134). + def subtypes_including_generic + subtypes = [] of Type + collect_subtypes_including_generic(base_type, subtypes) + subtypes + end + + def subtypes_including_generic(type : Type) + subtypes = [] of Type + type.subclasses.each do |subclass| + collect_subtypes_including_generic subclass, subtypes + end + subtypes + end + def collect_subtypes(type, subtypes) subtypes << type unless type.is_a?(GenericType) || type.unbound? type.subclasses.each do |subclass| @@ -3446,6 +3463,21 @@ module Crystal end end + def collect_subtypes_including_generic(type, subtypes) + if type.is_a?(GenericType) + # Only include the uninstantiated generic if it has no instantiations: + # those carry the dispatch otherwise. Including the uninstantiated form + # when instantiations exist can fail later if a class method on it + # references the type parameter (e.g. `def self.x; T.foo; end`). + subtypes << type if type.instantiated_types.empty? && !type.unbound? + elsif !type.unbound? + subtypes << type + end + type.subclasses.each do |subclass| + collect_subtypes_including_generic subclass, subtypes + end + end + def lookup_class_var?(name) class_var = @class_vars.try &.[name]? return class_var if class_var @@ -3519,7 +3551,10 @@ module Crystal end def each_concrete_type(&) - instance_type.subtypes.each do |type| + # Include uninstantiated generic subclasses: their metaclasses are + # callable (e.g. `Bar.foo` where `class Bar(T)`) and must have a + # matching branch in virtual dispatch on `Foo+.class` (#11134). + instance_type.subtypes_including_generic.each do |type| yield type.metaclass end end From 54dd7f753656070da7edad892bce269e2a00d02f Mon Sep 17 00:00:00 2001 From: Stephen von Takach Date: Thu, 21 May 2026 09:11:59 +1000 Subject: [PATCH 2/2] Fix spec coverage on PR #16982 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The #16947 codegen spec passes on master, so it is not a real regression test. The full original reproducer cannot be reproduced locally on master (in any build mode), so drop the test rather than keep one that doesn't bisect. The #10831 semantic spec used `assert_type` against `is_a?(...) ? true : false`, which is statically `bool` regardless of the filter result — so the spec passed even with the unsymmetric `common_descendent`. Rewrite as a `run(...).to_b` check that actually exercises runtime narrowing, and correct the misleading comment about `NoReturn` (the expression incorrectly evaluated to `false`). --- spec/compiler/codegen/class_spec.cr | 21 --------------------- spec/compiler/semantic/is_a_spec.cr | 10 ++++++---- 2 files changed, 6 insertions(+), 25 deletions(-) diff --git a/spec/compiler/codegen/class_spec.cr b/spec/compiler/codegen/class_spec.cr index 4b8d10234075..09386be7f3ef 100644 --- a/spec/compiler/codegen/class_spec.cr +++ b/spec/compiler/codegen/class_spec.cr @@ -1189,27 +1189,6 @@ describe "Code gen: class" do CRYSTAL end - it "does not segfault when an Array(Module) holds an instance of an uninstantiated generic subclass (#16947)" do - run(<<-CRYSTAL).to_i.should eq(2) - require "prelude" - - module Foo - end - - class GenericFoo(T) - include Foo - end - - class SubGenericFoo(T) < GenericFoo(T) - end - - array = [] of Foo - array << GenericFoo(String).new - array << SubGenericFoo(String).new - array.size - CRYSTAL - end - it "can assign virtual metaclass to virtual metaclass (#3007)" do run(<<-CRYSTAL).to_i.should eq(2) class Foo diff --git a/spec/compiler/semantic/is_a_spec.cr b/spec/compiler/semantic/is_a_spec.cr index 4f9e88cebfb2..34df1e0302bc 100644 --- a/spec/compiler/semantic/is_a_spec.cr +++ b/spec/compiler/semantic/is_a_spec.cr @@ -259,15 +259,17 @@ describe "Semantic: is_a?" do CRYSTAL end - it "preserves type info across `is_a?(GenericA) && is_a?(GenericB)` when GenericB <= GenericA (#10831)" do - assert_type(<<-CRYSTAL) { bool } + it "matches `is_a?(GenericA) && is_a?(GenericB)` when GenericB <= GenericA (#10831)" do + # Before the fix, `common_descendent(GenericClassType, GenericClassType)` + # only walked one direction, so the chained filter narrowed `x` to `B(T)` + # and `is_a?(C)` returned `false` for a runtime value that's actually a C. + run(<<-CRYSTAL).to_b.should be_true class A; end class B(T) < A; end class C(T) < B(T); end x = C(Int32).new.as(A) - # If the chain still produced NoReturn, the call would fail to type. - (x.is_a?(B) && x.is_a?(C)) ? true : false + x.is_a?(B) && x.is_a?(C) CRYSTAL end end