wgml marked 2 inline comments as done.
wgml added inline comments.

================
Comment at: clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:52
+    const NamespaceContextVec &Namespaces) {
+  std::ostringstream Result;
+  bool First = true;
----------------
aaron.ballman wrote:
> Can this be rewritten with `llvm::for_each()` and a `Twine` so that we don't 
> have to use `ostringstream` (which is a big hammer for this).
The main advantage of `stringstream` was that it is concise.

I don't think I can effectively use `Twine` to build a result in a loop. If I'm 
wrong, correct me, please.

I reworked `concatNamespaces` to use `SmallString` with another educated guess 
of `40` for capacity.


================
Comment at: clang-tidy/modernize/ConcatNestedNamespacesCheck.h:29
+private:
+  using NamespaceContextVec = llvm::SmallVector<const NamespaceDecl *, 6>;
+
----------------
aaron.ballman wrote:
> Why 6?
Educated guess. I considered the codebases I usually work with and it seemed a 
sane value in that context.


================
Comment at: docs/clang-tidy/checks/list.rst:13
    abseil-str-cat-append
+   abseil-string-find-startswith
    android-cloexec-accept
----------------
aaron.ballman wrote:
> This is an unrelated change -- feel free to make a separate commit that fixes 
> this (no review needed).
Setup script did that I guess. I reverted the change.


https://reviews.llvm.org/D52136



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to