Re: [PATCH] D20170: [clang-tidy] TypeTraits - Type is not expensive to copy when it has a deleted copy constructor.

alexfh added inline comments. Comment at: clang-tidy/utils/TypeTraits.cpp:20 @@ -18,1 +19,3 @@ +using namespace ::clang::ast_matchers; + aaron.ballman wrote: > I would prefer this be used locally instead of at namespace scope to avoid > potential name collision

Re: [PATCH] D18575: [clang-tidy] New checker to replace deprecated throw() specifications

alexfh requested changes to this revision. This revision now requires changes to proceed. Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:39 @@ +38,3 @@ + + const FunctionDecl *FuncDecl = + Result.Nodes.getNodeAs("functionDecl"); s/FunctionDecl/auto/

Re: [PATCH] D19547: [clang-tidy] Add FixIt for swapping arguments in string-constructor-checker.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG. http://reviews.llvm.org/D19547 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

Re: [PATCH] D16962: clang-tidy: avoid std::bind

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

Re: [PATCH] D19865: [clang-tidy] - PerformanceUnnecesaryCopyInitialization - only trigger for decl stmts with single VarDecl.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG. http://reviews.llvm.org/D19865 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

Re: [PATCH] D20053: [clang-tidy] Add misc-unnecessary-mutable check.

