This revision was automatically updated to reflect the committed changes.
Closed by commit rL269341: [clang-tidy] Adds modernize-avoid-bind check
(authored by jbcoe).
Changed prior to commit:
http://reviews.llvm.org/D16962?vs=57067&id=57095#toc
Repository:
rL LLVM
http://reviews.llvm.org/D1
jbcoe updated this revision to Diff 57067.
jbcoe added a comment.
update diff to allow arc to apply patch.
http://reviews.llvm.org/D16962
Files:
clang-tidy/modernize/AvoidBindCheck.cpp
clang-tidy/modernize/AvoidBindCheck.h
clang-tidy/modernize/CMakeLists.txt
clang-tidy/modernize/Moderni
jbcoe added a comment.
I can submit the patch myself, thanks.
Thanks again for taking the time to give such a thorough review.
http://reviews.llvm.org/D16962
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mail
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
Looks good. Thank you!
Do you need someone to submit the patch for you?
Comment at: clang-tidy/modernize/AvoidBindCheck.cpp:28
@@ +27,3 @@
+ BindArgumentKind Kind = BK_Other
jbcoe updated this revision to Diff 57022.
jbcoe marked 6 inline comments as done.
jbcoe added a comment.
Apply fixes from review.
http://reviews.llvm.org/D16962
Files:
clang-tidy/modernize/AvoidBindCheck.cpp
clang-tidy/modernize/AvoidBindCheck.h
clang-tidy/modernize/CMakeLists.txt
clan
alexfh requested changes to this revision.
This revision now requires changes to proceed.
Comment at: clang-tidy/modernize/AvoidBindCheck.cpp:27
@@ +26,3 @@
+ StringRef Tokens;
+ BindArgumentKind Kind = BK_Other;
+ size_t PlaceHolderIndex = 0;
I wonder whether
Modernize it is then.
The check currently only catches std::bind but further work can address that.
Jon
> On 4 May 2016, at 22:40, Arthur O'Dwyer wrote:
>
>> On Wed, May 4, 2016 at 1:43 PM, Aaron Ballman via cfe-commits
>> wrote:
>> jbcoe wrote:
>> > aaron.ballman wrote:
>> > > I believe we
jbcoe removed a reviewer: djasper.
jbcoe updated this revision to Diff 56273.
jbcoe added a comment.
Move to modernize. Rename to `avoid-bind` as a later patch will add support for
`boost::bind`, `std::bind1st` etc.
http://reviews.llvm.org/D16962
Files:
clang-tidy/modernize/AvoidBindCheck.cp
On Wed, May 4, 2016 at 1:43 PM, Aaron Ballman via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> jbcoe wrote:
> > aaron.ballman wrote:
> > > I believe we use "modernize" to really mean "migrate from the old way
> to the new way", which this definitely fits into since I think the point to
> thi
aaron.ballman added inline comments.
Comment at: clang-tidy/readability/ReadabilityTidyModule.cpp:17
@@ -15,2 +16,2 @@
#include "ContainerSizeEmptyCheck.h"
#include "DeletedDefaultCheck.h"
jbcoe wrote:
> aaron.ballman wrote:
> > I believe we use "modernize" to r
jbcoe added inline comments.
Comment at: clang-tidy/readability/ReadabilityTidyModule.cpp:17
@@ -15,2 +16,2 @@
#include "ContainerSizeEmptyCheck.h"
#include "DeletedDefaultCheck.h"
aaron.ballman wrote:
> I believe we use "modernize" to really mean "migrate from
aaron.ballman added inline comments.
Comment at: clang-tidy/readability/ReadabilityTidyModule.cpp:17
@@ -15,2 +16,2 @@
#include "ContainerSizeEmptyCheck.h"
#include "DeletedDefaultCheck.h"
I believe we use "modernize" to really mean "migrate from the old way to
alexfh added a comment.
Please regenerate the patch with the full context (see
http://llvm.org/docs/Phabricator.html).
Comment at: clang-tidy/readability/AvoidStdBindCheck.cpp:77
@@ +76,3 @@
+ for (size_t I = 1; I <= PlaceholderCount; ++I) {
+Stream << Delimiter << "auto &
jbcoe updated this revision to Diff 55974.
jbcoe added a comment.
Remove unused function `isStdBind`.
http://reviews.llvm.org/D16962
Files:
clang-tidy/readability/AvoidStdBindCheck.cpp
clang-tidy/readability/AvoidStdBindCheck.h
clang-tidy/readability/CMakeLists.txt
clang-tidy/readabilit
jbcoe updated this revision to Diff 55970.
jbcoe marked 16 inline comments as done.
jbcoe added a comment.
Minor fixes from review.
http://reviews.llvm.org/D16962
Files:
clang-tidy/readability/AvoidStdBindCheck.cpp
clang-tidy/readability/AvoidStdBindCheck.h
clang-tidy/readability/CMakeLis
jbcoe added inline comments.
Comment at: clang-tidy/readability/ReadabilityTidyModule.cpp:37
@@ -35,1 +36,3 @@
+CheckFactories.registerCheck(
+"readability-avoid-std-bind");
CheckFactories.registerCheck(
aaron.ballman wrote:
> I kind of wonder if
aaron.ballman added inline comments.
Comment at: clang-tidy/readability/AvoidStdBindCheck.cpp:116
@@ +115,3 @@
+ Finder->addMatcher(
+ callExpr(callee(namedDecl(isStdBind())),
+ hasArgument(0, declRefExpr(to(functionDecl().bind("f")
alexfh
alexfh added inline comments.
Comment at: clang-tidy/readability/AvoidStdBindCheck.cpp:46
@@ +45,3 @@
+BindArgument B;
+if (const auto * M = dyn_cast(E)) {
+ const auto * TE = M->GetTemporaryExpr();
In LLVM style this should be `const auto *M`.
Please
jbcoe added inline comments.
Comment at: clang-tidy/readability/AvoidStdBindCheck.cpp:42
@@ +41,3 @@
+
+ for (size_t I = 1, ArgCount = C->getNumArgs(); I < ArgCount; ++I) {
+const Expr *E = C->getArg(I);
alexfh wrote:
> Please use a range-based for loop over
jbcoe updated this revision to Diff 50623.
jbcoe marked 15 inline comments as done.
jbcoe added a comment.
Apply requested fixes from review.
Thanks for taking time to review this, the patch is much improved for the
attention.
http://reviews.llvm.org/D16962
Files:
clang-tidy/readability/Avo
alexfh added inline comments.
Comment at: clang-tidy/readability/AvoidStdBindCheck.cpp:37
@@ +36,3 @@
+
+std::vector
+buildBindArguments(const MatchFinder::MatchResult &Result, const CallExpr *C) {
`SmallVector<>` would be better here, since the number of argument
alexfh added a comment.
In http://reviews.llvm.org/D16962#346822, @jbcoe wrote:
> Moved check to readability module.
>
> Aside: would it be worthwhile creating a python script to move checks from
> one module to another?
It's reasonable to teach the `rename_check.py` script move across modules
jbcoe updated this revision to Diff 47259.
jbcoe added a comment.
Moved check to readability module.
Aside: would it be worthwhile creating a python script to move checks from one
module to another?
http://reviews.llvm.org/D16962
Files:
clang-tidy/readability/AvoidStdBindCheck.cpp
clang-t
jbcoe removed rL LLVM as the repository for this revision.
jbcoe updated this revision to Diff 47247.
jbcoe added a comment.
Require C++14 for improved fixits.
Do not attempt to generate fixits for more complicated uses of bind.
http://reviews.llvm.org/D16962
Files:
clang-tidy/misc/AvoidStdB
jbcoe planned changes to this revision.
jbcoe added a comment.
Placeholder handling needs correcting.
Repository:
rL LLVM
http://reviews.llvm.org/D16962
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman
25 matches
Mail list logo
