sammccall added a comment.
We manage to get rid of a little code here, but we add complexity to an
important API, and make... one test better and a few tests worse.
I'd like to get rid of the wart in the code, but the tradeoff doesn't seem
completely compelling... going to think about the API here a bit.
================
Comment at: clang-tools-extra/clangd/FindTarget.h:112
+ /// Underlying declarations for renaming alias (typedef decl, type alias
decl)
+ AliasUnderlying,
+ /// Underlying declarations for non-renaming alias, decltype, etc.
----------------
hokein wrote:
> The previous `Alias` vs `Underlying` were pretty nice, but they were not
> enough to support our non-renaming-alias-underlying case. Looking for the
> feedback on the API change here.
Inevitably this adds some noise, and the names are less clear (we certainly
need to avoid using "alias" to mean two different things, and we should try to
avoid negatives that confuse the boolean logic).
We could simplify this at least at query locations by defining Underlying =
NonAliasUnderlying | AliasUnderlying, but unfortunately we can't put it in
DeclRelation.
The other thing is this isn't complete either, because it doesn't distinguish
between renaming and non-renaming `Alias`es. This is why we have the regression
in the testcases.
That this is so verbose is a property of the original design: the idea was
"mark this edge as alias->underlying, and apply a bit when that edge is
traversed" but of course we decided to signal "the edge is not traversed" too.
So if we introduce a new type of edge, we need *two* new bits - one at each end.
I'm trying to think of ideas for improvements without totally ditching the
API...
================
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:1120
namespace ns { class [[Foo]] {}; }
- using ns::F^oo;
+ using ns::[[F^oo]];
)cpp",
----------------
hokein wrote:
> this seems a small regression, I think it is fine.
I can't follow why this is different from the second case of
TEST_F(TargetDeclTest, UsingDecl), where the usingdecl is not reported at all.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88472/new/
https://reviews.llvm.org/D88472
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
