Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0005545OCamlOCaml typingpublic2012-03-20 10:232015-03-27 02:26
Assigned Togarrigue 
PrioritylowSeveritytweakReproducibilityhave not tried
PlatformOSOS Version
Product Version 
Target Version4.02.3+devFixed in Version4.03.0+dev 
Summary0005545: Type annotations on methods cannot control the choice of abbreviation
DescriptionWhen using type abbreviations, annotations usually allow the programmer to pick an explicit choice for naming the type of a sub-expression:

type foo = int

let foo =
  let x : foo = 10 in
  let y : int = x in
  (x, y)

--> val foo : foo * int

For objects, this does not work so well:

class o =
    method x : foo = 10
    method y : int = this # x

--> class o : object method x : foo method y : foo end

Note that despite the type annotation on method y, the inferred type is "foo", not "int". We can get "int" by putting the annotation on the body:

class o =
    method x : foo = 10
    method y = (this # x : int)

--> class o : object method x : foo method y : int end

However, this does not work in general. In the following example, the definition of method y "pollutes" the type inferred for method x, and I cannot see a way to force the type-checker to report type "int" (without touching the definition for y):

class o =
    method x : int = (10 : int)
    method y = (this # x : foo)

--> class o : object method x : foo method y : foo end

TagsNo tags attached.
Attached Filesdiff file icon generalize_spine.diff [^] (2,463 bytes) 2015-03-25 02:24 [Show Content]
diff file icon patch_5545.diff [^] (6,709 bytes) 2015-03-25 12:28 [Show Content]

- Relationships
related to 0005619closedgarrigue Uncuaght CType.Unify exception 

-  Notes
garrigue (manager)
2012-03-21 08:30

A solution to this problem is to use the -principal option, as it duplicates enough nodes to avoid this kind of interferences.
Whether this should be done in all cases is another question. Originally, the unification algorithm was designed so as to propagate abbreviations. You are suggesting that this behavior is wrong in some cases.
Note that here int and foo are supposed to be strictly equivalent. The semantics of ocaml do not distinguish between the two.
The only place where abbreviations are guaranteed to be kept is in interfaces. Indeed, if foo is made abstract there, they would be different.
frisch (developer)
2012-03-21 10:08

> A solution to this problem is to use the -principal option

I tried that, but even for the last example, the call to "this # x" pollutes the type for method x with -principal.

> Originally, the unification algorithm was designed so as to propagate abbreviations.

I'm surprised by the difference between standard let definitions, where type annotations seem to be enough to have the type-checker picks the desired name, and typing of objects.
frisch (developer)
2015-03-20 13:41

Note: since commit 12289, method type are unshared in -principal mode.
garrigue (manager)
2015-03-25 02:28

generalize_spine.diff attempts to do the type separation cleanly, without waking up PR#5619.
I.e. only use generalized types for methods called through self.
This way it can be enabled even in non-principal mode.

Does it do what you want?
garrigue (manager)
2015-03-25 02:30

For the record, enabling it for normal mode has no noticeable performance cost, but increases the size of the cmi's for ocamldoc by 2% (476KB -> 486KB).
frisch (developer)
2015-03-25 12:32

Thanks Jacques!

I've tested the patch and it works as expected. I've uploaded a diff that extends your patch with a note in Changes, a new test testsuite/tests/typing-objects/, and updated expected results for two tests:

 - testsuite/tests/typing-objects/

   The result is now the same with and without -principal, and it seems correct.

 - testsuite/tests/typing-poly/

   A new warning ("this use of a polymorphic method is not principal") is triggered after the change. I don't know if this is correct or not. Can you have a look?
frisch (developer)
2015-03-25 17:32

Something else: should we done something about the call to generalize_spine in Typeclass.initial_env? (It is currently done only in principal mode.)
garrigue (manager)
2015-03-26 00:31

Committed patch_5545.diff in trunk at revision 15964.

Concerning the use of generalize_spine in initial_env, its role is different.
First, it acts on a fresh type, so it is just an abbreviation for begin_def / end_def / generalize_structure.
Next, its role (in the non-principal mode) would be to ensure that the type of the constructor is left unchanged. However this doesn't look like the right place to do that:

  # class c (x : int) = object method x = x method y = new c (1 : foo) end;;
  class c : foo -> object method x : int method y : c end

in both normal and principal mode.
This is similar to recursive definitions:

  # let rec f (x : int) = if x = 0 then x else f (1 : foo);;
  val f : foo -> int = <fun>
frisch (developer)
2015-03-26 10:14


What about the new warning in testsuite/tests/typing-poly/
garrigue (manager)
2015-03-27 02:25

It is a result of the "cleaner" approach: only methods of self itself are generalized, not those of objects with the same type as self.
I didn't come up with a repro case, but I'm afraid the old approach could lead to bugs such as 5619.

- Issue History
Date Modified Username Field Change
2012-03-20 10:23 frisch New Issue
2012-03-21 08:30 garrigue Note Added: 0007119
2012-03-21 08:36 garrigue Assigned To => garrigue
2012-03-21 08:36 garrigue Status new => assigned
2012-03-21 10:08 frisch Note Added: 0007120
2012-09-06 16:43 doligez Target Version => 4.00.1+dev
2012-09-21 13:29 doligez Target Version 4.00.1+dev => 4.01.0+dev
2013-08-19 14:30 doligez Target Version 4.01.0+dev => 4.01.1+dev
2014-05-25 20:20 doligez Target Version 4.01.1+dev => 4.02.0+dev
2014-07-16 20:49 doligez Target Version 4.02.0+dev => 4.02.1+dev
2014-09-04 00:25 doligez Target Version 4.02.1+dev => undecided
2014-09-22 22:10 doligez Target Version undecided => 4.02.2+dev / +rc1
2015-02-26 22:08 doligez Target Version 4.02.2+dev / +rc1 => 4.02.3+dev
2015-03-20 13:41 frisch Note Added: 0013521
2015-03-25 02:24 garrigue File Added: generalize_spine.diff
2015-03-25 02:28 garrigue Note Added: 0013541
2015-03-25 02:28 garrigue Status assigned => feedback
2015-03-25 02:30 garrigue Note Added: 0013542
2015-03-25 12:28 frisch File Added: patch_5545.diff
2015-03-25 12:32 frisch Note Added: 0013545
2015-03-25 12:32 frisch Status feedback => assigned
2015-03-25 17:32 frisch Note Added: 0013549
2015-03-26 00:31 garrigue Note Added: 0013553
2015-03-26 00:31 garrigue Status assigned => resolved
2015-03-26 00:31 garrigue Fixed in Version => 4.03.0+dev
2015-03-26 00:31 garrigue Resolution open => fixed
2015-03-26 10:14 frisch Note Added: 0013556
2015-03-27 02:25 garrigue Note Added: 0013572
2015-03-27 02:26 garrigue Relationship added related to 0005619

Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker