[PATCH] D86838: Improved for suggestions of code review. 1. Moved release notes to `Changes in existing checks` section. [Eugene.Zelenko] 2. Modified release notes can be describe user-facing. [Eu

2020-08-29 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. You need just upload new diff into existing Phabricator revision, not create new one. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86838/new/ https://reviews.llvm.org/D86838

[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

2020-08-28 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. In D86671#2244236 , @dougpuob wrote: > In D86671#2243788 , @Eugene.Zelenko > wrote: > >> It'll be good idea to add test case. > > Hi @Eugene.Zelenko, > I have created a

[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

2020-08-27 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. It'll be good idea to add test case. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:70 +* Added: Add IdentifierNamingCheck::CaseType, CT_HungarianNotation, supporting naming check with Hungarian notation. + Please move

[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

2020-08-27 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. Please fix Clang-tidy warnings and mention changes in Release Notes. Comment at: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:317 + if (Style.Case == IdentifierNamingCheck::CaseType::CT_HungarianNotation) { +const

[PATCH] D85697: [clang-tidy] Add cppcoreguidelines-prefer-scoped-enums-over-unscoped

2020-08-10 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. Please mention new check in Release Notes. Comment at: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-scoped-enums-over-unscoped.rst:22 + +After the fix is applied, the enum will become: + Please enclose enum

[PATCH] D80031: [clang-format] [NFC] release note placed in the wrong location and other rst linting errors

2020-08-10 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang/docs/ReleaseNotes.rst:70 motivating use case for these types is to limit 'bit' usage, these types don't - automatically promote to 'int' when operations are done between two ``ExtInt(N)`` - types, instead math occurs

[PATCH] D85523: [clang-tidy] Fix a crash in bugprone-not-null-terminated-result check when `__STDC_WANT_LIB_EXT1__` was undefined after definition.

2020-08-07 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-not-null-terminated-result-undef-stdc-want-lib-ext1.c:17 +} \ No newline at end of file Please add newline. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D84814: [clang-tidy] readability-identifier-naming checks configs for included files

2020-07-31 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:76 + + Added an option GetConfigPerFile to support including files which use + different naming styles. Please enclose GetConfigPerFile in single back-quotes.

[PATCH] D84924: [clang-tidy][WIP] Added command line option `fix-notes`

2020-07-30 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:109 SourceMgr(Diags, Files), Context(Context), ApplyFixes(ApplyFixes), -TotalFixes(0), AppliedFixes(0), WarningsAsErrors(0) { +ApplyAnyFix(ApplyAnyFix),

[PATCH] D84898: [clang-tidy] Add new checker for complex conditions with no meaning

2020-07-30 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:75 + + Finds complex conditions using `<`, `>`, `<=`, and `>=` that could be + interpreted ambiguously. Double back-ticks, not single ones. Comment at:

[PATCH] D84898: [clang-tidy] Add new checker for complex conditions with no meaning

2020-07-29 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. May be this check belongs to `bugprone` module? Comment at: clang-tools-extra/docs/ReleaseNotes.rst:75 + + Finds complex conditions using <, >, <=, and >= that have no mathematical + meaning. Please enclose comparison

[PATCH] D83717: [clang-tidy] Add check fo SEI CERT item ENV32-C

2020-07-15 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. In D83717#2153245 , @gamesh411 wrote: > @Eugene.Zelenko I have just rebase-d, and seen that the release notes page > itself was bumped to clang-tidy 12. I have added my check as a new check > there. Should I also add the

[PATCH] D83717: [clang-tidy] Add check fo SEI CERT item ENV32-C

2020-07-15 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:87 + ` check. + Finds functions registered by ``atexit`` and ``at_quick_exit`` that are calling + exit functions ``_Exit``, ``exit``, ``quick_exit`` or ``longjmp``.

[PATCH] D83717: [clang-tidy] Add check fo SEI CERT item ENV32-C

2020-07-15 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:87 + ` check. + Finds functions registered by ``atexit`` and ``at_quick_exit`` that are calling + exit functions ``_Exit``, ``exit``, ``quick_exit`` or ``longjmp``.

[PATCH] D83717: [clang-tidy] Add check fo SEI CERT item ENV32-C

2020-07-15 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/cert-env32-c.rst:6 + +This check implements SEI CERT rule ENV32-C by finding functions registered by +``atexit`` and ``at_quick_exit`` that are calling exit functions ``_Exit``,

[PATCH] D83717: [clang-tidy] Add check fo SEI CERT item ENV32-C

2020-07-14 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:87 + ` check. + Finds functions registered by ``atexit`` and ``at_quick_exit`` that are calling + exit functions ``_Exit``, ``exit``, ``quick_exit`` or ``longjmp``.

[PATCH] D82904: [clang-tidy] Warn pointer captured in async block

2020-06-30 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:100 + + Finds pointers with the `noescape` attribute that are captured by an + asynchronously-executed block. Please use double back-ticks for language constructs.

[PATCH] D82825: [clang-tidy] Added alias llvm-else-after-return.

2020-06-29 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/llvm-else-after-return.rst:15 + +Note: In this alias the options ``WarnOnUnfixable`` and +``RefactorConditionVariables`` are both set to ``false`` by default. Please use

[PATCH] D82824: [clang-tidy] Added option to readability-else-after-return

2020-06-29 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:200 + ` check now supports a + ``RefactorConditionVariables`` option to control whether to refactor condition + variables where possible. Please use single back-ticks

[PATCH] D82661: [clang-tidy][NFC} Remove unnecessary includes throughout clang-tidy header files

2020-06-26 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. I think will be good idea to run Include What You Use. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82661/new/ https://reviews.llvm.org/D82661 ___ cfe-commits mailing

[PATCH] D82223: [clang-tidy] Implement storeOptions for checks missing it.

2020-06-19 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. Looks OK for me, but Aaron is proper person to approve patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82223/new/ https://reviews.llvm.org/D82223 ___ cfe-commits

[PATCH] D75229: [clang-tidy] Add signal-in-multithreaded-program check

2020-06-18 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp:88 // C checkers // CON abelkocsis wrote: > Eugene.Zelenko wrote: > > Please use //check// here. > Should I replace the `// C++ checkers` comment

[PATCH] D82089: [clang-tidy] modernize-loop-convert reverse iteration support

2020-06-18 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. It'll be interesting to run improved check over LLVM code base. Comment at: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp:983 +const LangOptions ) +: IsEnabled(false), UseCxx20IfAvailable(UseCxx20IfAvailable), +

[PATCH] D75229: [clang-tidy] Add signal-in-multithreaded-program check

2020-06-17 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp:88 // C checkers // CON Please use //check// here. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION

[PATCH] D75229: [clang-tidy] Add signal-in-multithreaded-program check

2020-06-17 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/SignalInMultithreadedProgramCheck.h:23 +/// Finds ``signal`` function calls when the program is multithreaded. It +/// founds a program multithreaded when it finds at least one function call

[PATCH] D81923: [clang-tidy] Add modernize-use-ranges check.

2020-06-16 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:149 + Converts calls to std library algorithms that take a pair of iterators to use + the equivalent function in std::ranges. + Please highlight std::ranges with double

[PATCH] D81932: [clang-tidy] Improved accuracy of check list updater script

2020-06-16 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. Looks OK for me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81932/new/ https://reviews.llvm.org/D81932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D81272: [clang-tidy] New check `misc-redundant-condition`

2020-06-08 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/misc-redundant-condition.rst:6 + +Finds condition variables in nested `if` statements that were also checked in +the outer `if` statement and were not changed. Please use

[PATCH] D81272: [clang-tidy] New check `misc-redundant-condition`

2020-06-05 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:78 ^^ +- New :doc:`misc-redundant-condition + ` check. Please keep alphabetical order in new checks list. Comment at:

[PATCH] D80753: [clang-tidy] remove duplicate fixes of alias checkers

2020-05-28 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:750 + for (auto : Errors) { +auto Inserted = UniqueErrors.insert(); + Please don't use auto when type is not spelled in same statement or not

[PATCH] D80531: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro

2020-05-26 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize-replace-disallow-copy-and-assign-macro.rst:41 + specification untouched. You might want to run the check :doc:`modernize-use-equals-delete + ` to get warnings for deleted +

[PATCH] D80531: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro

2020-05-26 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize-replace-disallow-copy-and-assign-macro.rst:41 + specification untouched. You might want to run the check + `modernize-use-equals-delete `_ to get + warnings for deleted

[PATCH] D80531: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro

2020-05-26 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize-replace-disallow-copy-and-assign-macro.rst:31 +* Notice that the migration example above leaves the ``private`` access + specification untouched. This opens room for

[PATCH] D80531: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro

2020-05-26 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize-replace-disallow-copy-and-assign-macro.rst:23 + private: + - DISALLOW_COPY_AND_ASSIGN(Foo); + + Foo(const Foo &) = delete; I think two code snippets would

[PATCH] D80531: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro

2020-05-25 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.h:46 + bool isLanguageVersionSupported(const LangOptions ) const override { +return LangOpts.CPlusPlus; + } I think check

[PATCH] D80514: [clang-tidy] modernize-use-trailing-return-type support for C++20 concepts and decltype

2020-05-25 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:399 - return true; + return; } Non needed. See readability-redundant-control-flow. Comment at:

[PATCH] D66564: [clang-tidy] new altera struct pack align check

2020-05-21 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. In D66564#2049664 , @ffrankies wrote: > @Eugene.Zelenko Just checking in, is there anything I missed regarding what > we need to do for these checks? It'll be good idea to ping reviewers. Repository: rG LLVM Github

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

2020-05-20 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/abseil-string-find-str-contains.rst:4 +abseil-string-find-str-contains += + Please make same length as title. Repository: rG LLVM Github

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

2020-05-19 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:82 + + Finds s.find(...) == string::npos comparisons (for various string-like types) + and suggests replacing with absl::StrContains. Please use double back-ticks to

[PATCH] D80031: [clang-format] [NFC] release note placed in the wrong location and other rst linting errors

2020-05-15 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang/docs/ReleaseNotes.rst:114 + this occurs when the use of the ``extern`` keyword is neglected in the + declaration of a variable in a header file. In some cases, no specific translation unit This and

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

2020-05-15 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp:18 +#include "clang/Tooling/Transformer/Stencil.h" +#include + asserts are not used. Comment at:

[PATCH] D78985: [clang-tidy] NFC: Cleanup Python scripts

2020-04-28 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. In D78985#2007715 , @kbobyrev wrote: > In D78985#2007669 , @Eugene.Zelenko > wrote: > > > It would be great to also run Flake9 and PyLint. > > > Those were actually from pylint.

[PATCH] D78985: [clang-tidy] NFC: Cleanup Python scripts

2020-04-28 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. It would be great to also run Flake9 and PyLint. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78985/new/ https://reviews.llvm.org/D78985 ___ cfe-commits mailing list

[PATCH] D78890: [clang-tidy] Add check callee-namespace.

2020-04-26 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:119 + + Checks all calls resolve to functions within __llvm_libc namespace. + Please enclose __llvm_libc in double back-ticks. Comment at:

[PATCH] D46317: [clang-tidy] New check bugprone-map-subscript-operator-lookup

2020-04-24 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/MapSubscriptOperatorLookupCheck.cpp:31 +void MapSubscriptOperatorLookupCheck::registerMatchers(MatchFinder *Finder) { + if (!getLangOpts().CPlusPlus) +return; Should be

[PATCH] D77983: clang-tidy doc: add a note for every checker with an autofix

2020-04-12 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. Herald added a subscriber: wuzish. It'll be reasonable to use two spaces indent. At least this is what mostly used for code blocks. Options are exceptions, but will be good idea to reformat them eventually. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D77572: [clang-tidy] add new check readability-use-anyofallof

2020-04-06 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. See also suggestion for more generic loop-to-algorithm transformations. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77572/new/ https://reviews.llvm.org/D77572

[PATCH] D77572: [clang-tidy] add new check readability-use-anyofallof

2020-04-06 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp:26 +AST_MATCHER_P(Stmt, nextStmt, internal::Matcher, InnerMatcher) { + const auto = Finder->getASTContext().getParents(Node); + if (Parents.size() != 1)

[PATCH] D77571: Add ClangTidy check to find calls to NSInvocation methods under ARC that don't have proper object argument lifetimes.

2020-04-06 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/objc/NsinvocationArgumentLifetimeCheck.cpp:35 + +constexpr StringRef WeakText = "__weak"; +constexpr StringRef StrongText = "__strong"; Please use static instead of anonymous

[PATCH] D77482: [clang-tools-extra] NFC: Fix trivial typo in documents and comments

2020-04-05 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. In D77482#1962272 , @kiszk wrote: > I have done in https://reviews.llvm.org/D77458 Just abandon this revision. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77482/new/

[PATCH] D77482: [clang-tools-extra] NFC: Fix trivial typo in documents and comments

2020-04-04 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. In D77482#1962000 , @kiszk wrote: > @sammccall, I am very sorry that I led to the build break due to my > misoperation in https://reviews.llvm.org/D77458 You could just update diff in existing revision. No need to create

[PATCH] D76818: [clang-tidy] Add check llvmlibc-implementation-in-namespace.

2020-03-25 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. Please also add isLanguageVersionSupported(), because namespaces make sense only in C++. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76818/new/ https://reviews.llvm.org/D76818

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

2020-03-16 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.cpp:43 +} + + Unnecessary empty line. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76229/new/

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

2020-03-16 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.cpp:19 + +namespace { +const ValueDecl *getDescendantValueDecl(const Stmt *TheStmt) { Please use static instead of anonymous namespaces for functions.

[PATCH] D75332: [clang-tidy] Add module for llvm-libc and restrict-system-libc-header-check.

2020-03-10 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:100 + + Finds includes of sytem libc headers in llvm-libc implementation. + Please synchronize with first statement in documentation. Repository: rG LLVM Github

[PATCH] D74669: [clang-tidy] New check: bugprone-suspicious-include

2020-03-09 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:101 +- New :doc:`bugprone-suspicious-includei + ` check. Please keep alphabetical order. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:104 + +

[PATCH] D75786: [clang-tidy] Move fuchsia-restrict-system-includes to portability module for general use.

2020-03-06 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:130 ^^ +- The 'fuchsia-restrict-system-headers' check was renamed to :doc:`portability-restrict-system-includes + Please insert empty line above.

[PATCH] D75786: [clang-tidy] Move fuchsia-restrict-system-includes to portability module for general use.

2020-03-06 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:107 + + Moved fuchsia's restrict-system-includes check to portability to allow for + general use. I don't think that this statement is necessary.

[PATCH] D72235: [clang-tidy] new altera unroll loops check

2020-03-04 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/altera/UnrollLoopsCheck.cpp:41 + } + if (Unroll == PartiallyUnrolled) { +return; Please elide braces. Comment at:

[PATCH] D75569: [clang-tidy] New check for methods marked __attribute__((unavailable)) that do not override a method from a superclass.

2020-03-03 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:104 + + Finds methods marked unavailable that do not override any method from a + superclass. Please synchronize with first statement in documentation.

[PATCH] D75332: [clang-tidy] Add module for llvm-libc and restrict-system-libc-header-check.

2020-03-03 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/llvmlibc-restrict-system-libc-headers.rst:18 +lead to subtle and hard to detect bugs. For example consider a system libc +whose `dirent` struct has slightly different field ordering than

[PATCH] D75492: [clang-tidy]: modernize-use-using: don't diagnose typedefs in `extern "C"` DeclContexts

2020-03-02 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. See also https://llvm.org/PR35924. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75492/new/ https://reviews.llvm.org/D75492 ___ cfe-commits mailing list

[PATCH] D75332: [clang-tidy] Add module for llvm-libc and restrict-system-libc-header-check.

2020-02-28 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. Please mention new module in docs/clang-tidy/index.rst and Release Notes (new modules section is above new checks one and please add subsubsection). Comment at: clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp:21 + void

[PATCH] D75289: [clang-tidy] Added virtual isLanguageVersionSupported to ClangTidyCheck

2020-02-27 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. In D75289#1896925 , @njames93 wrote: > In D75289#1896902 , @Eugene.Zelenko > wrote: > > > Language and/or its standard is checked in other places too. Should all > > similar places

[PATCH] D75289: [clang-tidy] Added virtual isLanguageVersionSupported to ClangTidyCheck

2020-02-27 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. Language and/or its standard is checked in other places too. Should all similar places be refactored? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75289/new/ https://reviews.llvm.org/D75289

[PATCH] D72235: [clang-tidy] new altera unroll loops check

2020-02-27 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. Please rebase from master. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72235/new/ https://reviews.llvm.org/D72235 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D66564: [clang-tidy] new altera struct pack align check

2020-02-27 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. In D66564#1895916 , @aaron.ballman wrote: > Are you aware of plans that you or others have for adding additional checks > to the new `altera` module? There are several patches for `altera` module already. CHANGES

[PATCH] D75229: [clang-tidy] Add signal-in-multithreaded-program check

2020-02-27 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. Will be good idea to make callers function list configurable through option. See other checks as example. Comment at: clang-tools-extra/clang-tidy/bugprone/SignalInMultithreadedProgramCheck.cpp:20 +void

[PATCH] D75134: [clang-tidy] Improved renamer check logic

2020-02-25 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. It'll be reasonable to find out what are naming conventions for Python in LLVM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75134/new/ https://reviews.llvm.org/D75134

[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

2020-02-25 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. In D74669#1892577 , @jroelofs wrote: > Implement review feedback: > > - Re-purpose `HeaderFileExtensionsUtils.h` to support headers *and* sources. > - Surround file extension in diagnostic with `''`s. > > I have these as

[PATCH] D75134: [clang-tidy] Improved renamer check logic

2020-02-25 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. It'll be reasonable to have script to create check alias, but this could be made in separate patch. Comment at: clang-tools-extra/clang-tidy/add_new_check.py:17 import sys - -# Adapts the module's CMakelist file. Returns 'True' if it could add

[PATCH] D73580: [clang-tidy] rename_check.py: maintain alphabetical order in Renamed checks section

2020-02-19 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. In D73580#1883861 , @njames93 wrote: > LGTM, For what this is trying to achieve I have no issue, follow up patches > can be submitted for the other enhancements needed. Please commit patch if you'll have time. See

[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

2020-02-15 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. Herald added a subscriber: wuzish. How about .cc and .cxx extension? It'll be reasonable to provide option to configure list of extensions. Please add documentation and mention new check in Release Notes. Comment at:

[PATCH] D73580: [clang-tidy] rename_check.py: maintain alphabetical order in Renamed checks section

2020-02-02 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. Another side effect is that rename_check.py messes list.rst by removing "Offer fixes". Probably proper solution would be moving update_checks_list from add_new_check.py to shared module. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D73580: [clang-tidy] rename_check.py: maintain alphabetical order in Renamed checks section

2020-01-31 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. In D73580#1852032 , @njames93 wrote: > It looks good, but could you maybe create a child revision showing what it > looks like with a few checks renamed to make sure everything is all working > correctly. I think it'll

[PATCH] D73775: [clang-tidy] Cover cases like (b && c && b) in the redundant expression check

2020-01-31 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp:335 +return AsTExpr; + else +return nullptr; Please don;e use else after return. Comment at:

[PATCH] D73762: [clang] New warning for for-loops where the iteration does not match the loop condition

2020-01-31 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. Check should also consider cases like: i += 2; i = i + 2; i -= 2; i = i - 2. Probably same with multiplications, divisions and shifts. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73762/new/

[PATCH] D73762: [clang] New warning for for-loops where the iteration does not match the loop condition

2020-01-30 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. It's good question where this check belongs. There are two loop related checks in CLang-tidy: bugprone-infinite-loop and bugprone-too-small-loop-variable. Comment at: clang/lib/Sema/SemaStmt.cpp:1754 +if (!Cond->isRelationalOp()) return; +

[PATCH] D73580: [clang-tidy] rename_check.py: maintain alphabetical order in Renamed checks section

2020-01-28 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko created this revision. Eugene.Zelenko added reviewers: alexfh, hokein, aaron.ballman, njames93, MyDeveloperDay. Eugene.Zelenko added a project: clang-tools-extra. Herald added a subscriber: xazax.hun. Herald added a project: clang. Also use //check// in add_new_check.py for

[PATCH] D72527: [clang-tidy] adjust scripts to subsubsections in Release Notes

2020-01-28 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko updated this revision to Diff 240922. Eugene.Zelenko added a comment. Rebase from master. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72527/new/ https://reviews.llvm.org/D72527 Files:

[PATCH] D72527: [clang-tidy] adjust scripts to subsubsections in Release Notes

2020-01-27 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. As I wrote, I don't have GitHub commit access, so I need help with commit. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72527/new/ https://reviews.llvm.org/D72527 ___

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-27 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. In D73413#1843368 , @njames93 wrote: > In D73413#1843103 , @alexfh wrote: > > > How is this different from `-Wmissing-prototypes`? > > > This checks variables too, and it looks for a

[PATCH] D72527: [clang-tidy] adjust scripts to subsubsections in Release Notes

2020-01-27 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. Just ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72527/new/ https://reviews.llvm.org/D72527 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-25 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp:14 +#include "llvm/ADT/Optional.h" +#include "llvm/ADT/StringSwitch.h" +#include Please also include StringRef. Comment

[PATCH] D73300: [clang-tidy] Add library for clang-tidy main function

2020-01-23 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/tool/ClangTidyMain.h:5 +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. License seems to be old.

[PATCH] D73270: [clang-tidy] Fix false positive in bugprone-infinite-loop

2020-01-23 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp:179 + if (const auto *While = dyn_cast(LoopStmt)) { +if (const auto *LoopVarDecl = While->getConditionVariable()) { + if (const Expr *Init =

[PATCH] D73203: [clang-tidy] Prevent a remove only fixit causing a conflict when enclosed by another fixit

2020-01-22 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:726 + unsigned ErrorIndex = ErrorAndDiscarded.first; + const auto = ErrorAndDiscarded.second; + if (!Apply[ErrorIndex]) Please don't

[PATCH] D72488: [clang-tidy] Add check for CERT-OOP57-CPP

2020-01-20 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/cert-oop57-cpp.rst:25 + Default is an empty string. + The check will detect the following functions: + `std::memcpy`, `memcpy`, `std::memmove`, `memmove`, `std::strcpy`,

[PATCH] D72848: Remove some SVN-specific code.

2020-01-17 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang/lib/Basic/Version.cpp:33 #else - StringRef URL(""); + return ""; #endif hans wrote: > Eugene.Zelenko wrote: > > return {} should be better. > Why? I think "" is clearer for a string. It's default

[PATCH] D72848: Remove some SVN-specific code.

2020-01-16 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang/lib/Basic/Version.cpp:33 #else - StringRef URL(""); + return ""; #endif return {} should be better. Comment at: clang/lib/Basic/Version.cpp:42 #else - StringRef URL(""); + return

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-initialization-list

2020-01-16 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-member-initializer.rst:72 + +C(int nn, int mm) : n(nn) { + if (dice()) Please make indentation 2 characters, same as in above code

[PATCH] D71001: [clang-tidy] New check: bugprone-misplaced-pointer-arithmetic-in-alloc

2020-01-16 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-misplaced-pointer-arithmetic-in-alloc.rst:15 + +void bad_malloc(int n) { + char *p = (char*) malloc(n) + 10; Identification. I think 2 character should

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-initialization-list

2020-01-15 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. By the word, please rebase, because Release Notes were reset after version 10 branching. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71199/new/ https://reviews.llvm.org/D71199 ___ cfe-commits mailing list

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-initialization-list

2020-01-15 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp:52 +static bool isUnaryExprOfLiteral(const Expr *E) { + if (const auto *UnOp = dyn_cast(E)) { +return isLiteral(UnOp->getSubExpr());

[PATCH] D72488: [clang-tidy] Add check for CERT-OOP57-CPP

2020-01-14 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:112 + + Flags use of the `C` standard library functions 'memset', 'memcpy' and + 'memcmp' and similar derivatives on non-trivial types. Please use double back-ticks to

[PATCH] D72121: [clang-tidy] Fix readability-identifier-naming missing member variables

2020-01-13 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. Please also close PRs. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72121/new/ https://reviews.llvm.org/D72121 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D72527: [clang-tidy] adjust scripts to subsubsections in Release Notes

2020-01-13 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko marked an inline comment as done. Eugene.Zelenko added a comment. Both scripts works fine. However rename script should also sort entries alphabetically, but probably this should be separate patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D72527: [clang-tidy] adjust scripts to subsubsections in Release Notes

2020-01-13 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko updated this revision to Diff 237720. Eugene.Zelenko added a comment. Renamed "New aliases" to "New check aliases". Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72527/new/ https://reviews.llvm.org/D72527 Files:

[PATCH] D72484: [clang-tidy] Fix check for Abseil internal namespace access

2020-01-13 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. In D72484#1817187 , @rogeeff wrote: > It is my first time submitting clang-tidy change. So I'm not sure of the > procedure exactly. Do we wait for something from me? Reviewers should make comments and finally approve

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-12 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. In D54943#1815912 , @0x8000- wrote: > As an aside, once this is merged in, I dream of a "fix-it" for old style C > code: > > int foo; > > ... > /* page of code which does not use either foo or bar */ > ... >

<    1   2   3   4   5   6   7   8   9   10   >