carlosgalvezp added inline comments.
================ Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:347 + NoLint.SpecifiesChecks = true; + } + } ---------------- Nit: tab with spaces ================ Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:390 +static SmallVector<NoLintBlockToken> +collectNoLintBlocks(const ClangTidyContext &Context, const SourceManager &SM, + StringRef Buffer, SourceLocation BufferLoc, ---------------- As a non-native English speaker, I appreciate the name change :) ================ Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:422 + Errors.emplace_back(Error); + } + ---------------- Nit: tab with spaces ================ Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:448 +/// this line. +static std::pair<size_t, size_t> getLineStartAndEnd(StringRef S, size_t From) { + size_t StartPos = S.find_last_of('\n', From) + 1; ---------------- Don't we usually use `SourceLocation` objects for this? ================ Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:480 + if (isNoLintFound(PrevLine, NoLintToken::Type::NoLintNextLine, CheckName)) + return true; + } ---------------- Tab with spaces ================ Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:563 + return true; + GlobList Globs(Checks, /*KeepNegativeGlobs=*/false); + return Globs.contains(CheckName); ---------------- kadircet wrote: > this creates new regexes at each call, what about building the glob once in > constructor and storing that instead of `Checks` ? Maybe good to keep the original comment about why negative globs are excluded? ================ Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:563 + return true; + GlobList Globs(Checks, /*KeepNegativeGlobs=*/false); + return Globs.contains(CheckName); ---------------- carlosgalvezp wrote: > kadircet wrote: > > this creates new regexes at each call, what about building the glob once in > > constructor and storing that instead of `Checks` ? > Maybe good to keep the original comment about why negative globs are excluded? +1. Also, in case it helps, you can use `CachedGlobList`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116085/new/ https://reviews.llvm.org/D116085 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits