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