[PATCH] D107837: abseil-string-find-str-contains should not propose an edit for the three-parameter version of find().

2021-08-10 Thread Tom Lokovic via Phabricator via cfe-commits
tdl-g created this revision. tdl-g added a reviewer: ymandel. tdl-g requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. std::string, std::string_view, and absl::string_view all have a three-parameter version of find() which has a

[PATCH] D101037: [clang-tidy] Change shebang from python to python3

2021-04-22 Thread Tom Lokovic via Phabricator via cfe-commits
tdl-g accepted this revision. tdl-g added a comment. This revision is now accepted and ready to land. I'll defer to the consensus on https://lists.llvm.org/pipermail/cfe-dev/2021-April/068047.html regarding whether or not there are gotchas for requiring python3,, but assuming tests have

[PATCH] D93637: [libTooling] Add support for smart pointers to relevant Transformer `Stencil`s.

2021-01-05 Thread Tom Lokovic via Phabricator via cfe-commits
tdl-g added a comment. Ah, if it's just an optimization that makes sense. I still think it's worth having a test case to confirm that one of the specially-handled cases works. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93637/new/

[PATCH] D93637: [libTooling] Add support for smart pointers to releveant Transformer `Stencil`s.

2021-01-04 Thread Tom Lokovic via Phabricator via cfe-commits
tdl-g added inline comments. Comment at: clang/unittests/Tooling/StencilTest.cpp:273 + std::string Snippet = R"cc( +Smart x; +x; You're only testing the "QuacksLike" case. I suspect you should have tests that validate the "KnownSmartPointers".

[PATCH] D92658: [libTooling] Add `describe` stencil for formatting AST nodes for diagnostics.

2020-12-04 Thread Tom Lokovic via Phabricator via cfe-commits
tdl-g added inline comments. Comment at: clang/unittests/Tooling/StencilTest.cpp:513 +TEST(StencilToStringTest, DescribeOp) { + auto S = describe("Id"); ymandel wrote: > tdl-g wrote: > > Can you add a comment (or a more detailed test name) explaining what

[PATCH] D92658: [libTooling] Add `describe` stencil for formatting AST nodes for diagnostics.

2020-12-04 Thread Tom Lokovic via Phabricator via cfe-commits
tdl-g accepted this revision. tdl-g added a comment. This revision is now accepted and ready to land. Looks great, just one comment. Comment at: clang/unittests/Tooling/StencilTest.cpp:513 +TEST(StencilToStringTest, DescribeOp) { + auto S = describe("Id");

[PATCH] D89468: [libTooling] Change `after` range-selector to operate only on source ranges

2020-10-15 Thread Tom Lokovic via Phabricator via cfe-commits
tdl-g requested changes to this revision. tdl-g added a comment. This revision now requires changes to proceed. Just one comment about the tests. Comment at: clang/unittests/Tooling/RangeSelectorTest.cpp:196 +static void testAfterMacroArg(StringRef Code) { + const

[PATCH] D82126: [libTooling] Change Transformer's `cat` to handle some cases of text in macros.

2020-06-19 Thread Tom Lokovic via Phabricator via cfe-commits
tdl-g added a comment. Interesting, in all three of those cases, it's reasonable to replace the entire expression, thus eliminating the macro. None of those "tear" the macro; if we had a case like #define FOO(a,b,c,d) ((a).find(b) == std::string::npos ? (c) : (d)) FOO("helo", "x", 5, 6); I

[PATCH] D82126: [libTooling] Change `selection` stencil to handle some cases of text in macros.

2020-06-19 Thread Tom Lokovic via Phabricator via cfe-commits
tdl-g added a comment. LGTM. I found the change description confusing, since it talks about the selection() stencil but the code is all about the cat() stencil. I realize (now) that the former is deprecated in favor of the latter. But the change description is still confusing. Repository:

[PATCH] D80023: [clang-tidy] Add abseil-string-find-str-contains checker.

2020-05-28 Thread Tom Lokovic via Phabricator via cfe-commits
tdl-g added a comment. We see this broke the build for shared-lib config http://lab.llvm.org:8011/builders/llvm-avr-linux/builds/1879 Looking now (and I look forward to determining what I should have done to avoid this). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D80023: [clang-tidy] Add abseil-string-find-str-contains checker.

2020-05-26 Thread Tom Lokovic via Phabricator via cfe-commits
tdl-g updated this revision to Diff 266207. tdl-g added a comment. Fixed length of visual separator. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80023/new/ https://reviews.llvm.org/D80023 Files:

[PATCH] D80023: [clang-tidy] Add abseil-string-find-str-contains checker.

2020-05-26 Thread Tom Lokovic via Phabricator via cfe-commits
tdl-g marked 4 inline comments as done. tdl-g added a comment. Thanks again, addressed all comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-str-contains.cpp:2 +// RUN: %check_clang_tidy %s abseil-string-find-str-contains %t -- \ +// RUN:

[PATCH] D80023: [clang-tidy] Add abseil-string-find-str-contains checker.

2020-05-20 Thread Tom Lokovic via Phabricator via cfe-commits
tdl-g updated this revision to Diff 265234. tdl-g marked 12 inline comments as done. tdl-g added a comment. Addressed second round of comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80023/new/ https://reviews.llvm.org/D80023 Files:

[PATCH] D80023: [clang-tidy] Add abseil-string-find-str-contains checker.

2020-05-20 Thread Tom Lokovic via Phabricator via cfe-commits
tdl-g added a comment. Thanks, all, for the additional comments. I addressed them all except for the suggestion to add an options-specific test. I'm not against it, but (as I mention in the comment) I'm also unsure how to meaningfully test the include-inserting-related options.

[PATCH] D80023: [clang-tidy] Add abseil-string-find-str-contains checker.

2020-05-19 Thread Tom Lokovic via Phabricator via cfe-commits
tdl-g added a comment. Thanks, all for the comments. I believe I've addressed all comments. Note that TransformerClangTidyCheck interacts awkwardly with StoreOptions; I have a FIXME to clean that up. Comment at:

[PATCH] D80023: [clang-tidy] Add abseil-string-find-str-contains checker.

2020-05-19 Thread Tom Lokovic via Phabricator via cfe-commits
tdl-g updated this revision to Diff 264986. tdl-g marked 16 inline comments as done. tdl-g added a comment. Addressed review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80023/new/ https://reviews.llvm.org/D80023 Files:

[PATCH] D80023: [clang-tidy] Add abseil-string-find-str-contains checker.

2020-05-15 Thread Tom Lokovic via Phabricator via cfe-commits
tdl-g added a comment. Eugene, thank you for the comments, I'll address them soon. For the moment I'm trying to figure out what's up with the list.rst changes. Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:15 - `abseil-duration-addition `_, "Yes" -

[PATCH] D80023: [clang-tidy] Add abseil-string-find-str-contains checker.

2020-05-15 Thread Tom Lokovic via Phabricator via cfe-commits
tdl-g created this revision. tdl-g added a reviewer: ymandel. Herald added subscribers: cfe-commits, phosek, Charusso, mgorny. Herald added a project: clang. Eugene.Zelenko edited reviewers, added: alexfh, hokein, aaron.ballman, njames93; removed: ymandel. Eugene.Zelenko added a project: