This revision was automatically updated to reflect the committed changes.
Closed by commit rG987f9cb6b970: [clang-tidy] Add proper emplace checks to
modernize-use-emplace (authored by nicovank, committed by ivanmurashko).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://
ivanmurashko accepted this revision.
ivanmurashko added a comment.
LGTM
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D101471/new/
https://reviews.llvm.org/D101471
___
cfe-commits mailing list
cfe-commits
kuhar accepted this revision.
kuhar added a comment.
This revision is now accepted and ready to land.
LGTM but please get a second approval before submitting.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D101471/new/
https://reviews.llvm.org/D10147
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
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
kuhar resigned from this revision.
kuhar added inline comments.
Herald added a subscriber: carlosgalvezp.
Herald added a project: All.
Comment at: clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp:44
+ // FullNameTrimmed matches any of the given Names.
+ const StringRe
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(
---
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
kuhar added inline comments.
Comment at: clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp:28
+ std::string FullNameTrimmed;
+ int Count = 0;
+ for (const auto &Character : FullName) {
Can you add a comment explaining what this loops tries to do? Idea
Eugene.Zelenko added inline comments.
Comment at: clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp:29
+ int Count = 0;
+ for (const auto &Character : FullName) {
+if (Character == '<') {
I'm not sure, but probably braces could be elided in `for` a
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
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
Eugene.Zelenko added a comment.
Please mark fixed suggestions as done.
Comment at: clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp:248
+ const auto *Call = PushBackCall ? PushBackCall : EmplacyCall;
+
Please don't use auto unless type is spelled e
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
Eugene.Zelenko added inline comments.
Comment at: clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp:25
+ Names) {
+ const auto FullName = "::" + Node.getQualifiedNameAsString();
+
Please don't use auto unless type is spelled explicitly in s
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?
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
17 matches
Mail list logo
