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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
