Couldn't just [@@@ellipsis] work here?

Yes, I was too focused on having tests for [@ellipsis.start]...[@ellipsis.end].

This environment change and the one below look like they are likely to conflict with #1209 for no very good reason.

True, I will remove them. (I could also remove all changes to exten.etex since they are more here as an example than anything else.).

Shouldn't you check that left_mark is not active to guarantee the absence of nested ellipses?

Possibly. However, this case is already caught later by check_partition. I will think a bit about this independence problem.

I have added a test in order to catch nested ellipsis early in all cases (which makes the the test for unmatched stop/start simpler).

On reviewing I didn't think much of this assertion, but upon usage I see bugs caused by the fact that it is wrong. For example the following does not behave correctly:

module type DEVICE = sig val draw : picture -> unit[@@ellipsis] end

The ellipsis should elide the whole signature item val draw ..., but it only elides the unit part, because location iterators are called in the wrong order.

The mismatch can be found in the sources of ast_iterator.ml:

    module_type_declaration =
      (fun this {pmtd_name; pmtd_type; pmtd_attributes; pmtd_loc} ->
         iter_loc this pmtd_name;
         iter_opt (this.module_type this) pmtd_type;
         this.attributes this pmtd_attributes;
         this.location this pmtd_loc
);

(My first idea on how to fix it turned out not to work, so I'm not sure of what's the best fix. It may be to special-case the situations in which we support the ellipsis attribute as we can reliably extract locations, and fail on the others, for now.)

I should have indeed checked more carefully this assertion. If you find the fix too time consuming, please do not hesitate to ping me.

In fact I am currently under the impression that there is no nice fix that does not touch ast_iterator (which would be the condition to go into 4.07). I started writing the following

let located_attributed_iterator located_attribute =
  let open Ast_iterator in
  let super = default_iterator in
  let rec this = {
    super with
    structure_item = (fun this ({pstr_loc = loc; pstr_desc = desc} as si) ->
      begin match desc with
        | Pstr_eval (_, attrs) -> located_attributes loc attrs
        | Pstr_extension (_, attrs) -> located_attributes loc attrs
      end;
      super.structure_item this si;
      );

but the idea of having to extend with definition with all the cases under the sun is killing me, and I don't think this is the right approach.

I would be tempted to revert the ellipsis stuff from 4.07, because I don't like the idea of releasing something that we know is broken. Then for trunk we can discuss either:

• fixing the Ast_iterator control flow (but then I realized that some things have attributes but no locations, so we would probably need to change Parsetree as well if we want to enforce the invariant that all attributable things are located. This is the right thing to do, but it is bound to be long, painful, and break user code.)
• and/or using extension points instead of attributes (that was actually my very first interface idea).

@Octachron I'll wait until you confirm that you agree there is no simple/easy fix here before reverting.

Reading more carefully ast_iterator, I found only 7 exception to the location; attribute pattern:

• iter_type_extension
• iter_structure_item
• iter_signature_item
• value_description
• module_declaration
• module_type_declaration
• module_binding
  My biased opinion is that writing a custom iterator rewriting those part would be ugly but acceptable.

See this commit for some ugliness quantification.

Concerning your other points, extension nodes were my second idea after pseudo-latex macro; but I found that attribute were the right interface at the user level since there is nothing special going at the code level, and this is really a meta-information. I think it is worth it to try to support this interface.
In term of maintenability, would it be a bad idea to move (on trunk) the tuned iterator to parsing/ast_iterator.ml as another variant of ast iterator?
For the attributable items that need a more precise location, I noted attribute locations polymorphic variant and object tag, exceptions. This seems like a small set which could be fixed at an ulterior date.

At least, this my opinion on the spot, I would rethink the issue tomorrow.