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

salman-javed-nz added a comment. In D116085#3272200 , @nemanjai wrote: > This is causing buildbot failures with `-Werror` (i.e. > https://lab.llvm.org/buildbot/#/builders/57/builds/14322/steps/5/logs/stdio). > Please fix or revert. Thanks for the heads

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

nemanjai added a comment. In D116085#3272200 , @nemanjai wrote: > This is causing buildbot failures with `-Werror` (i.e. > https://lab.llvm.org/buildbot/#/builders/57/builds/14322/steps/5/logs/stdio). > Please fix or revert. The failure is: > > > ...

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

nemanjai added a comment. This is causing buildbot failures with `-Werror` (i.e. https://lab.llvm.org/buildbot/#/builders/57/builds/14322/steps/5/logs/stdio). Please fix or revert. The failure is: .../llvm-project/clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp:204:38: error: moving

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG19eaad94c47f: [clang-tidy] Cache the locations of NOLINTBEGIN/END blocks (authored by salman-javed-nz). Changed prior to commit: https://reviews.l

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

carlosgalvezp accepted this revision. carlosgalvezp added a comment. Looks great! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116085/new/ https://reviews.llvm.org/D116085 ___ cfe-commits mailing list c

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

salman-javed-nz updated this revision to Diff 402818. salman-javed-nz added a comment. Review comment: `formNoLintBlocks()` - drop the `&` so the vector must be std::moved in Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116085/new/ https://review

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

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! Comment at: clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp:187 +static SmallVector +formNoLintBlocks(SmallVector &NoLints, + SmallV

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

salman-javed-nz updated this revision to Diff 402774. salman-javed-nz added a comment. `ClangTidyContext::shouldSuppressDiagnostic()`: - Hook up `AllowIO` and `EnableNoLintBlocks` to `NoLintDirectiveHandler::shouldSuppress()` `NoLintToken`: -Remove copy ctor and assignment operator. Class is no

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

kadircet added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:214 + std::string CheckName = getCheckName(Info.getID()); + return NoLintHandler.shouldSuppress(DiagLevel, Info, CheckName, NoLintErrors); +} we don't seem

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

salman-javed-nz added a comment. Every review comment so far should be addressed now, with the exception of the following 2 points. Comment at: clang-tools-extra/clang-tidy/NoLintPragmaHandler.cpp:420 + // file if it is a . + Optional FileName = SrcMgr.getNonBuiltinFilenameF

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

salman-javed-nz added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:349 + if (Context.shouldSuppressDiagnostic(DiagLevel, Info, SuppressionErrors, + EnableNolintBlocks)) { ++Context.Stats.Erro

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

salman-javed-nz marked an inline comment as done. salman-javed-nz added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:448 +/// this line. +static std::pair getLineStartAndEnd(StringRef S, size_t From) { + size_t StartPos = S.find_last_

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

salman-javed-nz marked 12 inline comments as done. salman-javed-nz added inline comments. Comment at: clang-tools-extra/clang-tidy/NoLintPragmaHandler.cpp:63 +// as parsed from the file's character contents. +class NoLintToken { +public: kadircet wrote: > kadirce

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

salman-javed-nz marked 4 inline comments as done. salman-javed-nz added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h:84 + bool AllowEnablingAnalyzerAlphaCheckers = false, + bool AllowIO = true, bool En

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

salman-javed-nz updated this revision to Diff 402308. salman-javed-nz changed the visibility from "Only User: salman-javed-nz (Salman Javed)" to "Public (No Login Required)". salman-javed-nz added a comment. Pass the `NoLintErrors` SmallVector all the way through the call stack, instead of stori

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

salman-javed-nz added a comment. Thank you very much to both of you for having a look at the patch. Yes, I agree it is a large patch, and I could have done a better job splitting it up into more manageable chunks. One reason this change is so big is because I set myself an ambitious target for

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

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/ClangTidyDia

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

kadircet added inline comments. Comment at: clang-tools-extra/test/clang-tidy/infrastructure/Inputs/nolintbeginend/1st-translation-unit.cpp:1 +// NOLINTBEGIN +class A { A(int i); }; kadircet wrote: > no run lines or anything here (and the following file) oops, n

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

kadircet added a comment. here are some more comments, can't say I've been able to go through all of the patch yet, unfortunately it's considerably big in size. it would be great to get rid of some of those extra code by just dropping accessors/classes when not needed. Comme

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

salman-javed-nz updated this revision to Diff 401576. salman-javed-nz edited the summary of this revision. salman-javed-nz added a comment. Update according to review (rename NoLintPragmaHandler class to NoLintDirectiveHandler) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

salman-javed-nz added a comment. In D116085#3257210 , @carlosgalvezp wrote: > 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 th

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

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 th

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

salman-javed-nz added a comment. Friendly ping. Would be good to get these performance improvements into trunk soon, so that we're not prolonging the time that people are putting up with the current slow implementation. Also, I believe that LLVM 14.0.0 will be up for a release candidate soon,

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

salman-javed-nz updated this revision to Diff 398087. salman-javed-nz added a comment. Comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116085/new/ https://reviews.llvm.org/D116085 Files: clang-tools-extra/clang-tidy/CMakeLists.txt clang

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

salman-javed-nz updated this revision to Diff 398048. salman-javed-nz added a comment. Herald added subscribers: usaxena95, arphaman, mgorny. Updated according to review comments - Move implementation to a CPP file. Use a PIMPL for access. Repository: rG LLVM Github Monorepo CHANGES SINCE LA

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

salman-javed-nz added a comment. Thanks for taking a look. I will update the diff to address your comments. Have a good new years break. Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:197 + SourceLocation Loc = FileStartLoc.getLocWithOffset( + s

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

kadircet added a comment. some drive-by comments, i'll be AFK for a while though. so please don't block this on my approval if people disagree. Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:197 + SourceLocation Loc = FileStartLoc.getLocWithOffset( +

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

salman-javed-nz added a comment. Updated the review's edit permissions. Sorry about that, @kadircet. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116085/new/ https://reviews.llvm.org/D116085 ___ cfe-com

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

Hi Salman, it looks like patch doesn't have edit access for anyone but you. I had drafted some comments but can't hit the submit button. I think you can go to https://reviews.llvm.org/differential/revision/edit/116085/ and change the `Editable By` field to `All Users` On Tue, Dec 21, 2021 at 8:45

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

salman-javed-nz updated this revision to Diff 395606. salman-javed-nz added a comment. Remove unnecessary `llvm::` qualification. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116085/new/ https://reviews.llvm.org/D116085 Files: clang-tools-extra