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

Jump to bottom

Cty_constr not handled correctly by Subst #6650

Closed
vicuna opened this issue Nov 8, 2014 · 3 comments
Closed

Cty_constr not handled correctly by Subst #6650

vicuna opened this issue Nov 8, 2014 · 3 comments
Assignees
@garrigue
Labels
bug typing

Comments

@vicuna
Copy link

vicuna commented Nov 8, 2014

Original bug ID: 6650
Reporter: @lpw25
Assigned to: @garrigue
Status: closed (set by @xavierleroy on 2016-12-07T10:36:54Z)
Resolution: fixed
Priority: normal
Severity: minor
Fixed in version: 4.03.0+dev / +beta1
Category: typing
Monitored by: @gasche

Bug description

Cty_constr contains a path which is either to a class or a class type, but Subst treats them as type paths since it does not support substituting class or class type paths. This means that when Subst.signature renames all bound identifiers the class/class type paths are not properly updated.

Since Cty_constr is only consumed by the type printer, which only uses the name of the path, this doesn't cause any actual bugs. However, it is very annoying for tools which consume .cmi files.

I see a number of possible solutions for this:

  1. Add support for substituting class and class type paths to Subst.

  2. Cheat and add the class and class type idents as type substitutions in Subst.rename_bound_idents.

  3. Don't rename classes and class types (and values and extension constructors) in rename_bound_idents. These idents have no semantic meaning, and are only used for printing, so there is really no need to rename them.

  4. Put the corresponding object type path in Cty_constr (from cty_path) instead of the class or class type path. Then regular substitution of the object types will keep the paths up-to-date.

Personally, I think 3 would be the best solution.

Note that (if we don't do 4) it would also be a good idea to ensure that Cty_constr always contains a class type path rather than sometimes containing a class path.

Also note that the information in csig_inher is derived from Cty_constr, so it is also not correctly maintained by Subst.
@vicuna
Copy link
Author

vicuna commented Nov 8, 2014

Comment author: @lpw25

Actually I can get this to cause a bug in the printer:

# module type S = sig
    class type c = object method m : int end
    module M : sig
      class type d = c
    end
  end;;

module type S =
  sig
    class type c = object method m : int end
    module M : sig class type d = c end
  end

# module F (X : S) = X.M;;

module F : functor (X : S) -> sig class type d = c end

See that F produces a module with a class type d that is equal to a non-existent class type c.

@vicuna
Copy link
Author

vicuna commented Nov 8, 2014

Comment author: @lpw25

The above bug would not be fixed by option 3 (although I think that is a good idea anyway because it simplifies the case of idents for values and type extensions).

So I think that one of 1 or 4 should be implemented as well, probably 4 since 1 would increase the size of all Subst.t's just to improve printing of class types.

@vicuna
Copy link
Author

vicuna commented Nov 10, 2014

Comment author: @garrigue

Fixed in trunk at revision 15574.
Adopted solution (2): while this may be seen as cheating, the behavior is exactly the same for class/class types and types.

@vicuna vicuna closed this as completed Dec 7, 2016
@vicuna vicuna added the typing label Mar 14, 2019
@vicuna vicuna added the bug label Mar 20, 2019
@gasche gasche mentioned this issue Nov 14, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@garrigue garrigue

Labels
bug typing
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@garrigue @vicuna