[PATCH] D88472: [WIP][clangd] Support non-renaming alias underlying decls in TargetDecl.
hokein added inline comments.
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:1120
namespace ns { class [[Foo]] {}; }
- using ns::F^oo;
+ using ns::[[F^oo]];
)cpp",
sammccall wrote:
> 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.
I think you over-look it, the second case of `TEST_F(TargetDeclTest,
UsingDecl)` does report the using-decl.
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
[PATCH] D88472: [WIP][clangd] Support non-renaming alias underlying decls in TargetDecl.
hokein updated this revision to Diff 295238.
hokein added a comment.
refine the patch based on the offline discussion:
- don't set the Underlying bits for using-declaration's underlying decl.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88472/new/
https://reviews.llvm.org/D88472
Files:
clang-tools-extra/clangd/FindTarget.cpp
clang-tools-extra/clangd/FindTarget.h
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/unittests/FindTargetTests.cpp
clang-tools-extra/clangd/unittests/XRefsTests.cpp
Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1118,17 +1118,17 @@
// decls.
R"cpp(
namespace ns { class [[Foo]] {}; }
- using ns::F^oo;
+ using ns::[[F^oo]];
)cpp",
R"cpp(
namespace ns { int [[x]](char); int [[x]](double); }
- using ns::^x;
+ using ns::[[^x]];
)cpp",
R"cpp(
namespace ns { int [[x]](char); int x(double); }
- using ns::x;
+ using ns::[[x]];
int y = ^x('a');
)cpp",
@@ -1156,7 +1156,7 @@
};
template
struct Derived : Base {
-using Base::w^aldo;
+using Base::[[w^aldo]];
};
)cpp",
};
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -182,7 +182,7 @@
)cpp";
// f(char) is not referenced!
EXPECT_DECLS("DeclRefExpr", {"using foo::f", Rel::Alias},
- {"int f(int)", Rel::Underlying});
+ {"int f(int)"});
Code = R"cpp(
namespace foo {
@@ -193,8 +193,8 @@
)cpp";
// All overloads are referenced.
EXPECT_DECLS("UsingDecl", {"using foo::f", Rel::Alias},
- {"int f(int)", Rel::Underlying},
- {"int f(char)", Rel::Underlying});
+ {"int f(int)" },
+ {"int f(char)"});
Code = R"cpp(
struct X {
@@ -206,7 +206,7 @@
int x = Y().[[foo]]();
)cpp";
EXPECT_DECLS("MemberExpr", {"using X::foo", Rel::Alias},
- {"int foo()", Rel::Underlying});
+ {"int foo()"});
Code = R"cpp(
template
@@ -219,7 +219,7 @@
};
)cpp";
EXPECT_DECLS("UnresolvedUsingValueDecl", {"using Base::waldo", Rel::Alias},
- {"void waldo()", Rel::Underlying});
+ {"void waldo()"});
}
TEST_F(TargetDeclTest, ConstructorInitList) {
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -343,18 +343,6 @@
}
}
-// Give the underlying decl if navigation is triggered on a non-renaming
-// alias.
-if (llvm::isa(D) || llvm::isa(D)) {
- // FIXME: address more complicated cases. TargetDecl(... Underlying) gives
- // all overload candidates, we only want the targeted one if the cursor is
- // on an using-alias usage, workround it with getDeclAtPosition.
- llvm::for_each(
- getDeclAtPosition(AST, CurLoc, DeclRelation::Underlying, NodeKind),
- [&](const NamedDecl *UD) { AddResultDecl(UD); });
- continue;
-}
-
// Special case: if the class name is selected, also map Objective-C
// categories and category implementations back to their class interface.
//
@@ -1144,17 +1132,6 @@
DeclRelation::TemplatePattern | DeclRelation::Alias;
std::vector Decls =
getDeclAtPosition(AST, *CurLoc, Relations);
-std::vector NonrenamingAliasUnderlyingDecls;
-// If the results include a *non-renaming* alias, get its
-// underlying decls as well. (See similar logic in locateASTReferent()).
-for (const NamedDecl *D : Decls) {
- if (llvm::isa(D) || llvm::isa(D)) {
-for (const NamedDecl *AD :
- getDeclAtPosition(AST, *CurLoc, DeclRelation::Underlying))
- NonrenamingAliasUnderlyingDecls.push_back(AD);
- }
-}
-llvm::copy(NonrenamingAliasUnderlyingDecls, std::back_inserter(Decls));
// We traverse the AST to find references in the main file.
auto MainFileRefs = findRefs(Decls, AST);
Index: clang-tools-extra/clangd/FindTarget.h
===
--- clang-tools-extra/clangd/FindTarget.h
+++ clang-tools-extra/clangd/FindTarget.h
@@ -102,13 +102,20 @@
TemplatePattern,
// Alias options apply when the declaration is an alias.
- // e.g. namespace clang { [[StringRef]] S; }
+ // e.g. namespace client { [[X]] x; }
/// This declaration is an alias that was referred to.
- /// e.g. using llvm::StringRef (the UsingDecl
[PATCH] D88472: [WIP][clangd] Support non-renaming alias underlying decls in TargetDecl.
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
[PATCH] D88472: [WIP][clangd] Support non-renaming alias underlying decls in TargetDecl.
hokein added inline comments.
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.
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.
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:1120
namespace ns { class [[Foo]] {}; }
- using ns::F^oo;
+ using ns::[[F^oo]];
)cpp",
this seems a small regression, I think it is fine.
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:1130
namespace ns { int [[x]](char); int x(double); }
- using ns::x;
+ using ns::[[x]];
int y = ^x('a');
I think it is an improvement.
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
[PATCH] D88472: [WIP][clangd] Support non-renaming alias underlying decls in TargetDecl.
hokein created this revision.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman.
Herald added a project: clang.
hokein requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.
Extend the TargetDecl API to fix the workaround in
https://reviews.llvm.org/D87225 and
https://reviews.llvm.org/D74054.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D88472
Files:
clang-tools-extra/clangd/FindTarget.cpp
clang-tools-extra/clangd/FindTarget.h
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/unittests/FindTargetTests.cpp
clang-tools-extra/clangd/unittests/XRefsTests.cpp
Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1090,7 +1090,6 @@
R"cpp(
template struct function {};
template using [[callback]] = function;
-
c^allback foo;
)cpp",
@@ -1118,17 +1117,17 @@
// decls.
R"cpp(
namespace ns { class [[Foo]] {}; }
- using ns::F^oo;
+ using ns::[[F^oo]];
)cpp",
R"cpp(
namespace ns { int [[x]](char); int [[x]](double); }
- using ns::^x;
+ using ns::[[^x]];
)cpp",
R"cpp(
namespace ns { int [[x]](char); int x(double); }
- using ns::x;
+ using ns::[[x]];
int y = ^x('a');
)cpp",
@@ -1156,7 +1155,7 @@
};
template
struct Derived : Base {
-using Base::w^aldo;
+using Base::[[w^aldo]];
};
)cpp",
};
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -182,7 +182,7 @@
)cpp";
// f(char) is not referenced!
EXPECT_DECLS("DeclRefExpr", {"using foo::f", Rel::Alias},
- {"int f(int)", Rel::Underlying});
+ {"int f(int)", Rel::NonAliasUnderlying});
Code = R"cpp(
namespace foo {
@@ -193,8 +193,8 @@
)cpp";
// All overloads are referenced.
EXPECT_DECLS("UsingDecl", {"using foo::f", Rel::Alias},
- {"int f(int)", Rel::Underlying},
- {"int f(char)", Rel::Underlying});
+ {"int f(int)", Rel::NonAliasUnderlying},
+ {"int f(char)", Rel::NonAliasUnderlying});
Code = R"cpp(
struct X {
@@ -206,7 +206,7 @@
int x = Y().[[foo]]();
)cpp";
EXPECT_DECLS("MemberExpr", {"using X::foo", Rel::Alias},
- {"int foo()", Rel::Underlying});
+ {"int foo()", Rel::NonAliasUnderlying});
Code = R"cpp(
template
@@ -219,7 +219,7 @@
};
)cpp";
EXPECT_DECLS("UnresolvedUsingValueDecl", {"using Base::waldo", Rel::Alias},
- {"void waldo()", Rel::Underlying});
+ {"void waldo()", Rel::NonAliasUnderlying});
}
TEST_F(TargetDeclTest, ConstructorInitList) {
@@ -275,7 +275,7 @@
int y = [[b]]::x;
)cpp";
EXPECT_DECLS("NestedNameSpecifierLoc", {"namespace b = a", Rel::Alias},
- {"namespace a", Rel::Underlying});
+ {"namespace a", Rel::AliasUnderlying});
}
TEST_F(TargetDeclTest, Types) {
@@ -291,14 +291,14 @@
[[X]] x;
)cpp";
EXPECT_DECLS("TypedefTypeLoc", {"typedef S X", Rel::Alias},
- {"struct S", Rel::Underlying});
+ {"struct S", Rel::AliasUnderlying});
Code = R"cpp(
namespace ns { struct S{}; }
typedef ns::S X;
[[X]] x;
)cpp";
EXPECT_DECLS("TypedefTypeLoc", {"typedef ns::S X", Rel::Alias},
- {"struct S", Rel::Underlying});
+ {"struct S", Rel::AliasUnderlying});
// FIXME: Auto-completion in a template requires disabling delayed template
// parsing.
@@ -325,7 +325,7 @@
S X;
[[decltype]](X) Y;
)cpp";
- EXPECT_DECLS("DecltypeTypeLoc", {"struct S", Rel::Underlying});
+ EXPECT_DECLS("DecltypeTypeLoc", {"struct S", Rel::NonAliasUnderlying});
Code = R"cpp(
struct S{};
@@ -534,8 +534,9 @@
)cpp";
EXPECT_DECLS("TemplateSpecializationTypeLoc",
{"template<> class SmallVector",
-Rel::TemplateInstantiation | Rel::Underlying},
- {"class SmallVector", Rel::TemplatePattern | Rel::Underlying},
+Rel::TemplateInstantiation | DeclRelation::AliasUnderlying},
+ {"class SmallVector",
+Rel::TemplatePattern | DeclRelation::AliasUnderlying},
{"using TinyVector = SmallVector",
Rel::Alias | Rel::TemplatePattern});
}
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -314,8 +314,9 @@
};
// Emit all symbol
