[PATCH] D131939: [clang-tidy] Add performance-expensive-flat-container-operation check

nicovank added a comment. Ping :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131939/new/ https://reviews.llvm.org/D131939 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org

[PATCH] D142939: Fix handling of -> calls for modernize-use-emplace

nicovank added a comment. Nevermind, this was fixed as I was looking at it. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142939/new/ https://reviews.llvm.org/D142939 ___ cfe-commits mailing list c

[PATCH] D142939: Fix handling of -> calls for modernize-use-emplace

nicovank added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp:100 +auto cxxMemberCallExprOnContainer(StringRef MethodName, + const std::vector ContainerNames) { + return cxxMemberCallExpr( ---

[PATCH] D142939: Fix handling of -> calls for modernize-use-emplace

nicovank accepted this revision. nicovank added a comment. This revision is now accepted and ready to land. This is becoming repetitive, but I guess that's the nature of those things. Maybe something like this would help clean it up, not sure if any better for right now. template // Lambda t

[PATCH] D131939: [clang-tidy] Add performance-expensive-flat-container-operation check

nicovank marked an inline comment as done. nicovank added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131939/new/ https://reviews.llvm.org/D131939 ___ cfe-commits mailing list cfe-comm

[PATCH] D131939: [clang-tidy] Add performance-expensive-flat-container-operation check

nicovank added a comment. Ping. @njames93 If you have a minute, could you please take a second look at this? Let me know what's needed to get this change landed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131939/new/ https://reviews.llvm.org/D

[PATCH] D131939: [clang-tidy] Add performance-expensive-flat-container-operation check

nicovank added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131939/new/ https://reviews.llvm.org/D131939 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/c

[PATCH] D131939: [clang-tidy] Add performance-expensive-flat-container-operation check

nicovank added a comment. Thank you for the feedback! In D131939#3822766 , @njames93 wrote: > I have a feeling the default should be to only warm in loops otherwise this > could get noisy. Agreed, the less noisy version was already the default, renamin

[PATCH] D131939: [clang-tidy] Add performance-expensive-flat-container-operation check

nicovank updated this revision to Diff 464362. nicovank added a comment. Rename OnlyWarnInLoops to WarnOutsideLoops, cover missed cxxForRangeStmt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131939/new/ https://reviews.llvm.org/D131939 Files: c

[PATCH] D131939: [clang-tidy] Add performance-expensive-flat-container-operation check

nicovank added a reviewer: JonasToth. nicovank added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131939/new/ https://reviews.llvm.org/D131939 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D131939: [clang-tidy] Add performance-expensive-flat-container-operation check

nicovank added a comment. Ping. F24615059: image.png Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131939/new/ https://reviews.llvm.org/D131939 ___ cfe-commits maili

[PATCH] D131939: [clang-tidy] Add performance-expensive-flat-container-operation check

nicovank updated this revision to Diff 461592. nicovank added a comment. Rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131939/new/ https://reviews.llvm.org/D131939 Files: clang-tools-extra/clang-tidy/performance/CMakeLists.txt clang-t

[PATCH] D131939: [clang-tidy] Add performance-expensive-flat-container-operation check

nicovank added a comment. Mid-CppCon ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131939/new/ https://reviews.llvm.org/D131939 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists

[PATCH] D131939: [clang-tidy] Add performance-expensive-flat-container-operation check

nicovank added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131939/new/ https://reviews.llvm.org/D131939 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/c

[PATCH] D131939: [clang-tidy] Add performance-expensive-flat-container-operation check

nicovank added a comment. In D131939#3755399 , @Eugene.Zelenko wrote: > I got notification. Thank you. I got an email for this message but not my own updates, which usually also CC me 🤷. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D131939: [clang-tidy] Add performance-expensive-flat-container-operation check

nicovank added a comment. Ping. I feel like Phabricator is not sending notifications for this patch like it usually does (I'm not getting any emails). I'll create a new, identical patch tomorrow if there's still no activity. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[PATCH] D131939: [clang-tidy] Add performance-expensive-flat-container-operation check

nicovank added a reviewer: alexfh. nicovank added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131939/new/ https://reviews.llvm.org/D131939 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D131939: [clang-tidy] Add performance-expensive-flat-container-operation check

nicovank created this revision. Herald added subscribers: carlosgalvezp, xazax.hun, mgorny. Herald added a project: All. nicovank edited the summary of this revision. nicovank edited the summary of this revision. nicovank updated this revision to Diff 452886. nicovank updated this revision to Diff

[PATCH] D127807: [clang-tidy] Properly forward clang-tidy output when running tests

nicovank added a comment. I do not have commit access for the LLVM repository, could one of you commit this for me (Nicolas van Kempen )? I can keep an eye on this today, otherwise I will discuss with someone internally and commit Tuesday. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D127807: [clang-tidy] Properly forward clang-tidy output when running tests

nicovank added a reviewer: serge-sans-paille. nicovank added a comment. Did some digging, looks like this was added in rG7f92a1a84b96 , most likely to also fix that CI build error with `misc-misleading-identifier`. Repository

[PATCH] D127807: [clang-tidy] Properly forward clang-tidy output when running tests

nicovank added a reviewer: LegalizeAdulthood. nicovank added a comment. Ping, adding one more person who has history changing this script. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127807/new/ https://reviews.llvm.org/D127807 _

[PATCH] D127886: [clang-tidy] Allow access to the SourceManager in clang-tidy checks

nicovank abandoned this revision. nicovank added a comment. Looks like `registerPPCallbacks` / `Result.Context->getSourceManager()` gives me the information I need. Thank you @gribozavr2 for your help! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D127886: [clang-tidy] Allow access to the SourceManager in clang-tidy checks

nicovank planned changes to this revision. nicovank added a comment. Thanks! Looks like `registerPPCallbacks` might be what I am looking for, I was not aware of it. I will mark this as planning changes temporarily, then abandon if solved. Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D127886: [clang-tidy] Allow access to the SourceManager in clang-tidy checks

nicovank created this revision. Herald added subscribers: carlosgalvezp, xazax.hun. Herald added a project: All. nicovank added reviewers: alexfh, aaron.ballman, njames93. nicovank published this revision for review. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits.

[PATCH] D127807: [clang-tidy] Properly forward clang-tidy output when running tests

nicovank updated this revision to Diff 437163. nicovank added a comment. Looks like this causes the `misc-misleading-identifier` test to fail on the BuildKite Windows setup because it uses cp1252 and not utf-8. I think this is why the encode was added in the first place. This workaround should h

[PATCH] D127807: [clang-tidy] Properly forward clang-tidy output when running tests

nicovank created this revision. Herald added a subscriber: xazax.hun. Herald added a project: All. nicovank retitled this revision from "[clang-tidy] Remove encode when outputting tool ouput running tests" to "[clang-tidy] Properly forward clang-tidy output when running tests". nicovank updated t

[PATCH] D101471: [clang-tidy] Add proper emplace checks to modernize-use-emplace

nicovank updated this revision to Diff 431426. nicovank added a comment. Fix formatting issues Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101471/new/ https://reviews.llvm.org/D101471 Files: clang-tools-extra/clang-tidy/modernize/UseEmplaceChe

[PATCH] D101471: [clang-tidy] Add proper emplace checks to modernize-use-emplace

nicovank updated this revision to Diff 431387. nicovank added a comment. Update! 1. Rebased. 2. Fixed a minor bug which occasionaly caused false negatives. 3. Cleared up fix/hint generation and another nit following comments. 4. Re-organized and added a couple tests. 5. Made a note of this extens

[PATCH] D101471: [clang-tidy] Add proper emplace checks to modernize-use-emplace

nicovank added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp:294 + } else { +if ((MakeCall ? MakeCall->getNumArgs() : CtorCall->getNumArgs()) == 0) { + Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( ---

[PATCH] D101471: [clang-tidy] Add proper emplace checks to modernize-use-emplace

nicovank updated this revision to Diff 343399. nicovank marked 7 inline comments as done. nicovank added a comment. Fix some style comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101471/new/ https://reviews.llvm.org/D101471 Files: clang-t

[PATCH] D101471: [clang-tidy] Add proper emplace checks to modernize-use-emplace

nicovank marked 3 inline comments as done. nicovank added a comment. Thank you for the feedback, I appreciate it! Let me know if there is any other I missed, AFAIK all `auto` uses introduced by me should be good now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[PATCH] D101471: [clang-tidy] Add proper emplace checks to modernize-use-emplace

nicovank updated this revision to Diff 341349. nicovank added a comment. Explicitely specify one more type Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101471/new/ https://reviews.llvm.org/D101471 Files: clang-tools-extra/clang-tidy/modernize/U

[PATCH] D101471: [clang-tidy] Add proper emplace checks to modernize-use-emplace

nicovank updated this revision to Diff 341342. nicovank added a comment. Fix a couple variable names and explicitely specify a couple types Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101471/new/ https://reviews.llvm.org/D101471 Files: clang-t

[PATCH] D101471: [clang-tidy] Add proper emplace checks to modernize-use-emplace

nicovank added a comment. As you can see, this will fail some tests as there is one corner case which is tricky to handle: std::vector>> vec; // This is valid and should not be changed. vec.emplace_back(std::make_tuple(13, 31)); Does anyone have a suggestion for a good way to handle this?

[PATCH] D101471: [clang-tidy] Add proper emplace checks to modernize-use-emplace

nicovank created this revision. nicovank added reviewers: alexfh, Prazek, kuhar. nicovank added projects: clang, clang-tools-extra. Herald added a subscriber: xazax.hun. nicovank requested review of this revision. Herald added a subscriber: cfe-commits. modernize-use-emplace only recommends going