[PATCH] D116085: [clang-tidy] Performance improvements for NOLINTBEGIN/END blocks

Carlos Galvez via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 19 23:34:08 PST 2022

carlosgalvezp added a comment.

Amazing job @salman-javed-nz ! Here's some initial comments. Reviewing the `NoLintPragmaHandler.cpp` will take some more time from me. It would have been easier to see the diff if the contents had been moved as an NFC patch to a separate file, and then applying the optimizations in this patch. But it's done now so I'll deal with it :)

Comment at: clang-tools-extra/clang-tidy/CMakeLists.txt:20
+  NoLintPragmaHandler.cpp
I think "Pragma" is a very specific term, used for example in `#pragma gcc diagnostic` or `// IWYU pragma: keep`, but in `clang-tidy` we don't use the word `pragma`, so that might be confusing. What about renaming it to `NoLintHandler.cpp` or `NoLintDirectiveHandler.cpp`?

Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h:125
+                                const Diagnostic &Info,
+                                SmallVectorImpl<NoLintError> &NoLintErrors);
Why not `SmallVector`? Sounds like the `Impl` is some "private" implementation?

Comment at: clang-tools-extra/clang-tidy/NoLintPragmaHandler.h:18
+namespace llvm {
+template <typename T> class SmallVectorImpl;
+} // namespace llvm
Why not `SmallVector`?

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list