Looks good.

That would prevent introducing the dot notation for redefinitions of () and [].

I don't see why it would.

# module M = struct type t = () end;;
module M : sig type t = () end
# M.();;
- : M.t = M.()

Oh, so this patch actually implement the dot notations I was thinking of, nevermind then.

@yallop Shouldn't M.() and M.[] be constr_longident (instead of simple_expr) ? My intuition tells me they should behave like any constructor M.Foo, syntax wise.

I think an argument can be made for either constr_longident or simple_expr: M.[] is currently treated as a special case of M.[ exp; ... exp ] (which is a simple_expr), but it could equally well be treated as a variation of [] (which is a constr_longident).

except constr_longident is called inside simple_expr, so putting it inside constr_longident accepts more (valid, since they are accepted with []/()) form. Also, I argue that having M.[] in patterns is a good thing.

I don't see any reason to accept M.[] in patterns for the moment, since [] can't be redefined.

Thinking about this a bit more, the problem with updating constr_longident instead of simple_expr is that it breaks the local open semantics. As it is, M.[e] and M.[] have the same resolution behaviour: bring everything in M into scope, then resolve everything in the local environment. If we were to change constr_longident instead of simple_expr then M.[e] would continue to behave that way, but M.[] would fail unless M defined [].

So I think the patch is better as it is.

Thinking about this a bit more, the problem with updating constr_longident instead of simple_expr is that it breaks the local open semantics. As it is, M.[e] and M.[] have the same resolution behaviour: bring everything in M into scope, then resolve everything in the local environment. If we were to change constr_longident instead of simple_expr then M.[e] would continue to behave that way, but M.[] would fail unless M defined [].

Ok, that's a good argument, even if I personally think we should be allowed to redefined [] anyway.

Before you redefine anything, have a look at the discussion of PR#5936.

Merged in trunk, thanks.

I had one minor complaint with the patch: you wrote an excellent rationale for the change as your pull request text, but it was absent from the commit messages itself. I like good commit messages.