[PATCH] D105904: [clangd] Support `#pragma mark` in the outline

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

In D105904#2877281 <https://reviews.llvm.org/D105904#2877281>, @dgoldman wrote:

> Yep, I can you send you some searches internally with how prevalent they are.

Thanks!

> I don't think this an Xcode limitation really - more of an LSP spec limitation. If we create a group inside of another symbol, we are effectively contained in it and cannot escape without violating the spec, right? To be clear, Xcode rules are a bit different - see the link above - the `#pragma mark -` just creates a divider which we can't mirror.

Sorry I don't follow what you mean here. I was talking about inability to have something like:

  #pragma mark -Foo
    #pragma mark -Bar
    #pragma mark SPECIAL_BAR_END
  #pragma mark SPECIAL_FOO_END

By making sure groups are terminated **explicitly** we gain the ability to nest them, whereas otherwise `-Bar` above will always terminate the previous group implicitly.

> This assumes (like this code today) that the nodes are sorted properly and the ranges of the parent are accurate.

Well it is not as strong as that. We'll mostly rely on the fact that symbols are nested properly. Since we just need to figure out the mark container for a symbol, we don't care about the ordering (or am I missing something here?).
OTOH, range of the parent being accurate is indeed required. I think today it is only broken for macro parents, but hopefully it shouldn't cause as any troubles as in all the practical cases these are still contained inside an AST node (I think), e.g:

  #define FOO(X) class X
  FOO(Bar) {
  #pragma mark -Test
  void bar();
  }

`-Test` mark is contained in `class X` so it can be nested properly. I don't think it is ever possible to have it nest directly under a macro name since you can't have other PP directives expanded from macros (even though it is probably possible via weird tricks like `__pragma`/`_Pragma`).
Now ranges for the pragma-mark containers will also be broken, but again it is not a problem as we are never going to attach any pragma-mark symbols directly into another pragma-mark (they'll either be siblings or there'll be a regular AST node in between, as explained previously).

> I was thinking of adding a proper sort in a post-processing phase but maybe we could do it + range fix up during insertion. Seems complicated to move everything over to insertion, but if you think that's reasonable, I can try that out.

As explained above, I think we can just keep adjusting the ranges after building the tree as we do today. Let me know if I am missing something though.

> I think it's a bit more complicated since the leaf nodes terminate the groups. As long as we do that it should be fine, assuming editors don't require it to be sorted.

Well actually handling the leafs that terminate the group they are contained in is easy (since we'll make use of the closest mark-pragma to the declaration), but the other way around is hard, e.g:

  #pragma mark -Foo
  class X {
    #pragma mark -Bar
  };
  void foo();

`foo` above needs to be nested under `Foo`, even though the closest mark-pragma is `Bar`. I think these can be handled more naturally by making sure we throw away mark-pragmas contained inside a symbol after processing the containing symbol (e.g. `Bar` will be dropped from our container after `X` has been inserted into the tree).

> We could - I considered it, but with the existing behavior and potential subtle ordering issues I thought it would be simpler, but less efficient to do it after.

Does my new set of comments help clarify things a little more? I don't think we need any ordering guarantees, we mostly rely on the fact that mark-pragmas will be sorted properly and symbols are nested correctly.

> I took a look at the Macro Expansion code and didn't understand it + potential edge cases well enough to add it there.

I don't think this should directly go into the macro parent logic, but rather be orthogonal. Then we can just chain them, by first looking for a macro container and then looking for a mark-pragma one above that container (since we know the other-way around is not possible, i.e. a mark-pragma cannot nest directly under a macro).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105904/new/

https://reviews.llvm.org/D105904