alexfh added a subscriber: alexfh. Comment at: docs/clang-tidy/checks/misc-unnecessary-mutable.rst:9 @@ +8,3 @@ + +.. code-block:: c++ + class SomeClass { Please verify the documentation can be built without errors. On Ubuntu this boils down to: Install sphinx:

Re: [PATCH] D20010: [clang-tidy] UnnecessaryCopyInitialization - Extend to trigger on non-const "this" object argument if it is not modified

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. How many more (in relative numbers) results does this check generate now? Comment at: clang-tidy/performance/UnnecessaryCopyInitialization.cpp:45 @@ -44,7 +44,3 @@

Re: [PATCH] D20182: [clang-tidy] Add missing dependency of libtooling to misc module

alexfh added a comment. LG. Thanks for fixing. Repository: rL LLVM http://reviews.llvm.org/D20182 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20180: [tooling] Fix missing inline keyworkd, breaking build bot.

alexfh added a subscriber: alexfh. alexfh added a comment. `inline` seems to be completely redundant here. Can you try removing it and running whatever build configurations were failing without http://reviews.llvm.org/D20182? Repository: rL LLVM http://reviews.llvm.org/D20180 ___

Re: [PATCH] D18575: [clang-tidy] New checker to replace deprecated throw() specifications

alexfh requested changes to this revision. This revision now requires changes to proceed. Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:36 @@ +35,3 @@ + +bool UseNoexceptCheck::helper(const MatchFinder::MatchResult &Result, + const SourceRange

Re: [PATCH] D20189: [tooling] Remove redundant inline keyword

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG http://reviews.llvm.org/D20189 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm

Re: [PATCH] D16962: clang-tidy: avoid std::bind

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

Re: [PATCH] D19703: [clang-tidy] Improve misc-redundant-expression and decrease false-positive

alexfh added inline comments. Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:139 @@ +138,3 @@ + const LangOptions &LO = Finder->getASTContext().getLangOpts(); + SourceLocation Loc = Node.getExprLoc(); + while (Loc.isMacroID()) { Ah, found it: `llvm/AD

Re: [PATCH] D17981: [clang-tidy] Fix clang-tidy to support parsing of assembly statements.

alexfh added a comment. Manuel, ping. http://reviews.llvm.org/D17981 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20170: [clang-tidy] TypeTraits - Type is not expensive to copy when it has a deleted copy constructor.

alexfh added inline comments. Comment at: clang-tidy/utils/TypeTraits.cpp:22 @@ +21,3 @@ + +using namespace ::clang::ast_matchers; + Now seems to be unused. http://reviews.llvm.org/D20170 ___ cfe-commits mailing lis

Re: [PATCH] D20218: [Tooling] Fix broken dependency for shared build

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG http://reviews.llvm.org/D20218 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm

Re: [PATCH] D20170: [clang-tidy] TypeTraits - Type is not expensive to copy when it has a deleted copy constructor.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good after addressing Etienne's comment. Thanks! http://reviews.llvm.org/D20170 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

Re: [PATCH] D18575: [clang-tidy] New checker to replace deprecated throw() specifications

alexfh requested changes to this revision. This revision now requires changes to proceed. Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:37 @@ +36,3 @@ +bool UseNoexceptCheck::checkHelper(const MatchFinder::MatchResult &Result, + const Sour

Re: [PATCH] D20196: [clang-tidy] Inefficient string operation

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. The last diff has nothing but a 2-line change in the docs. http://reviews.llvm.org/D20196 ___ cfe-commits mailing list cfe-commits@list

Re: [PATCH] D20279: [clang-tidy] Cleanups utils files

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG http://reviews.llvm.org/D20279 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm

Re: [PATCH] D20326: [clang-tidy] Fix a template function false positive in misc-unused-using-decls check.

alexfh added inline comments. Comment at: clang-tidy/misc/UnusedUsingDeclsCheck.cpp:24 @@ +23,3 @@ +AST_MATCHER(CallExpr, hasUnresolvedLookupExpr) { + return isa(Node.getCallee()); +} I think, we should use a node matcher for `UnresolvedLookupExpr` and instead u

Re: [PATCH] D20326: [clang-tidy] Fix a template function false positive in misc-unused-using-decls check.

alexfh requested changes to this revision. This revision now requires changes to proceed. Comment at: clang-tidy/misc/UnusedUsingDeclsCheck.cpp:24 @@ +23,3 @@ +// FIXME: Add a node matcher for UnresolvedLookupExpr in ASTMatcher. +AST_MATCHER(CallExpr, hasUnresolvedLookupExpr) { +

Re: [PATCH] D19841: [clang-tidy] Lift common matchers to utils namespace

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG. Thank you! Comment at: clang-tidy/utils/Matchers.h:20 @@ -19,1 +19,3 @@ +AST_MATCHER_P(Expr, ignoringImplicit, + ast_matchers::internal::Matcher, InnerMatch

Re: [PATCH] D18821: Add bugprone-bool-to-integer-conversion

alexfh requested changes to this revision. This revision now requires changes to proceed. Comment at: docs/clang-tidy/checks/bugprone-bool-to-integer-conversion.rst:43 @@ +42,3 @@ + +This is because z and ``true`` will be implicitly converted to int by promotion. +To get rid of t

Re: [PATCH] D19105: Changes in clang after running http://reviews.llvm.org/D18821

alexfh added a comment. In http://reviews.llvm.org/D19105#427046, @Prazek wrote: > In http://reviews.llvm.org/D19105#426903, @alexfh wrote: > > > returning a bool from a function that is declared to return a typedef to an > > integral type that contains `bool` in its name (e.g. `LLVMBool`), and

Re: [PATCH] D18575: [clang-tidy] New checker to replace deprecated throw() specifications

alexfh added inline comments. Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:22 @@ +21,3 @@ +namespace { + +StringRef makeDynamicExceptionString(const SourceManager &SM, It's much more common for LLVM code to use static functions: http://llvm.org/docs/Codi

Re: [PATCH] D18575: [clang-tidy] New checker to replace deprecated throw() specifications

alexfh added inline comments. Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:39 @@ +38,3 @@ + + // FIXME: Add paren matching so we can parse more complex throw statements, + // e.g., (examples provided by Aaron Ballman): aaron.ballman wrote: > alexfh wrot

Re: [PATCH] D18575: [clang-tidy] New checker to replace deprecated throw() specifications

alexfh added inline comments. Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:39 @@ +38,3 @@ + + // FIXME: Add paren matching so we can parse more complex throw statements, + // e.g., (examples provided by Aaron Ballman): aaron.ballman wrote: > alexfh wrot

Re: [PATCH] D20326: [clang-tidy] Fix a template function false positive in misc-unused-using-decls check.

alexfh added inline comments. Comment at: clang-tidy/misc/UnusedUsingDeclsCheck.cpp:24 @@ +23,3 @@ +// FIXME: Move this node matcher to ASTMatcher. +AST_MATCHER(Stmt, unresolvedLookupExpr) { + return isa(Node); That's because we need a node matcher, not narrowing

Re: [PATCH] D20326: [clang-tidy] Fix a template function false positive in misc-unused-using-decls check.

alexfh added inline comments. Comment at: clang-tidy/misc/UnusedUsingDeclsCheck.cpp:24 @@ +23,3 @@ +// FIXME: Move this node matcher to ASTMatcher. +AST_MATCHER(Stmt, unresolvedLookupExpr) { + return isa(Node); hokein wrote: > alexfh wrote: > > That's because we

Re: [PATCH] D20326: [clang-tidy] Fix a template function false positive in misc-unused-using-decls check.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG Comment at: clang-tidy/misc/UnusedUsingDeclsCheck.cpp:95 @@ +94,3 @@ + if (auto *ULE = Result.Nodes.getNodeAs("used")) { +for (auto I = ULE->decls_begin(), E = ULE->de

Re: [clang-tools-extra] r269896 - [clang-tidy] Fix a functional change from r269656.

Thank you for fixing this! On Wed, May 18, 2016 at 11:48 AM, Benjamin Kramer via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: d0k > Date: Wed May 18 04:48:46 2016 > New Revision: 269896 > > URL: http://llvm.org/viewvc/llvm-project?rev=269896&view=rev > Log: > [clang-tidy] Fix a func

Re: [PATCH] D20360: [ASTMatcher] Add a node matcher for UnresolvedLookupExpr.

alexfh added a comment. In http://reviews.llvm.org/D20360#432874, @aaron.ballman wrote: > The changes to docs/tools/dump_ast_matchers.py look to be spurious, can they > be reverted? The script should be executable, so the change looks fine to me. http://reviews.llvm.org/D20360 ___

Re: Patch submission for bug 27400

Thank you for the patch! Note, however, that most clang-tidy reviews are done using Phabricator (see llvm.org/docs/Phabricator.html). It's not required, but it makes the reviews much easier (and much easier to keep track of). On Tue, May 17, 2016 at 10:47 PM, Mads Ravn via cfe-commits < cfe-commit

Re: [PATCH] D20365: [PATCH] clang-tidy: Bug 27731 - modernize-pass-by-value suggest using std::move for types that perform copies on move

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG. Do you need me to submit the patch for you? Next time please create diff with the full context (http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface). Thank you

Re: [PATCH] D20196: [clang-tidy] Inefficient string operation

alexfh added a comment. Please create a diff with the full context: http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface Comment at: clang-tidy/performance/InefficientStringAdditionCheck.cpp:84 @@ +83,3 @@ + const auto DiagMsg = + "Inefficient s

Re: [PATCH] D20277: [clang-tidy] UnnecessaryValueParamCheck - suggest std::move() if non-const value parameter can be moved.

alexfh added inline comments. Comment at: clang-tidy/performance/UnnecessaryValueParamCheck.h:30 @@ -29,1 +29,3 @@ void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void registerPPCallbacks(clang::CompilerInstance &Compiler) override; + void storeOp

Re: [PATCH] D20360: [ASTMatcher] Add a node matcher for UnresolvedLookupExpr.

alexfh added a comment. In http://reviews.llvm.org/D20360#432882, @aaron.ballman wrote: > In http://reviews.llvm.org/D20360#432878, @alexfh wrote: > > > In http://reviews.llvm.org/D20360#432874, @aaron.ballman wrote: > > > > > The changes to docs/tools/dump_ast_matchers.py look to be spurious, ca

Re: [PATCH] D20369: [ASTMatcher] Fix a ASTMatcher test failure on Windows.

alexfh added inline comments. Comment at: unittests/ASTMatchers/ASTMatchersNodeTest.cpp:187 @@ +186,3 @@ + "}", + unresolvedLookupExpr(), true, + "-fno-delayed-template-parsing"))

Re: [PATCH] D15089: Patch to google checks in clang-tidy

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. As Eugene noted, the patch is seriously out of date. The only place where an old URL is still used, is docs/clang-tidy/checks/google-runtime-int.rst. However, we should check if styl

[clang-tools-extra] r270032 - [clang-tidy] Fix doc titles.

Author: alexfh Date: Thu May 19 04:29:46 2016 New Revision: 270032 URL: http://llvm.org/viewvc/llvm-project?rev=270032&view=rev Log: [clang-tidy] Fix doc titles. Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/google-runtime-references.rst clang-tools-extra/trunk/docs/clang-tidy

[clang-tools-extra] r270033 - [clang-tidy] Fix/add style guide links.

Author: alexfh Date: Thu May 19 04:31:30 2016 New Revision: 270033 URL: http://llvm.org/viewvc/llvm-project?rev=270033&view=rev Log: [clang-tidy] Fix/add style guide links. Thanks to Tim Halloran for the initial patch (http://reviews.llvm.org/D15089)! Modified: clang-tools-extra/trunk/clang-

Re: [PATCH] D15089: Patch to google checks in clang-tidy

alexfh added a comment. Anyways, committed all useful changes from here in r270033. Thank you for the patch! Repository: rL LLVM http://reviews.llvm.org/D15089 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin

Re: [PATCH] D20428: Tracking exception specification source locations

alexfh added a comment. Full context diff, please. > I'm not certain of the best way to test this functionality in isolation; Same way other locations/ranges are tested in unittests/AST/SourceLocationTest.cpp? http://reviews.llvm.org/D20428 ___

Re: [PATCH] D20429: [clang-tidy] Handle using-decls with more than one shadow decl.

alexfh requested changes to this revision. This revision now requires changes to proceed. Comment at: clang-tidy/misc/UnusedUsingDeclsCheck.cpp:22 @@ +21,3 @@ +namespace { +bool IsValidDecl(const Decl *TargetDecl) { + // Ignores using-declarations defined in macros. -

Re: [PATCH] D20428: Tracking exception specification source locations

alexfh added a comment. In http://reviews.llvm.org/D20428#434242, @aaron.ballman wrote: > In http://reviews.llvm.org/D20428#434238, @alexfh wrote: > > > Full context diff, please. > > > Pardon my complete ignorance, but how? I generated the diff from svn the > usual way, so I assume I've missed

Re: [clang-tools-extra] r261991 - [clang-tidy] Fix a crash issue when clang-tidy runs with compilation database.

On Thu, May 19, 2016 at 4:45 PM, Hans Wennborg wrote: > +Tom who manages 3.8.1 > +Alex who's owner of clang-tidy: is this ok for 3.8.1? > Yes, would be nice to have this in 3.8.1. This fixes a rather annoying problem. > > On Thu, May 19, 2016 at 1:56 AM, Edoardo P. via cfe-commits > wrote: >

Re: [PATCH] D20429: [clang-tidy] Handle using-decls with more than one shadow decl.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG with a couple of nits. Comment at: clang-tidy/misc/UnusedUsingDeclsCheck.cpp:22 @@ +21,3 @@ +// A function that helps to tell whether a TargetDecl will be checked. +// We o

Re: [PATCH] D20429: [clang-tidy] Handle using-decls with more than one shadow decl.

alexfh added inline comments. Comment at: clang-tidy/misc/UnusedUsingDeclsCheck.cpp:59 @@ -59,1 +58,3 @@ /*SkipTrailingWhitespaceAndNewLine=*/true)); +for (const auto It : Using->shadows()) { + const auto *TargetDecl = It->getTargetDecl()->getCanonicalDecl();

Re: [PATCH] D20463: [clang-tidy] Add more descriptive comments and examples in misc-definitions-in-headers check.

alexfh requested changes to this revision. This revision now requires changes to proceed. Comment at: docs/clang-tidy/checks/misc-definitions-in-headers.rst:8 @@ +7,3 @@ +If these headers are included from multiple translation units, there will be +redefinition linker errors. + --

Re: [PATCH] D20196: [clang-tidy] Inefficient string operation

alexfh requested changes to this revision. This revision now requires changes to proceed. Comment at: clang-tidy/performance/InefficientStringAdditionCheck.cpp:28 @@ +27,3 @@ +: ClangTidyCheck(Name, Context), + IsStrictMode(Options.get("isStrictMode", 0)) {} + ---

Re: [PATCH] D20463: [clang-tidy] Add more descriptive comments and examples in misc-definitions-in-headers check.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG. Thank you! http://reviews.llvm.org/D20463 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listi

Re: [PATCH] D18575: [clang-tidy] New checker to replace deprecated throw() specifications

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. Let's wait for http://reviews.llvm.org/D20428 http://reviews.llvm.org/D18575 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[clang-tools-extra] r270215 - [clang-tidy] Switch to a more common way of customizing check behavior.

Author: alexfh Date: Fri May 20 08:42:40 2016 New Revision: 270215 URL: http://llvm.org/viewvc/llvm-project?rev=270215&view=rev Log: [clang-tidy] Switch to a more common way of customizing check behavior. This should have been done this way from the start, however I somehow missed r257177. Modif

Re: [PATCH] D18408: readability check for const params in declarations

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good with one nit. Thank you for the new check! Do you need me to submit the patch for you? Comment at: clang-tidy/readability/AvoidConstParamsInDecls.cpp:24 @@ +23,3 @

Re: [PATCH] D18475: [clang-tidy] Add more detection rules for redundant c_str calls.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good to me. Adding Samuel, since he has done a lot of similar stuff by now and might have good ideas on improving this and making this more general. http://reviews.llvm.org/D18475 __

Re: [PATCH] D18457: [clang-tidy] Add a new checker to detect missing comma in initializer list.

alexfh added inline comments. Comment at: clang-tidy/misc/SuspiciousMissingCommaCheck.cpp:42 @@ +41,3 @@ + const auto *ConcatenatedLiteral = Result.Nodes.getNodeAs("str"); + if (InitializerList && ConcatenatedLiteral) { +// Skip small arrays as they often generate false-posi

Re: [PATCH] D18265: [clang-tidy] New: checker misc-assign-operator-return

alexfh added a comment. Adding the original author in case he has something to say here. Comment at: clang-tidy/misc/AssignOperatorCheck.h:24 @@ +23,3 @@ +/// * Works with move-assign and assign by value. +/// * Private and deleted operators are ignored. +class AssignOperato

Re: [PATCH] D18243: [ASTMatchers] Existing matcher hasAnyArgument fixed

alexfh accepted this revision. alexfh added a reviewer: alexfh. alexfh added a comment. No, thank you for the fix! LG http://reviews.llvm.org/D18243 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listi

Re: [PATCH] D18265: [clang-tidy] New: checker misc-assign-operator-return

alexfh added a comment. > What about NonIdiomaticAddignOperator or UnchainableAssignOperator? Yep, "unchainable" doesn't cover all problems the check detects. `misc-non-idiomatic-assign-operator` seems good though. I'd wait for the original author to chime in before making the change. http:/

Re: [PATCH] D18408: readability check for const params in declarations

This revision was automatically updated to reflect the committed changes. Closed by commit rL264856: [clang-tidy] readability check for const params in declarations (authored by alexfh). Changed prior to commit: http://reviews.llvm.org/D18408?vs=51954&id=52037#toc Repository: rL LLVM http:/

[clang-tools-extra] r264856 - [clang-tidy] readability check for const params in declarations

Author: alexfh Date: Wed Mar 30 06:31:33 2016 New Revision: 264856 URL: http://llvm.org/viewvc/llvm-project?rev=264856&view=rev Log: [clang-tidy] readability check for const params in declarations Summary: Adds a clang-tidy warning for top-level consts in function declarations. Reviewers: hokei

Re: [PATCH] D18408: readability check for const params in declarations

alexfh added a comment. Actually, we missed one thing: may I ask you to update docs/ReleaseNotes.rst with a short description of the new check? Repository: rL LLVM http://reviews.llvm.org/D18408 ___ cfe-commits mailing list cfe-commits@lists.llv

[clang-tools-extra] r264858 - [docs] Added 3.8 clang-tidy release notes, fixed formatting.

Author: alexfh Date: Wed Mar 30 07:05:33 2016 New Revision: 264858 URL: http://llvm.org/viewvc/llvm-project?rev=264858&view=rev Log: [docs] Added 3.8 clang-tidy release notes, fixed formatting. Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst Modified: clang-tools-extra/trunk/docs/Rel

Re: [PATCH] D18582: [Clang-tidy] Update release notes with list of checks added since 3.8

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good with a nit. Thank you! Comment at: docs/ReleaseNotes.rst:129 @@ +128,3 @@ + + This check looks for procedures (functions returning no value) with `return` + state

Re: [PATCH] D18582: [Clang-tidy] Update release notes with list of checks added since 3.8

alexfh added a comment. A few more nits. Comment at: docs/ReleaseNotes.rst:68 @@ +67,3 @@ + + This check flags calls to ``system()``, ``popen()``, and ``_popen()``, which + execute a command processor. I'd remove "This check" at the start of all check descript

[clang-tools-extra] r264862 - [clang-tidy] Fix MSVC build.

Author: alexfh Date: Wed Mar 30 07:35:05 2016 New Revision: 264862 URL: http://llvm.org/viewvc/llvm-project?rev=264862&view=rev Log: [clang-tidy] Fix MSVC build. Modified: clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.h Modified: clang-tools-extra/trunk/clang-tidy/r

Re: [clang-tools-extra] r264856 - [clang-tidy] readability check for const params in declarations

m\tools\clang\tools\extra\clang-tidy\readability\ReadabilityTidyModule.cpp(37) > : see reference to function template instantiation 'void > clang::tidy::ClangTidyCheckFactories::registerCheck(llvm::StringRef)' > being compiled > > Can you take a look? > > > On

Re: [PATCH] D18265: [clang-tidy] New: checker misc-assign-operator-return

alexfh added a comment. In http://reviews.llvm.org/D18265#386717, @aaron.ballman wrote: > In http://reviews.llvm.org/D18265#386676, @alexfh wrote: > > > > What about NonIdiomaticAddignOperator or UnchainableAssignOperator? > > > > > > Yep, "unchainable" doesn't cover all problems the check detect

Re: [PATCH] D18265: [clang-tidy] New: checker misc-assign-operator-return

alexfh added a comment. In http://reviews.llvm.org/D18265#386720, @baloghadamsoftware wrote: > Actually, there was nothing wrong with assign operator signatures per se > either although the original name of the checker was AssignOperatorSignature. > The only change here is that it does not chec

Re: [PATCH] D18457: [clang-tidy] Add a new checker to detect missing comma in initializer list.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good with a few comments. Further functionality improvements can be done in a follow up, I think. Please update docs/ReleaseNotes.rst. Comment at: clang-tidy/misc/Susp

Re: [PATCH] D18457: [clang-tidy] Add a new checker to detect missing comma in initializer list.

alexfh added inline comments. Comment at: clang-tidy/misc/SuspiciousMissingCommaCheck.cpp:75 @@ +74,3 @@ + diag(ConcatenatedLiteral->getLocStart(), + "suspicious string literal, probably missing a comma"); +} xazax.hun wrote: > alexfh wrote: > > We need to

Re: [PATCH] D18582: [Clang-tidy] Update release notes with list of checks added since 3.8

alexfh added a comment. In http://reviews.llvm.org/D18582#387525, @Eugene.Zelenko wrote: > Sections headers are not problem :-) > > See newly added "Clang-tidy changes from 3.7 to 3.8" section. It contains > only names of checks with link to documentation page without brief > description. The

[clang-tools-extra] r265007 - note for top-level consts in function decls tidy

Author: alexfh Date: Thu Mar 31 07:06:47 2016 New Revision: 265007 URL: http://llvm.org/viewvc/llvm-project?rev=265007&view=rev Log: note for top-level consts in function decls tidy Summary: Add missing release note Reviewers: alexfh Subscribers: LegalizeAdulthood, cfe-commits Patch by Matt Ku

[clang-tools-extra] r265008 - [docs] Fix a typo, change the style of the clang-tidy release notes a bit.

Author: alexfh Date: Thu Mar 31 07:06:54 2016 New Revision: 265008 URL: http://llvm.org/viewvc/llvm-project?rev=265008&view=rev Log: [docs] Fix a typo, change the style of the clang-tidy release notes a bit. Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst Modified: clang-tools-extra/

Re: [PATCH] D18608: note for top-level consts in function decls tidy

This revision was automatically updated to reflect the committed changes. Closed by commit rL265007: note for top-level consts in function decls tidy (authored by alexfh). Changed prior to commit: http://reviews.llvm.org/D18608?vs=52126&id=52187#toc Repository: rL LLVM http://reviews.llvm.o

Re: [PATCH] D18608: note for top-level consts in function decls tidy

alexfh added inline comments. Comment at: docs/ReleaseNotes.rst:73 @@ +72,3 @@ + + This check warns about top-level const parameters in function delcartions. + Typo: "delcartions". Fixed in a follow up. Repository: rL LLVM http://reviews.llvm.org/D18608 _

Re: [PATCH] D18608: note for top-level consts in function decls tidy

alexfh added a comment. Thank you for the patch, btw! Repository: rL LLVM http://reviews.llvm.org/D18608 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18584: Complete support for C++ Core Guidelines Type.6: Always initialize a member variable.

alexfh added a comment. Thank you for the patch! I'd like the original author to take a look at it, so just a couple of peanut gallery comments from me for now. Comment at: clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp:51 @@ +50,3 @@ +static bool +isTriviallyConstruct

Re: [PATCH] D18509: clang-tidy: add_new_check.py stubs out release notes

alexfh added a comment. I like the idea, but we could make this a bit more reliable, see the inline comment. Comment at: clang-tidy/add_new_check.py:280 @@ +279,3 @@ + if not clang_tidy_found: +if line.startswith("Improvements to ``clang-tidy``"): + clang_

Re: [PATCH] D18408: readability check for const params in declarations

On Wed, Mar 30, 2016 at 4:01 PM, Matthew Fowles Kulukundis < matt.fow...@gmail.com> wrote: > My attempts to do this end with: > > $ arc diff > I normally use `arc diff --create` or `arc diff --update D`, and this works. > Linting... > No lint engine configured for this project. > Running un

Re: [PATCH] D18582: [Clang-tidy] Update release notes with list of checks added since 3.8

alexfh added a comment. In http://reviews.llvm.org/D18582#388496, @Eugene.Zelenko wrote: > Updated per Etienne comments. > > Links are still need to be added, but I'm not clear about address. I'd point the links to the live version (e.g. http://clang.llvm.org/extra/clang-tidy/checks/readabilit

Re: [PATCH] D18582: [Clang-tidy] Update release notes with list of checks added since 3.8

alexfh added inline comments. Comment at: docs/clang-tidy/checks/performance-implicit-cast-in-loop.rst:4 @@ -3,3 +3,3 @@ -This warning appears in range-based loop with loop variable of const ref type +This warning appears in range-based loop with loop a variable of const ref typ

Re: [PATCH] D17926: [clang-tidy] Don't delete unused parameter in class override method in anonymous namespace.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG. Thank you! http://reviews.llvm.org/D17926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listi

Re: [PATCH] D17926: [clang-tidy] Don't delete unused parameter in class override method in anonymous namespace.

alexfh added inline comments. Comment at: clang-tidy/misc/UnusedParametersCheck.cpp:80 @@ -78,2 +79,3 @@ !Result.SourceManager->isInMainFile(Function->getLocation()) || - UsedByRef()) { + !ast_matchers::match(DeclRefExpr, *Result.Context).empty()|| + isOverri

Re: [PATCH] D18582: [Clang-tidy] Update release notes with list of checks added since 3.8

alexfh added inline comments. Comment at: docs/ReleaseNotes.rst:67 @@ -67,1 +66,3 @@ +- New `cert-env33-c + `_ check Should these lines be indented to `New ...` (i.e. with just two spaces)?

Re: [PATCH] D18582: [Clang-tidy] Update release notes with list of checks added since 3.8

alexfh added a comment. One more nit. Comment at: docs/clang-tidy/checks/performance-implicit-cast-in-loop.rst:4 @@ -3,3 +3,3 @@ -This warning appears in range-based loop with loop variable of const ref type +This warning appears in range-based loop with a loop variable of con

Re: [PATCH] D18396: Clang-tidy:modernize-use-override. Fix for __declspec attributes and const=0 without spacse

alexfh added a comment. In http://reviews.llvm.org/D18396#388287, @Rob wrote: > I tried > > check_clang_tidy.py modernize-use-override-ms.cpp modernize-use-override > temp -- -- -fms-extensions It also needs -std=c++11, which needs to be specified, when you specify any options manually. >

Re: [PATCH] D18396: Clang-tidy:modernize-use-override. Fix for __declspec attributes and const=0 without spacse

alexfh added a comment. In http://reviews.llvm.org/D18396#388287, @Rob wrote: > I tried > > check_clang_tidy.py modernize-use-override-ms.cpp modernize-use-override > temp -- -- -fms-extensions It also needs -std=c++11, since you specify options manually and "opt out" of the defaults (which

Re: [PATCH] D18582: [Clang-tidy] Update release notes with list of checks added since 3.8

alexfh added a comment. In http://reviews.llvm.org/D18582#388931, @Eugene.Zelenko wrote: > I align them as in 3.8 changes, to first `. If you think that alignment > should be other way, I'll change it. This observation doesn't capture the intent correctly. They are aligned with the first colu

Re: [PATCH] D18582: [Clang-tidy] Update release notes with list of checks added since 3.8

alexfh added a comment. Good to submit now. Thank you for working on this! Repository: rL LLVM http://reviews.llvm.org/D18582 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18509: clang-tidy: add_new_check.py stubs out release notes

alexfh added a comment. In http://reviews.llvm.org/D18509#388071, @hintonda wrote: > With this change, won't you need to update 2 different repos: clang and > extra? Is it possible to update both with a single Phabricator patch? Clang-tidy release notes are now in the clang-tools-extra reposi

Re: [PATCH] D18649: [clang-tidy] cppcoreguidelines-interfaces-global-init

alexfh added a comment. In http://reviews.llvm.org/D18649#389294, @courbet wrote: > In http://reviews.llvm.org/D18649#388337, @Eugene.Zelenko wrote: > > > Please mention new check in docs/ReleaseNotes.rst. > > > That should be in a different commit, right ? Release notes are in a > different rep

Re: [PATCH] D18649: [clang-tidy] cppcoreguidelines-interfaces-global-init

alexfh added a comment. Thank you for working on the new clang-tidy check! We usually recommend authors to run their checks on a large code base to ensure it doesn't crash and doesn't generate obvious false positives. It would be nice, if you could provide a quick summary of such a run (total n

Re: [PATCH] D18695: [clang-tidy] Reduce false-positive ratio in misc-suspicious-missing-comma check.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good! http://reviews.llvm.org/D18695 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo

Re: [PATCH] D18442: A clang-tidy check for std:accumulate.

alexfh added a comment. Sorry for the delay (mostly due to the holidays here). The check looks very useful, at least, the pattern is hard to spot by manual inspection. A few comments though, mostly style-related. Comment at: clang-tidy/misc/FoldInitTypeCheck.cpp:21 @@ +20,3 @@

Re: [PATCH] D18180: [clang-tidy] Add a check to detect static definitions in anonymous namespace.

alexfh added a comment. Sorry for the delay. A few minor issues. Comment at: clang-tidy/readability/StaticDefinitionInAnonymousNamespaceCheck.cpp:46 @@ +45,3 @@ + auto Diag = diag(Def->getLocation(), "'%0' is a static definition in " + "an

Re: [PATCH] D18694: [ClangTidy] Add an 'explain-checks' option to diagnose where each checks comes from.

alexfh added a comment. Nice! I like the idea, but we need to figure out a bunch of details (see the comments inline). Comment at: clang-tidy/ClangTidyOptions.cpp:264 @@ +263,3 @@ + ParsedOptions->CheckSources[*ParsedOptions->Checks] = + std::string(ConfigFile.c_s

Re: [PATCH] D18649: [clang-tidy] cppcoreguidelines-interfaces-global-init

alexfh added inline comments. Comment at: clang-tidy/cppcoreguidelines/InterfacesGlobalInitCheck.cpp:46 @@ +45,3 @@ + "initializing static variable with non-const expression depending on " + "static variable '%0'.") + << Referencee->getName(); nit

Re: [PATCH] D17987: [clang-tidy] Extension of checker misc-misplaced-widening-cast

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good! Thank you for addressing the comments. Comment at: clang-tidy/misc/MisplacedWideningCastCheck.cpp:117 @@ +116,3 @@ +static llvm::SmallDenseMap createRelativeCharSi

<    7   8   9   10   11   12   13   14   15   16   >