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 include_cleaner::Header &H : Providers) {
----------------
hokein wrote:
> can you move it out of the callback? otherwise we will construct an
> `include_cleaner::Includes` every time when the callback is invoked, which is
> an unnecessary cost.
Oh right, sorry.
================
Comment at: clang-tools-extra/clangd/Hover.cpp:1444
+
+ if (UsedSymbolNames.size() >= SymbolNamesLimit) {
+ P.appendCode(UsedSymbolNames[SymbolNamesLimit - 1]);
----------------
hokein wrote:
> I think it'd be better to move this into the above for loop (adding a special
> logic) rather than having a non-trivial handling for the last element.
>
> What do you think about the something like below (it has less usages of
> `.size()` and `SymbolNamesLimit-1`)
>
> ```
> auto Fronts =
> llvm::ArrayRef(UsedSymbolNames).take_front(std::min(UnusedSymbolNames.size(),
> SymbolNamesLimit));
>
> for (const auto& Sym : Fronts) {
> P.appendCode(Sym);
> if (Sym != Fronts.back())
> P.appendText(", ");
> }
>
> if (UsedSymbolNames.size() > Fronts.size()) {
> P.appendText(" and ");
> P.appendText(std::to_string(UsedSymbolNames.size() - Fronts.size()));
> P.appendText(" more");
> }
> ```
Sure, thank you.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146244/new/
https://reviews.llvm.org/D146244
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
