It would be good to check that Cmts.Loc_tree.of_ast is implemented in such a way that the newly added locations in the 4.11 parsetree make into the location tree. Maybe nothing needs to be done, but I'm not sure and it would be good to check.

FTR, I suggested that the new location added to Pconst_string could replace the tokens_at part in Source.string_literal. That's true, but with a twist: the location in the token refers to the string with delimiters (always double quotes, since delimited strings are raw), while the location in the AST only refers to the payload. So if we want to use this location (which requires a 4.11 parser), we'll need to adjust the literal lexer to skip the enclosing quotes.

About comments attached to the new location: my understanding is that there is always a strict inclusion between the two locs:

Consider a string literal as an expression, let's destructure it as {pexp_loc = loc_expr; pexp_descr = Pconst_string (_, loc_const, _)}.

The locs look like this:

double quoted string

"hello"
 ^^^^^  loc_const
^^^^^^^ loc_expr

raw string

{delim|hello|delim}
       ^^^^^        loc_const
^^^^^^^^^^^^^^^^^^^ loc_expr

quoted extension

{%f|hello|}
    ^^^^^   loc_const
^^^^^^^^^^^ loc_expr

quoted extension with delimiter

{%f delim|hello|delim}
          ^^^^^        loc_const
^^^^^^^^^^^^^^^^^^^^^^ loc_expr

There is no lexical way that I know of to put comments within loc_expr (in the %f delim case, it's possible to insert whitespace, but no comments). I'm not super familiar with the comment attachment algorithm, but I think that means that nothing will get attached to loc_const. So an assertion that the loc tree is empty at the end of formatting (which is already in place?) should be enough.

Does that seem correct @hhugo @gpetiot @jberdine ?

I expect that by and large the comment placement behavior won't be affected due to the strict nesting. However, uses of Cmts.is_adjacent could care. It would be good to check those and see whether that will matter, and decide whether it is better to adjust is_adjacent to take delimiters into account, or to ensure that the string locations include them.

I think this should be OK, we can refine that if necessary when we add tests for 4.11 features. So this seems ready to get merged in my eyes. What do you think @gpetiot ?

(we'll add changelog entries for explicit 4.11 features)

Is there a way to integrate #1383 in this ? i.e. use the loc of constants from the ast for ocaml >= 4.11 and use the functions of #1383 for ocaml < 4.11 ?

I don't think so. These new locs are not the location of the constants themselves, so no comments should get attached to them. The only place we can leverage the new locs is for re-lexing the payload.

They are the loc for string contents (within delimiters), highlighted as loc_const in #1387 (comment).