One problem I am aware of is that the following code

class type t1 = object method f : int end
type t = < #t1 >

will give you error message: "Error: The type #t1 is not an object type".

Not sure if it makes sense to support this pattern though.

Update (04/06):

class type t1 = object method f : int end
type t = < t1 >

is supported now

this looks really useful, structural typing are very useful for projects like JSOO and BuckleScript. some bikeshedding about syntax, g looks punning, maybe we can it more explicit:

type t = < x : int ; y : int; inherit g >

some bikeshedding about syntax, g looks punning, maybe we can it more explicit

There's no punning anywhere in the type algebra so I doubt it will confuse people. Whereas using inherit will likely perpetuate people's misunderstandings about the relation between class and object types. Inheritance is about inheriting behaviour, whilst it can cause the object type associated with a class to be extended it is not the same thing and I don't think it should use the same syntax.

Inheritance is about inheriting behaviour

inherit is also used in class types, where it doesn't imply any inheritance of behavior:

class type c = object method x: int end
class type d = object inherit c method y: int end

So maybe it's more accurate to say that inherit is associated with nominal typing rather than structural, and we don't want to confuse the two?

inherit is also used in class types, where it doesn't imply any inheritance of behavior

I know and it's kind of wrong there too. The class system has quite a few things like this which subtly confuse different concepts, and I've always thought that it was one of the reasons people have a lot of trouble understanding it. I'd rather not add one more to the list, even if it would not be the worst offender.

I like the syntax that @objmagic has implemented because it parallels the syntax for polymorphic variants:

  type d = [`A of int | `B of int | c ]
  type d = < a :  int ;  b :  int ; c >

Rather less importantly, it's also reminiscent of the functional update syntax for records:

  { r with x = a ; y = b }
  < r ;    x : a ; y : b >

it parallels the syntax for polymorphic variants

This is the killer argument, and I think it concludes the syntactic discussion.

Any takers for an actual code review? :-)

@yallop Thanks! These are good tests. I fixed one bug when handling empty object type expression (7743e96)

@yallop would you like to also have a look at my question at #1118 (comment)?

@Octachron would you like to help review change on odoc? I was thinking to add some tests for that but I do not know how to do that.

I've been thinking of adding this features a number of times.
I always stopped short of doing it, because one can already do that using class types.
So this is a "political decision".
I agree that re-establishing the symmetry with polymorphic variants would be nice, but we need to discuss that on caml-devel.

For the implementation, I'm a bit concerned by the way transl_fields is implemented.
This is clearly not symmetrical with polymorphic variants: rather than unification, fields are copied, and as a result repeated declaration of a field is not allowed. This may be a problem with multiple inheritance. I think that keeping things in line with polymorphic variants would be better; actually the restriction on duplicate field declarations doesn't seem relevant for object types.

@garrigue I agree that implementation should be symmetrical with polymorphic variants' implementation. However I don't understand your statement "the restriction on duplicate field declarations doesn't seem relevant for object types". Do you mean type t = <a: int; a: string> (not allowed now) should give you type t = <a:string>, following the rule "last wins" in class?

@objmagic No, this should give you a unification error, like for polymorphic variants. We're talking about objects, not modules.

@garrigue Thank you very much for the comments! I have updated my PR to use unification instead (d459674). If I understand your words correctly, I removed "restriction on duplicate field declarations" (as a side-effect of using unification)

@objmagic On the ocamldoc side, if you want to add tests, there are two kind of tests available in the testsuite. First, basic type printing tests are located in testsuite/tests/tool-ocamldoc, in particular testsuite/tests/tool-ocamldoc/t01.ml already tests the printing of object types.
Then, the various backend are tested in

• tool-ocamldoc-2: latex
• tool-ocamldoc-html: html
• tool-ocamldoc-man: man pages

Adding an Object_types.mli test file in at least one of the back-end tests would probably be a good idea.

Looking at the code and ocamldoc output, currently the information about inherited object type is not used by ocamldoc. Consequently

type t = <a:int (** comment for a *) ; b:int (** comment for b*) >
type r = < t (**comment for inherited t*) ; c:int (** comment for c*) >

