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 VS2013 supports inline member initializers? ================ Comment at: clang-tidy/modernize/AvoidBindCheck.cpp:77 @@ +76,3 @@ + for (size_t I = 1; I <= PlaceholderCount; ++I) { + Stream << Delimiter << "auto &&" + << " arg" << I; ---------------- clang-format, please. Also, please join the two string literals here. ================ Comment at: clang-tidy/modernize/AvoidBindCheck.cpp:116 @@ +115,3 @@ +void AvoidBindCheck::check(const MatchFinder::MatchResult &Result) { + if (!getLangOpts().CPlusPlus14) // Need C++14 for generic lambdas. + return; ---------------- This check should be done in `registerMatchers` to avoid unnecessary work. ================ Comment at: clang-tidy/modernize/AvoidBindCheck.cpp:161 @@ +160,3 @@ + MatchedDecl->getLocStart(), + Lexer::getLocForEndOfToken(MatchedDecl->getLocEnd(), 0, + *Result.SourceManager, ---------------- I don't think you need `getLocForEndOfToken`, since `FixItHint::CreateReplacement(SourceRange)` treats the range as a token range. ================ Comment at: docs/clang-tidy/checks/modernize-avoid-bind.rst:26 @@ +25,3 @@ + +.. code:: C++ + void f() { ---------------- `..code:: C++` directive should be followed by an empty line. Please check the documentation builds without warnings. ================ Comment at: docs/clang-tidy/checks/modernize-avoid-bind.rst:32 @@ +31,3 @@ + +We created this check because ``std::bind`` can be hard to read and can result +in larger object files and binaries due to type information that will not be ---------------- s/We created this check because // ================ Comment at: docs/clang-tidy/checks/readability-avoid-std-bind.rst:13 @@ +12,3 @@ + +.. code:: C++ + int add(int x, int y) { return x + y; } ---------------- alexfh wrote: > `.. code::` should be followed by an empty line. > > Please also verify the documentation can be built without errors. On Ubuntu > this boils down to: > > 1. Install sphinx: > > $ sudo apt-get install sphinx-common > > 2. Enable `LLVM_BUILD_DOCS` and maybe some other options in cmake. > > 3. `ninja docs-clang-tools-html` (or something similar, if you use make). This comment is not addressed. ================ Comment at: test/clang-tidy/modernize-avoid-bind.cpp:25 @@ +24,3 @@ + auto clj = std::bind(add, x, y); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind [modernize-avoid-bind] + // CHECK-FIXES: auto clj = [=] { return add(x, y); }; ---------------- Remove `[modernize-avoid-bind]` from all CHECK-MESSAGES lines except for the first one. http://reviews.llvm.org/D16962 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits