Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recursive modules do not correctly handle class inheritance #6491

Closed
vicuna opened this issue Jul 19, 2014 · 6 comments
Closed

Recursive modules do not correctly handle class inheritance #6491

vicuna opened this issue Jul 19, 2014 · 6 comments

Comments

@vicuna
Copy link

vicuna commented Jul 19, 2014

Original bug ID: 6491
Reporter: @lpw25
Status: acknowledged (set by @damiendoligez on 2014-07-30T20:39:30Z)
Resolution: open
Priority: normal
Severity: feature
Target version: later
Category: typing
Tags: recmod
Monitored by: @gasche

Bug description

Recursive modules can contain classes or class types which inherit from each other. However, since inheritance is done by copying the methods and values, the information only passes through one layer of inheritance. For example the x method is missing from Baz.c in the following:

# module rec Foo : sig class type c = object method x : int end end = Foo
  and Bar : sig class type c = object inherit Foo.c end end = Bar
  and Baz : sig class type c = object inherit Bar.c end end = Baz;;
    module rec Foo : sig class type c = object method x : int end end
and Bar : sig class type c = object method x : int end end
and Baz : sig class type c = object  end end

Module types would suffer from the same problem, but instead it gives an error:

# module rec Foo : sig module type c = sig val x : int end end = Foo
  and Bar : sig module type c = sig include Foo.c end end = Bar
  and Baz : sig module type c = sig include Bar.c end end = Baz;;
    Characters 111-116:
    and Bar : sig module type c = sig include Foo.c end end = Bar
                                              ^^^^^
Error: Illegal recursive module reference

The same could be achieved for inheritance by calling find_class_type on any inherited class types during approx_class.

@github-actions
Copy link

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

@github-actions github-actions bot added the Stale label May 11, 2020
@garrigue
Copy link
Contributor

  • There is no specification of what recursive modules should do w.r.t. class inheritance.
  • I don't think we are going to the suggested behaviour anytime soon.
    So I would suggest closing this one.

@lpw25
Copy link
Contributor

lpw25 commented May 11, 2020

The same could be achieved for inheritance by calling find_class_type on any inherited class types during approx_class.

I think this would fix the issue and probably break no programs in practice. Should I make a quick PR to do this? or do you think there will be some issue with this approach.

@garrigue
Copy link
Contributor

You mean, having an error rather than inferring a wrong type?
In this case I think this is fine indeed. Just have to check that this doesn't break existing programs, but I agree that would be very surprising.

@github-actions
Copy link

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

@github-actions github-actions bot added the Stale label Jul 21, 2021
@garrigue garrigue removed the Stale label Jul 21, 2021
@github-actions
Copy link

github-actions bot commented Aug 1, 2022

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

@github-actions github-actions bot added the Stale label Aug 1, 2022
gasche added a commit that referenced this issue Dec 7, 2022
Prohibit using classes through recursive modules (fixes #6491)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants