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

Re: [PATCH] D18717: [Clang-tidy] Improve checks documentation consistency

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Awesome! Thanks! Repository: rL LLVM http://reviews.llvm.org/D18717 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.

Re: [PATCH] D18717: [Clang-tidy] Improve checks documentation consistency

alexfh added a comment. Also, could you, please, next time generate diffs with full context as documented in http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface (or use arcanist, which does this automatically)? This sometimes (not in this case) saves reviewers manua

Re: [PATCH] D15332: new clang-tidy checker readability-non-const-parameter

alexfh added inline comments. Comment at: clang-tidy/readability/NonConstParameterCheck.cpp:44 @@ +43,3 @@ +addParm(Parm); + } else if (const CXXConstructorDecl *Ctor = + Result.Nodes.getNodeAs("Ctor")) { `const auto *Ctor` C

Re: [PATCH] D17043: Check that the result of a library call w/o side effects is used

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. What's the status of this patch? Do you still want to continue working on it or are you fine with the warn_unused_result/nodiscard-based solution? http://reviews.llvm.org/D17043

Re: [PATCH] D18136: boost-use-to-string check

alexfh requested changes to this revision. This revision now requires changes to proceed. Comment at: clang-tidy/boost/UseToStringCheck.cpp:55 @@ +54,3 @@ +void UseToStringCheck::check(const MatchFinder::MatchResult &Result) { + const auto *MatchedToString = Result.Nodes.getNodeA

Re: [PATCH] D18191: [clang-tidy] Add check for function parameters that are const& to builtin types

alexfh added a comment. Adding Samuel, who's written a similar check internally and might want to upstream it or suggest improvements to this patch. http://reviews.llvm.org/D18191 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists

Re: [PATCH] D15032: [clang-tidy] new checker cppcoreguidelines-pro-lifetime

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. FYI, I'm waiting with reviewing this change until http://reviews.llvm.org/D15031 is landed, since it can affect this patch. http://reviews.llvm.org/D15032 ___

Re: [PATCH] D16248: [Clang-tidy] rename misc-inefficient-algorithm to performance-inefficient-algorithm

alexfh added a comment. Please rebase the patch and add full context to the diffs (see http://llvm.org/docs/Phabricator.html). Comment at: docs/clang-tidy/checks/misc-inefficient-algorithm.rst:4 @@ -5,1 +3,3 @@ +.. meta:: + :http-equiv=refresh: 5;URL=performance-inefficient-a

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:16 @@ +15,3 @@ + +// FIXME: Should this be moved to ASTMatchers.h? +namespace ast_matchers { Yes, it might make sense to

[clang-tools-extra] r265210 - [clang-tidy] Update an example. NFC.

Author: alexfh Date: Fri Apr 1 22:44:23 2016 New Revision: 265210 URL: http://llvm.org/viewvc/llvm-project?rev=265210&view=rev Log: [clang-tidy] Update an example. NFC. Modified: clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py Modified: clang-tools-extra/trunk/test/clang-tidy/ch

Re: [PATCH] D17482: [clang-tidy] Allow tests to verify changes made to header files

alexfh added a comment. In http://reviews.llvm.org/D17482#387457, @LegalizeAdulthood wrote: > I'm sorry to be such a nag and I know you're busy, but > > This changeset has been held up for a month and a half. (6 weeks since > originally posted in http://reviews.llvm.org/D16953) > > The chang

Re: [PATCH] D16183: Added CheckName field to YAML report

alexfh added a comment. Sorry for the lng delay. I missed you patch among all other ones. Could you rebase your patch on top of HEAD and clang-format the files you changed? I'll come back with more substantial comments early next week. http://reviews.llvm.org/D16183

Re: [PATCH] D18703: [clang-tidy] Add new checker for comparison with runtime string functions.

alexfh added inline comments. Comment at: clang-tidy/misc/SuspiciousStringCompareCheck.cpp:38 @@ +37,3 @@ + callExpr(hasDeclaration(functionDecl( + hasAnyName("__builtin_memcmp", + "__builtin_strcasecmp", Should we add a configura

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

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 patch! Please tell, if you need someone to commit the patch for you. Comment at: docs/ReleaseNotes.rst:73 @@ +72,3 @@ + + The fix-

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

alexfh added inline comments. Comment at: test/clang-tidy/modernize-use-override-ms.cpp:2 @@ +1,3 @@ +// RUN: %check_clang_tidy %s modernize-use-override %t -- -- -fms-extensions -std=c++11 + +// This test is designed to test ms-extension __declspec(dllexport) attributes. ---

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

alexfh added a comment. In http://reviews.llvm.org/D18649#390862, @courbet wrote: > In http://reviews.llvm.org/D18649#389363, @alexfh wrote: > > > 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 doe

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

alexfh added inline comments. Comment at: clang-tidy/readability/StaticDefinitionInAnonymousNamespaceCheck.cpp:52 @@ +51,3 @@ + "anonymous namespace; static is redundant here") + << Def->getName(); + Token Tok; `DiagnosticBuild

Re: [PATCH] D18745: [clang-tidy] Adds misc-use-bool-literals check.

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. 1. Looks like `modernize` would be a better fit for this check, since it targets a pattern common for pre-standard C++ (or C code). 2. Please clang-format the code. 3. Please run the

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

alexfh added a comment. The patch doesn't apply cleanly. Please rebase it on top of HEAD. http://reviews.llvm.org/D18396 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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#391031, @Rob wrote: > ok, I'm not sure if its worth rolling back the change to > modernize-use-override-ms.cpp, see ammended comments above. No, the test is fine as it is. > Apart from this it should be good to go. > > I think I am

[clang-tools-extra] r265298 - [clang-tidy] fix a couple of modernize-use-override bugs

Author: alexfh Date: Mon Apr 4 09:31:36 2016 New Revision: 265298 URL: http://llvm.org/viewvc/llvm-project?rev=265298&view=rev Log: [clang-tidy] fix a couple of modernize-use-override bugs Fix for __declspec attributes and const=0 without space This patch is to address 2 problems I found with

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL265298: [clang-tidy] fix a couple of modernize-use-override bugs (authored by alexfh). Changed prior to commit: http://reviews.llvm.org/D18396?vs=52543&id=52555#toc Repository: rL LLVM http://review

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#391084, @alexfh wrote: > The patch doesn't apply cleanly. Please rebase it on top of HEAD. Actually, the conflict was only in the ReleaseNotes.rst file, which I had to change anyway to fix the formatting around the place you changed. Th

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

alexfh added a comment. In http://reviews.llvm.org/D18649#391112, @courbet wrote: > In http://reviews.llvm.org/D18649#391001, @alexfh wrote: > > > In http://reviews.llvm.org/D18649#390862, @courbet wrote: > > > > > In http://reviews.llvm.org/D18649#389363, @alexfh wrote: > > > > > > > Thank you f

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

alexfh added inline comments. Comment at: clang-tidy/tool/ClangTidyMain.cpp:329 @@ +328,3 @@ +for (const std::string& Check : EnabledChecks) { + for (const ClangTidyOptions::StringPair &CheckSource: + EffectiveOptions.CheckSources) { hokein wrot

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

alexfh added inline comments. Comment at: clang-tidy/readability/StaticDefinitionInAnonymousNamespaceCheck.cpp:49 @@ +48,3 @@ + + DiagnosticBuilder Diag = + diag(Def->getLocation(), "%0 is a static definition in " nit: `auto`? Comment at:

Re: [PATCH] D18764: [clang-tidy] Fix documentation of misc-suspicious-missing-comma

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

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

alexfh added inline comments. Comment at: docs/clang-tidy/checks/readability-static-definition-in-anonymous-namespace.rst:9 @@ +8,3 @@ +In this case, `static` is redundant, because anonymous namespace limits the +visibility of definitions to a single translation unit. + -

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

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

Re: [PATCH] D18765: [clang-tidy] Don't complain about const pass_object_size params in declarations

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. There's a principal difference between top-level `const` on parameters in definitions and in declarations: in definitions `const` has an effect, as it makes the variable constant, bu

Re: [PATCH] D18765: [clang-tidy] Don't complain about const pass_object_size params in declarations

alexfh added a comment. To clarify: the `pass_object_size` attribute is not the only reason to mark parameter `const` in the definition. However, there's no reason to also mark them `const` in declarations, since that `const` is not a part of the function's interface. http://reviews.llvm.org/

Re: [clang-tools-extra] r264563 - clang-tidy: Fix broken buildbot

I'm not sure I understand what this test has to do with VS2013. Clang-tidy should be able to parse this code, and if it doesn't (e.g. due to -fms-compatibility being turned on by default on windows), we should put unsupported constructs under an appropriate #ifdef, not comment them out completely.

Re: [PATCH] D17586: Add a new check, readability-redundant-string-init, that checks unnecessary string initializations.

alexfh added a comment. Shuai, if you have time, could you take a look at http://llvm.org/PR27087? Thanks! Repository: rL LLVM http://reviews.llvm.org/D17586 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/m

Re: [PATCH] D18803: [clang-tidy] fix building clang-tidy documentation.

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

Re: [PATCH] D18806: [clang-tidy] filter plugins and plugin arguments of the command-line

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG in general. A few nits. Comment at: clang-tidy/ClangTidy.cpp:439 @@ +438,3 @@ +CommandLineArguments AdjustedArgs; +for (size_t i = 0, e = Args.size(); i != e; ++i)

Re: [PATCH] D18783: [clang-tidy] add new checker for string literal with NUL character.

alexfh added a comment. A couple of nits for now. Will take a closer look later. Comment at: clang-tidy/misc/StringLiteralWithEmbeddedNulCheck.cpp:22 @@ +21,3 @@ +static unsigned int GetCharAt(const StringLiteral *SL, size_t offset) { + if (offset >= SL->getLength()) return 0;

Re: [PATCH] D18852: [clang-tidy] fix a crash with -fdelayed-template-parsing in UnnecessaryValueParamCheck.

alexfh added a comment. Please add a test file with -fms-compatibility or -fdelayed-template-parsing. http://reviews.llvm.org/D18852 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18783: [clang-tidy] add new checker for string literal with NUL character.

alexfh added a comment. A couple of nits. Comment at: clang-tidy/misc/MisplacedWideningCastCheck.cpp:64 @@ -62,5 +63,3 @@ unsigned RHSWidth = getMaxCalculationWidth(Context, Bop->getRHS()); -if (Bop->getOpcode() == BO_Mul) - return LHSWidth + RHSWidth; -if (Bop

Re: [PATCH] D18783: [clang-tidy] add new checker for string literal with NUL character.

alexfh added a comment. Wait, it looks like you've updated the patch from an incorrect branch: I see only clang-tidy/misc/MisplacedWideningCastCheck.cpp file here. http://reviews.llvm.org/D18783 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[clang-tools-extra] r265655 - [docs] Update version (http://llvm.org/PR27253)

Author: alexfh Date: Thu Apr 7 05:17:23 2016 New Revision: 265655 URL: http://llvm.org/viewvc/llvm-project?rev=265655&view=rev Log: [docs] Update version (http://llvm.org/PR27253) Modified: clang-tools-extra/trunk/docs/conf.py Modified: clang-tools-extra/trunk/docs/conf.py URL: http://llvm

Re: [PATCH] D18829: [clang-tidy] Fix FP with readability-redundant-string-init for default arguments

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Awesome! Thanks for the fix! http://reviews.llvm.org/D18829 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin

Re: [PATCH] D18833: [clang-tidy] Fix infinite loop in MisplacedWideningCastCheck.

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. Formatting is broken. `clang-format -style=file`, please. http://reviews.llvm.org/D18833 ___ cfe-commits mailing list cfe-commits@lists

Re: [PATCH] D18810: [Clang-tidy] Fix readability-static-definition-in-anonymous-namespace warnings; other minor fixes.

alexfh added a comment. Please regenerate the patch with the full context. Comment at: clang-tidy/ClangTidy.cpp:39 @@ -38,1 +38,3 @@ #include "clang/Tooling/Tooling.h" +#include "llvm/ADT/SmallString.h" +#include "llvm/ADT/SmallVector.h" Did you make these chan

Re: [PATCH] D18833: [clang-tidy] Fix infinite loop in MisplacedWideningCastCheck.

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

Re: [PATCH] D18852: [clang-tidy] fix a crash with -fdelayed-template-parsing in UnnecessaryValueParamCheck.

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

Re: [PATCH] D18833: [clang-tidy] Fix infinite loop in MisplacedWideningCastCheck.

alexfh added a comment. Missed a couple of nits. Comment at: clang-tidy/misc/MisplacedWideningCastCheck.cpp:98 @@ -96,40 +97,3 @@ -static llvm::SmallDenseMap createRelativeIntSizesMap() { - llvm::SmallDenseMap Result; - Result[BuiltinType::UChar] = 1; - Result[BuiltinType::

Re: [clang-tools-extra] r265680 - [clang-tidy] Remove unnecessary getName() on Decls and Types feeding into a DiagnosticBuilder

Thanks! On Thu, Apr 7, 2016 at 4:55 PM, Benjamin Kramer via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: d0k > Date: Thu Apr 7 09:55:25 2016 > New Revision: 265680 > > URL: http://llvm.org/viewvc/llvm-project?rev=265680&view=rev > Log: > [clang-tidy] Remove unnecessary getName() on

Re: [PATCH] D18703: [clang-tidy] Add new checker for comparison with runtime string functions.

alexfh added inline comments. Comment at: clang-tidy/misc/SuspiciousStringCompareCheck.cpp:73 @@ +72,3 @@ + functionDecl( + hasAnyName("__builtin_memcmp", + "__builtin_strcasecmp", I'd initialize the list as a comma/semicolon-sep

Re: [PATCH] D18783: [clang-tidy] add new checker for string literal with NUL character.

alexfh requested changes to this revision. This revision now requires changes to proceed. Comment at: docs/clang-tidy/checks/misc-string-literal-with-embedded-nul.rst:6 @@ +5,3 @@ + +Find occurences of string literal with embedded NUL character and validate +their usage. -

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

alexfh added a comment. Actually, did you think about adding this as a clang diagnostic? Richard, what do you think about complaining in Clang about `int i = true;` kind of code? Comment at: clang-tidy/modernize/BoolToIntegerConversionCheck.cpp:49 @@ +48,3 @@ +diag(BoolLit

Re: [PATCH] D18783: [clang-tidy] add new checker for string literal with NUL character.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG with one nit. Comment at: clang-tidy/misc/StringLiteralWithEmbeddedNulCheck.cpp:70 @@ +69,3 @@ +diag(SL->getLocStart(), "suspicious embedded NUL character"); +

Re: [PATCH] D18766: [clang-tidy] Add check misc-multiple-statement-macro

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG with a nit. Comment at: clang-tidy/misc/MultipleStatementMacroCheck.cpp:99 @@ +98,3 @@ + + diag(InnerRanges.back().first, "multiple statement macro spans unbraced " +

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

alexfh added inline comments. Comment at: clang-tidy/modernize/BoolToIntegerConversionCheck.cpp:47 @@ +46,3 @@ + const auto Type = Cast->getType().getLocalUnqualifiedType(); + if (isPreprocessorIndependent(BoolLiteral, Result)) { +diag(BoolLiteral->getLocation(), "implicitly

Re: [PATCH] D18797: [Clang-tidy] Mention readability-static-definition-in-anonymous-namespace in release notes

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

Re: [PATCH] D17482: [clang-tidy] Allow tests to verify changes made to header files

alexfh added a comment. Let's try to turn this to a productive lane. I'm not against adding this functionality, if it's actually useful, but as is the patch doesn't meet the bar. Right now, there are a few action items. First, could you split the refactorings and send me as a separate patch? Th

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

alexfh added inline comments. Comment at: clang-tidy/ClangTidyOptions.cpp:269 @@ +268,3 @@ + ->CheckSources[ClangTidyOptions::ConfigCommandlineOptionOrFile] + .push_back(ClangTidyOptions::StringPair(*ParsedOptions->Checks, +

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

alexfh added inline comments. Comment at: clang-tidy/tool/ClangTidyMain.cpp:281 @@ +280,3 @@ +.push_back(ClangTidyOptions::StringPair( +Checks, "command-line option '-checks'")); + } alexfh wrote: > Please pull this string literal to a constan

Re: [PATCH] D18191: [clang-tidy] Add check for function parameters that are const& to builtin types

alexfh added inline comments. Comment at: docs/clang-tidy/checks/misc-const-ref-builtin.rst:28 @@ +27,2 @@ + +The instantiation of g will not be flagged. nit: enclose `g` with double backquotes, please. Same with other inline code snippets (`int`, `double`, etc.)

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

alexfh added a comment. A couple of nits. Comment at: clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp:181 @@ +180,3 @@ + Init->isMemberInitializer() + ? static_cast(Init->getMember()) + : static_cast( If you're doing th

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

alexfh added inline comments. Comment at: test/clang-tidy/cppcoreguidelines-interfaces-global-init.cpp:7 @@ +6,3 @@ + +extern int ExternGlobal; +static int GlobalScopeBadInit1 = ExternGlobal; What happens if you add: extern int ExternGlobal; extern int Extern

Re: [PATCH] D18745: [clang-tidy] Adds modernize-use-bool-literals check.

alexfh requested changes to this revision. This revision now requires changes to proceed. Comment at: clang-tidy/modernize/UseBoolLiteralsCheck.cpp:39 @@ +38,3 @@ + + return LiteralSource.size() >= 1 && isDigit(LiteralSource.front()); +} Use `!x.empty()` instead

Re: [PATCH] D16183: Added CheckName field to YAML report

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. The main concern here is that the clang-apply-replacements tool should be able to read this format. See the clang/tools/extra/clang-tidy/tool/run-clang-tidy.py script for an example

Re: [PATCH] D18810: [Clang-tidy] Fix readability-static-definition-in-anonymous-namespace warnings; other minor fixes.

alexfh added a comment. Actually, removal of `static` is not the right thing for LLVM: > Because of this, we have a simple guideline: make anonymous namespaces as > small as possible, and only use them for class declarations. http://llvm.org/docs/CodingStandards.html#anonymous-namespaces Rep

Re: [PATCH] D18271: Avoid -Wshadow warnings about constructor parameters named after fields

alexfh added a comment. Richard, is there anything else that blocks this patch? Comment at: lib/Sema/SemaDecl.cpp:6372 @@ +6371,3 @@ + if (isa(OldDC)) { +if (isa(ShadowedDecl)) + return SDK_Field; How about `return isa(ShadowedDecl) ? SDK_Field : SDK_S

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

alexfh accepted this revision. alexfh added a comment. Looks good from my side. Please wait for the Aaron's approval. Thank you for working on this! http://reviews.llvm.org/D18584 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists

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

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good! Thank you for the new check! Do you need me to submit the patch for you? http://reviews.llvm.org/D18649 ___ cfe-commits mailing list

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

alexfh added a comment. (if you do, please rebase the patch on top of HEAD) http://reviews.llvm.org/D18649 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r265774 - [clang-tidy] cppcoreguidelines-interfaces-global-init

Author: alexfh Date: Fri Apr 8 04:51:06 2016 New Revision: 265774 URL: http://llvm.org/viewvc/llvm-project?rev=265774&view=rev Log: [clang-tidy] cppcoreguidelines-interfaces-global-init Summary: This check flags initializers of globals that access extern objects, and therefore can lead to order

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL265774: [clang-tidy] cppcoreguidelines-interfaces-global-init (authored by alexfh). Changed prior to commit: http://reviews.llvm.org/D18649?vs=53001&id=53010#toc Repository: rL LLVM http://reviews.l

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

alexfh added a comment. I've just realized that the approach is artificially limited to just keeping track of the origin of the `Checks` option, while it could be easily extended to track the origin of all configuration items. What if instead of `std::vector CheckSources;` we introduce `std::v

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

alexfh added inline comments. Comment at: clang-tidy/readability/DeletedDefaultCheck.cpp:26 @@ +25,3 @@ +void DeletedDefaultCheck::registerMatchers(MatchFinder *Finder) { + const auto NotTemplate = unless(hasAncestor( + cxxRecordDecl(::clang::ast_matchers::isTemplateInstant

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

alexfh added inline comments. Comment at: clang-tidy/readability/DeletedDefaultCheck.cpp:82 @@ +81,3 @@ + } + if (const auto* copy_assignment = + Result.Nodes.getNodeAs("copy-assignment")) { nit: CopyAssignment http://reviews.llvm.org/D18961 _

Re: [PATCH] D18300: [clang-tidy] ProTypeMemberInitCheck - check that field decls do not have in-class initializer

alexfh accepted this revision. alexfh added a comment. Missed this patch somehow: it was in a wrong part of my dashboard =\ LG. Thanks for the fix. Comment at: test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp:94 @@ +93,3 @@ + bool Bool{false}; + // CHECK-FIXES: bool

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

alexfh added a comment. In http://reviews.llvm.org/D18961#397163, @dblaikie wrote: > I'd consider just making this a compiler warning, perhaps? I agree that it might be a good idea. However, it doesn't hurt to have this in clang-tidy (at least as a prototype - to figure out details and see how

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

alexfh added a comment. Thank you for addressing the comments! Comment at: clang-tidy/readability/DeletedDefaultCheck.cpp:27 @@ +26,3 @@ + const auto NotTemplate = unless(hasAncestor( + cxxRecordDecl(::clang::ast_matchers::isTemplateInstantiation(; +

Re: [PATCH] D18991: [ASTMatchers]: fix crash in hasReturnValue

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG. This should also fix http://llvm.org/PR27298 http://reviews.llvm.org/D18991 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://l

Re: [PATCH] D18991: [ASTMatchers]: fix crash in hasReturnValue

alexfh added inline comments. Comment at: include/clang/ASTMatchers/ASTMatchers.h:5018 @@ -5017,1 +5017,3 @@ InnerMatcher) { + if (!Node.getRetValue()) +return false; nit: might be a bit clearer, if you reverse the condition: if (const auto

Re: [PATCH] D18993: [clang-tidy] fix readability-avoid-const-params-in-decls creating invalid code in fix-its

alexfh accepted this revision. alexfh added a comment. LG. Thank you! http://reviews.llvm.org/D18993 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

alexfh added inline comments. Comment at: clang-tidy/readability/DeletedDefaultCheck.cpp:28 @@ +27,3 @@ + // - actually deleted + // - not in template instantiation. + const auto isBadlyDefaulted = For decls there is `isInstantiated()`, which is defined as

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good. A couple of comments. I'm happy to commit the patch for you once you answer the comments. Thank you for the work! Comment at: clang-tidy/readability/DeletedDefa

Re: [PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

alexfh added a comment. The check is really awesome! Thank you for coming up with all the patterns that frequently happen to result from coding errors! Please update release notes. A few inline comments as well. Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:22 @@ +21,

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

alexfh added inline comments. Comment at: clang-tidy/readability/DeletedDefaultCheck.cpp:37 @@ +36,3 @@ + +void DeletedDefaultCheck::check(const MatchFinder::MatchResult &Result) { + const StringRef Message = "%0 is explicitly defaulted but implicitly " Will it

Re: [PATCH] D18766: [clang-tidy] Add check misc-multiple-statement-macro

alexfh added inline comments. Comment at: clang-tidy/misc/MultipleStatementMacroCheck.cpp:100 @@ +99,3 @@ + diag(InnerRanges.back().first, "multiple statement macro used without " + "braces; some statements will be " +

Re: [PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

alexfh added inline comments. Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:197 @@ +196,3 @@ + this); +} + What about this comment? Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:203 @@ +202,3 @@ + if (const auto *E = Result.

Re: [PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

alexfh added inline comments. Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:28 @@ +27,3 @@ + return Node.getValue().getZExtValue() > N; +} + There are no firm rules. It mostly depends on how generic/useful in other tools the matcher could be. This one se

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

alexfh added a comment. BTW, why is the check in the 'modernize' module? It doesn't seem to make anything more modern. I would guess, the pattern it detects is most likely to result from a programming error. Also, the fix, though it retains the behavior, has a high chance to be incorrect. Can y

Re: [PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

alexfh added inline comments. Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:198 @@ +197,3 @@ +} + +void SizeofExpressionCheck::check(const MatchFinder::MatchResult &Result) { I'm probably wrong about "a more effective approach in general", but for `sizeof

Re: [PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good with one more nit. I'm not sure what to do with `hasSizeOfAncestor`, we can leave it as is for now. Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:47 @@ +46

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

alexfh added a comment. Do you have svn commit access? http://reviews.llvm.org/D18584 ___ 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. I can commit the patch for you, but you need to split it in two: one for the cfe repository, one for clang-tools-extra. http://reviews.llvm.org/D18584 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm

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

alexfh added a comment. In http://reviews.llvm.org/D18821#399064, @Prazek wrote: > In http://reviews.llvm.org/D18821#398843, @alexfh wrote: > > > BTW, why is the check in the 'modernize' module? It doesn't seem to make > > anything more modern. I would guess, the pattern it detects is most likel

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

alexfh added a comment. In http://reviews.llvm.org/D18584#399056, @michael_miller wrote: > In http://reviews.llvm.org/D18584#399024, @alexfh wrote: > > > I can commit the patch for you, but you need to split it in two: one for > > the cfe repository, one for clang-tools-extra. > > > Great—thanks

[clang-tools-extra] r266181 - [clang-tidy] add_new_check.py should fail if check name starts with the module name

Author: alexfh Date: Wed Apr 13 03:46:32 2016 New Revision: 266181 URL: http://llvm.org/viewvc/llvm-project?rev=266181&view=rev Log: [clang-tidy] add_new_check.py should fail if check name starts with the module name + updated formatting Modified: clang-tools-extra/trunk/clang-tidy/add_new_

r266182 - [clang-tidy] Disable misc-unused-parameters for clang.

Author: alexfh Date: Wed Apr 13 03:47:42 2016 New Revision: 266182 URL: http://llvm.org/viewvc/llvm-project?rev=266182&view=rev Log: [clang-tidy] Disable misc-unused-parameters for clang. Modified: cfe/trunk/.clang-tidy Modified: cfe/trunk/.clang-tidy URL: http://llvm.org/viewvc/llvm-projec

r266184 - Try to use readability-identifier-naming check on Clang.

Author: alexfh Date: Wed Apr 13 03:59:49 2016 New Revision: 266184 URL: http://llvm.org/viewvc/llvm-project?rev=266184&view=rev Log: Try to use readability-identifier-naming check on Clang. Modified: cfe/trunk/.clang-tidy Modified: cfe/trunk/.clang-tidy URL: http://llvm.org/viewvc/llvm-proj

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

alexfh added a comment. Thank you for addressing the comments! I'll commit the patch for you. http://reviews.llvm.org/D18961 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

alexfh added a comment. The patch doesn't apply cleanly. Please rebase it against current HEAD. http://reviews.llvm.org/D18961 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

alexfh added a comment. Something is weird with your patch: it has names starting with a space (note two spaces between "Index:" and "clang-tidy/..."): Index: clang-tidy/readability/CMakeLists.txt === --- clang-tidy/readabil

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