[PATCH] D92788: [clangd] NFC: Use SmallVector<T> where possible

Nathan James via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Dec 9 08:50:49 PST 2020


njames93 added inline comments.


================
Comment at: clang-tools-extra/clangd/ConfigProvider.cpp:100
       llvm::StringRef Parent = path::parent_path(P.Path);
-      llvm::SmallVector<llvm::StringRef, 8> Ancestors;
       for (auto I = path::begin(Parent, path::Style::posix),
----------------
This should be using the same size as the vector below as they are guaranteed to be the same size. There may be an argument for merging them both into the same vector using a pair but obviously that wouldn't be part of this patch. 


================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:608
   for (const auto &Line : Tokens) {
-    llvm::SmallVector<char, 128> LineByteTokens;
+    llvm::SmallVector<char> LineByteTokens;
     llvm::raw_svector_ostream OS(LineByteTokens);
----------------
Looks like this is referring to how many bytes are in a line, having 128 seems like a good amount, most coding standards don't like lines longer than that. As a follow up refractor, this could be extracted out the loop to reuse the buffer on the case it does need to allocate. 


================
Comment at: clang-tools-extra/clangd/unittests/TestTU.cpp:190
 const NamedDecl &findDecl(ParsedAST &AST, llvm::StringRef QName) {
-  llvm::SmallVector<llvm::StringRef, 4> Components;
+  llvm::SmallVector<llvm::StringRef> Components;
   QName.split(Components, "::");
----------------
A follow up refractor, this doesn't strictly need to be stored in a vector. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92788/new/

https://reviews.llvm.org/D92788



More information about the cfe-commits mailing list