is printed as

type t = <a:int (** comment for a *) ; b:int (** comment for b*) >
type r = < a:int; b:int ; c:int (** comment for c*) >

Ideally, I think it would be better to not expand inherited fields in the documentation and add the possibility to comment on inherited fields. However, these are quality of life improvements, and could be the object of a subsequent PR, since ocamldoc is neither crashing nor missing object fields in presence of inherited fields.

One problem I am aware of is that the following code

class type t1 = object method f : int end
type t = < #t1 >

will give you error message: "Error: The type #t1 is not an object type".

That's the correct behaviour, since #t1 is not a closed object type. The following should be accepted, though (and apparently is):

class type t1 = object method f : int end
type t = < t1 >

@garrigue Ping again. What do you or caml-dev think about this PR?

Should check it in more detail, but the feature is straightforward and the behavior reasonable.
The only question left is whether we want to include something that is already possible using class type. I suppose a strong argument is from Javascript users, who use object types but not classes.

Ultimately, since this is a new feature, I think we need a consensus: discuss it on caml-devel or at a physical meeting.

If the code is good (and I'm not competent to review it), I would be in favor of taking the feature in. It does not add complexity (especially given that polymorphic variants already support it), and I don't find the redundancy with class types an issue here -- and I have used object types myself in situations where this would have been handy.

The feature seems generally reasonable to me.

What is the desired behavior in case two methods have the same name?
Do we get a type error? a unification of the two types of the two methods? the more recent method hiding the older one?

Also, considering that repeated method declarations are currently not allowed:

# type t = < m: int; m: int >;;
Error: This is the second method `m' of this object type.
       Multiple occurences are not allowed.

it might make sense to allow this if it is allowed indirectly (via inheritance).

@fpottier Thanks for your review.

it might make sense to allow this

I agree. And I'd like to hear other people's opinion of your comments as well.

When both inherited types have methods with the same names I think unifying the types of the methods is the best approach.

OCaml already performs unification when two inherited polymorphic variable types have the same tag. For example, in the following code 'a and 'b are unified, since the type annotation on x inherits the types [`M of 'a] and [`M of 'b]:

# fun (_ : ('a * 'b)) x -> (x : [ [`M of 'a] | [`M of 'b] ]);;
- : 'a * 'a -> [ `M of 'a ] -> [ `M of 'a ] = <fun>

With this PR the behaviour for object types is similar, unifying the types of the methods:

# fun (_ : ('a * 'b)) x -> (x : < <m: 'a> ; <m: 'b> >);;
- : 'a * 'a -> < m : 'a > -> < m : 'a > = <fun>

This PR also relaxes the constraint that currently prohibits having multiple occurrences of methods with the same name. In OCaml 4.05.0:

# type 'a t = [`M of 'a | `M of int];;
type 'a t = [ `M of 'a ] constraint 'a = int
# type 'a t = <m: 'a ; m: int>;;
Characters 12-28:
  type 'a t = <m: 'a ; m: int>;;
              ^^^^^^^^^^^^^^^^
Error: This is the second method `m' of this object type.
       Multiple occurrences are not allowed.

With this PR:

# type 'a t = <m: 'a ; m: int>;;
type 'a t = < m : 'a > constraint 'a = int

Yes I just realized this has been implemented. I have updated tests.

This PR also relaxes the constraint that currently prohibits having multiple occurrences of methods with the same name.

I'd like to hear what @garrigue has to say about this point.

If we seek symmetry with polymorphic variants, I think it is natural to remove this constraint.

I think that merging earlier is better than later to avoid conflicts. I'm planning to go ahead and merge in a few days if nobody does it before or objects.

I checked that the CI failures are not an issue (they are failure in Camlp4 currently not building under trunk).

@gasche Sounds good. I agree that if this PR looks all good, merge it sooner so that we don't need to deal with more merge conflicts.

I made a mistake when assessing the travis CI results: it is indeed this PR that is breaking Camlp4, and this should be fixed. In the meantime, #1265 proposes to temporarily disable camlp4 building to avoid breaking other PRs as well.

See camlp4/camlp4#123.