Ancestors of module metaclasses are inconsistent

[HertzDevil]

Modules that don't include any other types have no ancestors at all, not even Object:

module A; end
module B(T); end

{% A <= Object %}        # => false
{% B <= Object %}        # => false
{% B(Int32) <= Object %} # => false

{% A.ancestors %}        # => [] of ::NoReturn
{% B.ancestors %}        # => [] of ::NoReturn
{% B(Int32).ancestors %} # => [] of ::NoReturn

It follows that Class should be the sole direct parent of those modules' metaclasses. Instead, the metaclasses of generic module instances contain Object.class in their hierarchy:

{% A.class <= Object.class %}        # => false
{% B.class <= Object.class %}        # => false
{% B(Int32).class <= Object.class %} # => true

{% A.class.ancestors %}        # => [Class, Value, Object]
{% B.class.ancestors %}        # => [Class, Value, Object]
{% B(Int32).class.ancestors %} # => [Object.class, Class, Value, Object]

This is unexpected, because B(Int32) still isn't a class. This hierarchy directly affects class method lookup. The standard library seems to rely on this in at least one place:

crystal/spec/std/json/serialization_spec.cr

Lines 280 to 283 in b35c8c9

	Enum::ValueConverter(JSONSpecEnum).from_json("0").should eq(JSONSpecEnum::Zero)
	Enum::ValueConverter(JSONSpecEnum).from_json("1").should eq(JSONSpecEnum::One)
	Enum::ValueConverter(JSONSpecEnum).from_json("2").should eq(JSONSpecEnum::Two)
	Enum::ValueConverter(JSONSpecEnum).from_json("3").should eq(JSONSpecEnum::OneHundred)

Where from_json is defined as:

class Object
  def self.from_json(string_or_io)
    parser = JSON::PullParser.new(string_or_io)
    new parser
  end
end

module Enum::ValueConverter(T)
  def self.from_json(pull : JSON::PullParser) : T
    T.from_value?(pull.read_int) || pull.raise "Unknown enum #{T} value: #{pull.int_value}"
  end
end

A String is not a JSON::PullParser, so the overload in Enum::ValueConverter.class fails to match and the one in Object.class is chosen. Now suppose that we specialize Enum::ValueConverter(JSONSpecEnum) ourself. The following no longer compiles (as it shouldn't):

# `Object.from_json` is defined as above

module MyConverter
  def self.from_json(pull : JSON::PullParser) : JSONSpecEnum
    JSONSpecEnum.from_value?(pull.read_int) || pull.raise "Unknown enum #{JSONSpecEnum} value: #{pull.int_value}"
  end
end

MyConverter.from_json("0") # Error: no overload matches 'MyConverter.from_json' with type String

We should remove Object.class from the ancestors of those generic module instance metaclasses. Even in that case, the same behaviour can be achieved with either of the following:

# extract methods on `Object.class` to an extended module
module JSON::ConverterExtensions
  def from_json(string_or_io)
    parser = JSON::PullParser.new(string_or_io)
    new parser
  end
end

class Object; extend JSON::ConverterExtensions; end
module Enum::ValueConverter(T); extend JSON::ConverterExtensions; end
module MyConverter; extend JSON::ConverterExtensions; end

# define the method on `Class` instead, since any instance or class method on
# `Class` is also a class method for every metaclass
class Class
  def from_json(string_or_io)
    parser = JSON::PullParser.new(string_or_io)
    new parser
  end
end

[HertzDevil]

The Object.class appears because of Crystal::MetaclassType#replace_type_parameters:

class Crystal::MetaclassType
  def replace_type_parameters(instance)
    instance_type.replace_type_parameters(instance).metaclass
  end
end

class Crystal::GenericModuleInstanceMetaclassType
  def parents
    instance_type.generic_type.metaclass.parents.try &.map do |parent|
      parent.replace_type_parameters(instance_type)
    end
  end
end

The direct parents of B(Int32).class are the results of substituting T = Int32 into B.class's direct parents, which are just Class. But Class's instance type is currently Object, so performing any type parameter substitution on Class will produce Object.class.

Metaclasses of generic class instances are immune because they must have a superclass (Reference or Value, neither of which is generic).

[straight-shoota]

The assessment sounds good and the proposed solution seems convincing.

However, I fear this might be too big a breaking change in regards to how class methods on Object are currently utilized.

As I understand it, Object.from_x methods (which are used quite a lot) would not be directly affected. But use cases like Enum::ValueConverter(T).from_json which expect to have Object.from_json in the same scope would break. I'm not sure how much that would be used, but it feels too much of a breaking change.
I don't think we can do that before 2.0. If we can avoid it altogether, that would even be better.

I personally don't like the idea of making such methods instance methods of Class. I guess it makes sense, but still, it doesn't look right to me.

Could it be an option to implicitly define class methods of Object as instance methods of Class? Looking from a user's perspective, I can't really see why we would need to differentiate the namespaces Object.class and Class. Not the types, just the namespaces where people define methods like Object.from_json.
This is obviously more complex. But if we can avoid a breaking change with regards to the defintion of methods that are supposed to be available on all types, I think that should be worth it.

[HertzDevil]

The method in question is actually also related to #5697, so if we succeed in debloating Object's interface then there would be less friction here, and I think that is the way to go.

[HertzDevil]

Another problem I just discovered is that generic module instances implicitly inherit Object.allocate because of the current hierarchy:

module Foo(T)
end

puts Foo(Int32).allocate # can't execute `obj.to_s(self)` at /usr/lib/crystal/io.cr:174:5: `obj.to_s(self)` has no type

module Foo(T)
end

class Bar
  include Foo(Int32)
  
  @x = 1
end

puts Foo(Int32).allocate # => #<Bar:0x7f308c5a1ea0>

module Foo(T)
end

class Bar
  include Foo(Int32)
end

class Baz
  include Foo(Int32)
end

Foo(Int32).allocate # BUG: called llvm_struct_type for (Bar | Baz)

.allocate is properly undefined for uninstantiated generic module metaclasses:

module Foo(T)
end

Foo.allocate # Error: undefined method 'allocate' for Foo(T):Module (modules cannot be instantiated)