This revision was automatically updated to reflect the committed changes.
VitaNuo marked an inline comment as done.
Closed by commit rGe028c9742897: Move the BySpelling map to IncludeStructure.
(authored by VitaNuo).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://revie
kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.
thanks (both for the patch and for bearing with me)!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143509/new/
https://reviews.llvm.org/D143
VitaNuo marked 2 inline comments as done.
VitaNuo added inline comments.
Comment at: clang-tools-extra/clangd/Headers.cpp:303
+ llvm::SmallVector Includes;
+ auto It = MainFileIncludesBySpelling.find(Spelling);
+ if (It == MainFileIncludesBySpelling.end())
kad
VitaNuo updated this revision to Diff 499456.
VitaNuo marked an inline comment as done.
VitaNuo added a comment.
Address review comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143509/new/
https://reviews.llvm.org/D143509
Files:
clang-too
kadircet added inline comments.
Comment at: clang-tools-extra/clangd/Headers.cpp:303
+ llvm::SmallVector Includes;
+ auto It = MainFileIncludesBySpelling.find(Spelling);
+ if (It == MainFileIncludesBySpelling.end())
VitaNuo wrote:
> kadircet wrote:
> > nit:
>
VitaNuo added a comment.
Thanks for the comments!
Comment at: clang-tools-extra/clangd/Headers.cpp:303
+ llvm::SmallVector Includes;
+ auto It = MainFileIncludesBySpelling.find(Spelling);
+ if (It == MainFileIncludesBySpelling.end())
kadircet wrote:
> nit:
>
VitaNuo updated this revision to Diff 499436.
VitaNuo marked 2 inline comments as done.
VitaNuo added a comment.
Address review comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143509/new/
https://reviews.llvm.org/D143509
Files:
clang-too
kadircet added inline comments.
Comment at: clang-tools-extra/clangd/Headers.cpp:303
+ llvm::SmallVector Includes;
+ auto It = MainFileIncludesBySpelling.find(Spelling);
+ if (It == MainFileIncludesBySpelling.end())
nit:
```
llvm::SmallVector Includes;
for (au
VitaNuo updated this revision to Diff 498428.
VitaNuo added a comment.
Move to source file.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143509/new/
https://reviews.llvm.org/D143509
Files:
clang-tools-extra/clangd/Headers.cpp
clang-tools-extr
VitaNuo marked an inline comment as done.
VitaNuo added a comment.
Thanks for the comments!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143509/new/
https://reviews.llvm.org/D143509
___
cfe-commits mail
VitaNuo updated this revision to Diff 498419.
VitaNuo added a comment.
Address comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143509/new/
https://reviews.llvm.org/D143509
Files:
clang-tools-extra/clangd/Headers.cpp
clang-tools-extra/cl
kadircet added inline comments.
Comment at: clang-tools-extra/clangd/Headers.h:167
+ // Spelling should include brackets or quotes, e.g. .
+ llvm::SmallVector
+ mainFileIncludesWithSpelling(llvm::StringRef Spelling) const {
we're still returning just the `Head
VitaNuo added a comment.
Thanks for the review!
Comment at: clang-tools-extra/clangd/Headers.cpp:76
+Out->MainFileIncludesBySpelling.try_emplace(Inc.Written)
+.first->second.push_back(static_cast(*Inc.HeaderID));
}
kadircet wrote:
> r
VitaNuo updated this revision to Diff 497602.
VitaNuo marked an inline comment as done.
VitaNuo added a comment.
Address review comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143509/new/
https://reviews.llvm.org/D143509
Files:
clang-too
kadircet added a comment.
oh forgot to mention, could you also please add some tests into
llvm-project/clang-tools-extra/clangd/unittests/HeadersTests.cpp ?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143509/new/
https://reviews.llvm.org/D143509
kadircet added inline comments.
Comment at: clang-tools-extra/clangd/Headers.cpp:76
+Out->MainFileIncludesBySpelling.try_emplace(Inc.Written)
+.first->second.push_back(static_cast(*Inc.HeaderID));
}
right now we're only storing "resolve
VitaNuo added a comment.
Thanks for review!
Comment at: clang-tools-extra/clangd/Headers.h:164
+ llvm::StringMap>
+ buildMainFileIncludesBySpelling() const {
+llvm::StringMap> BySpelling;
kadircet wrote:
> instead of building this on-demand, what about bu
VitaNuo updated this revision to Diff 496463.
VitaNuo added a comment.
Expose API for lookup instead of retrieving the whole map.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143509/new/
https://reviews.llvm.org/D143509
Files:
clang-tools-extra
VitaNuo updated this revision to Diff 496460.
VitaNuo added a comment.
Address review comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143509/new/
https://reviews.llvm.org/D143509
Files:
clang-tools-extra/clangd/Headers.cpp
clang-tools-
kadircet added inline comments.
Comment at: clang-tools-extra/clangd/Headers.h:164
+ llvm::StringMap>
+ buildMainFileIncludesBySpelling() const {
+llvm::StringMap> BySpelling;
instead of building this on-demand, what about building it as we're collecting
d
VitaNuo created this revision.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
VitaNuo requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D1
21 matches
Mail list logo
