This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG655baae2af0b: [clangd] Show used symbols on #include line
hover. (authored by VitaNuo).
Changed prior to commit:
VitaNuo added inline comments.
Comment at: clang-tools-extra/clangd/Hover.cpp:1175
+
+for (const include_cleaner::Header : Providers) {
+ if (!ConvertedIncludes.match(H).empty()) {
hokein wrote:
> VitaNuo wrote:
> > kadircet wrote:
> > > note
VitaNuo updated this revision to Diff 508994.
VitaNuo marked 4 inline comments as done.
VitaNuo added a comment.
Address review comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146244/new/
https://reviews.llvm.org/D146244
Files:
kadircet accepted this revision.
kadircet added a comment.
thanks the pieces i looked at LG too
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146244/new/
https://reviews.llvm.org/D146244
___
cfe-commits
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.
thanks, looks good from my side.
Comment at: clang-tools-extra/clangd/Hover.cpp:1175
+
+for (const include_cleaner::Header : Providers) {
+ if
VitaNuo added a comment.
Thanks for the comments, see the updated version.
Comment at: clang-tools-extra/clangd/Hover.cpp:1173
+if (Ref.RT != include_cleaner::RefType::Explicit ||
+UsedSymbolNames.find(getRefName(Ref)) != UsedSymbolNames.end())
+
VitaNuo updated this revision to Diff 508948.
VitaNuo marked 3 inline comments as done.
VitaNuo added a comment.
Herald added a subscriber: mgrang.
Address review comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146244/new/
kadircet added inline comments.
Comment at: clang-tools-extra/clangd/Hover.cpp:1173
+if (Ref.RT != include_cleaner::RefType::Explicit ||
+UsedSymbolNames.find(getRefName(Ref)) != UsedSymbolNames.end())
+ return;
creating these symbol
VitaNuo updated this revision to Diff 508708.
VitaNuo added a comment.
Remove extra check (already done in walkUsed).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146244/new/
https://reviews.llvm.org/D146244
Files:
VitaNuo updated this revision to Diff 508675.
VitaNuo added a comment.
Add a couple more macro test cases.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146244/new/
https://reviews.llvm.org/D146244
Files:
clang-tools-extra/clangd/Hover.cpp
VitaNuo added a comment.
Thanks for the comments!
Comment at: clang-tools-extra/clangd/Hover.cpp:1175
+
+for (const include_cleaner::Header : Providers) {
+ if (!ConvertedIncludes.match(H).empty()) {
kadircet wrote:
> note that this will
VitaNuo updated this revision to Diff 508661.
VitaNuo marked 10 inline comments as done.
VitaNuo added a comment.
Address review comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146244/new/
https://reviews.llvm.org/D146244
Files:
kadircet added a comment.
(pardon the interruption, some drive-by comments :))
Comment at: clang-tools-extra/clangd/Hover.cpp:1174
+ return;
+
+for (const include_cleaner::Header : Providers) {
note that we don't care about each reference of
hokein added a comment.
thanks, the implementation looks good. I left some comments around the test.
Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:2984
+TEST(Hover, UsedSymbols) {
+ struct {
Can you add a test in the existing `TEST(Hover,
VitaNuo marked an inline comment as done.
VitaNuo added a comment.
Thanks for the comments!
Comment at: clang-tools-extra/clangd/Hover.cpp:1172
+include_cleaner::Includes ConvertedIncludes =
+convertIncludes(SM, llvm::ArrayRef{Inc});
+for (const
VitaNuo updated this revision to Diff 507793.
VitaNuo marked 5 inline comments as done.
VitaNuo added a comment.
Address review comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146244/new/
https://reviews.llvm.org/D146244
Files:
hokein added inline comments.
Comment at: clang-tools-extra/clangd/Hover.cpp:1158
+ const SourceManager = AST.getSourceManager();
+ std::set UsedSymbolNames;
+ include_cleaner::walkUsed(
VitaNuo wrote:
> hokein wrote:
> > just want to check the intention,
VitaNuo added a comment.
Thanks for the comments!
Comment at: clang-tools-extra/clangd/Hover.cpp:1158
+ const SourceManager = AST.getSourceManager();
+ std::set UsedSymbolNames;
+ include_cleaner::walkUsed(
hokein wrote:
> just want to check the intention,
VitaNuo updated this revision to Diff 507326.
VitaNuo marked 10 inline comments as done.
VitaNuo added a comment.
Address review comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146244/new/
https://reviews.llvm.org/D146244
Files:
hokein added inline comments.
Comment at: clang-tools-extra/clangd/Hover.cpp:1141
+std::string getRefName(include_cleaner::SymbolReference Ref) {
+ std::string Name;
we have a similar function `symbolName` in include-cleaner's `FindHeaders.cpp`,
but it is
VitaNuo updated this revision to Diff 506141.
VitaNuo added a comment.
Finish the test.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146244/new/
https://reviews.llvm.org/D146244
Files:
clang-tools-extra/clangd/Hover.cpp
VitaNuo created this revision.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
VitaNuo requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
Repository:
rG LLVM Github Monorepo
22 matches
Mail list logo
