[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

2018-08-10 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: docs/ReleaseNotes.rst:58 + + Warns Abseil users if they attempt to depend on internal details. I think will be good idea to omit user, and just refer to code which depend on internal details. Please synchron

[PATCH] D50580: [clang-tidy] Abseil: no namespace check

2018-08-10 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. Check documentation is missing. Comment at: clang-tidy/abseil/NoNamespaceCheck.cpp:21 +void NoNamespaceCheck::registerMatchers(MatchFinder *Finder) { + if (!getLangOpts().CPlusPlus) return; + Please place return in separate line

[PATCH] D48909: [clang-doc] Update BitcodeReader to use llvm::Error

2018-08-13 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. You could use //Differential revision: // in commit description to close review automatically. https://reviews.llvm.org/D48909 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mai

[PATCH] D50862: [clang-tidy] Abseil: faster strsplit delimiter check

2018-08-16 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: docs/clang-tidy/checks/abseil-faster-strsplit-delimiter.rst:6 + +This check triggers on calls to ``absl::StrSplit()`` or ``absl::MaxSplits()`` +where the delimiter is a single character string literal. The check will offer ---

[PATCH] D50389: [clang-tidy] Abseil: integral division of Duration check

2018-08-17 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. Somehow documentation file was not committed. https://reviews.llvm.org/D50389 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50580: [clang-tidy] Abseil: no namespace check

2018-08-21 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. Please rebase from trunk. Comment at: docs/ReleaseNotes.rst:63 + + Checks to ensure code does not open `namespace absl` as that + violates Abseil's compatibility guidelines. Just Ensures. Please use `` for language constructs.

[PATCH] D50580: [clang-tidy] Abseil: no namespace check

2018-08-21 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: docs/ReleaseNotes.rst:60 +- New :doc:`abseil-no-namespace + ` check. Please sort new checks list alphabetically. Comment at: docs/ReleaseNotes.rst:79 +>>> .r340314 Improvements to incl

[PATCH] D51061: [clang-tidy] abseil-str-cat-append

2018-08-22 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: docs/ReleaseNotes.rst:70 + + Flags uses of ``absl::StrCat()`` to append to a std::string. Suggests + ``absl::StrAppend()`` should be used instead. Please enclose std::string into `` Comment a

[PATCH] D43778: [clang-tidy] Adding RestrictIncludes check to Fuchsia module

2018-05-08 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: docs/ReleaseNotes.rst:116 + + Checks for allowed system includes and suggests removal of any others. If no + includes are specified, the check will exit without issuing any warnings. Is it necessary to highlight

[PATCH] D46602: [clang-tidy] Store checks profiling info as CSV files

2018-05-08 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. I think will be good idea to store data in JSON format too. Comment at: docs/ReleaseNotes.rst:60 +- clang-tidy learned to store checks profiling info as CSV files. + May be //Profile information could be stored in SSV format.//

[PATCH] D47157: Warning for framework headers using double quote includes

2018-05-21 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. See also PR22165. Repository: rC Clang https://reviews.llvm.org/D47157 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47157: Warning for framework headers using double quote includes

2018-05-21 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. >>> The warning is off by default. >> >> We typically do not add off-by-default warnings because experience has shown >> the rarely get enabled in practice. Can you explain a bit more about why >> this one is off by default? > > Right. I believe this is going to

[PATCH] D47157: Warning for framework headers using double quote includes

2018-05-23 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. In https://reviews.llvm.org/D47157#1110430, @bruno wrote: > Hi Eugene, > > > You could just not include new warning into -Wall and -Wextra. Those who > > will want to check #include statements correctness could enable it > > explicitly. > > This is exactly what's

[PATCH] D39121: [clang-tidy] Misplaced Operator in Strlen in Alloc

2017-10-20 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. Same mistake could be made with new[] operator. Comment at: docs/clang-tidy/checks/list.rst:10 android-cloexec-creat + android-cloexec-dup android-cloexec-epoll-create I think will be better to fix order of other check

[PATCH] D51132: [clang-tidy] abseil-redundant-strcat-calls-check

2018-08-23 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: docs/clang-tidy/checks/abseil-redundant-strcat-calls.rst:16 + + string s = StrCat("A", StrCat("B", StrCat("C", "D"))); + ==> string s = StrCat("A", "B", "C", "D"); Please add namespaces and use empty l

[PATCH] D51132: [clang-tidy] abseil-redundant-strcat-calls-check

2018-08-23 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: docs/clang-tidy/checks/abseil-redundant-strcat-calls.rst:16 + + string s = absl::StrCat("A", absl::StrCat("B", absll::StrCat("C", "D"))); + string s = absl::StrCat("A", "B", "C", "D"); std::string

[PATCH] D51061: [clang-tidy] abseil-str-cat-append

2018-08-23 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: docs/clang-tidy/checks/abseil-str-cat-append.rst:11 + + a = StrCat(a, b); // Use StrAppend(&a, b) instead. + Please add namespace. https://reviews.llvm.org/D51061

[PATCH] D51132: [clang-tidy] abseil-redundant-strcat-calls-check

2018-08-24 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. Example in documentation was not fixed. Comment at: docs/clang-tidy/checks/abseil-redundant-strcat-calls.rst:16 + + std::string s = absl::StrCat("A", absl::StrCat("B", absll::StrCat("C", "D"))); + std::string s = absl::StrCat("A",

[PATCH] D51132: [clang-tidy] abseil-redundant-strcat-calls-check

2018-08-24 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. Indentation of example in documentation is still fixed. https://reviews.llvm.org/D51132 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

2018-08-28 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. I still thik will be good idea to rename check (deps -> dependencies). https://reviews.llvm.org/D50542 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51575: [clang-tidy] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜

2018-09-02 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: docs/ReleaseNotes.rst:60 +- New :doc:`google-objc-function-naming + ` check. Please use alphabetical order. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51575 ___

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

2018-09-11 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:29 + +namespace { +std::string GetIsolatedDecl(const VarDecl *D, const SourceManager &SM, Please use static instead. Comment at: clang-tidy/readabilit

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

2018-09-11 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. I would suggest to use declaration or declarations in check name instead of abbreviation. Actually there are existing coding guidelines for thus rule, like SEI CERT

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

2018-07-24 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.h:42 +return false; + }; + bool isSyntheticValue(const clang::SourceManager *SourceManager, Unnecessary semicolon. Please also add empty line after. ===

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

2018-07-24 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: docs/clang-tidy/checks/readability-magic-numbers.rst:56 +By default only `0`, `1` and `-1` integer values are accepted without a warning. +This can be overridden with the 'IgnoredIntegerValues' option. In addition, +the `0.0` fl

[PATCH] D49890: Clang-Tidy Export Problem

2018-07-26 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: ClangTidy.cpp:627 +Files->makeAbsolutePath(ErrorAbsoluteFilePath); +if (SingleErrors.find(ErrorAbsoluteFilePath) == SingleErrors.end()) +{ Please run Clang-format. Repository: rCTE Clang Tools Extr

[PATCH] D49890: Clang-Tidy Export Problem

2018-07-26 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: ClangTidy.cpp:620 + llvm::StringMap SingleErrors; + for (const ClangTidyError &Error : Errors) { +if (!Error.BuildDirectory.empty()) { You could use auto, since it's iterator over container. ==

[PATCH] D49890: Clang-Tidy Export Problem

2018-07-26 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: ClangTidy.cpp:612 + + FileManager *Files = new FileManager(FileSystemOptions()); + vfs::FileSystem &FileSystem = *Files->getVirtualFileSystem(); You could use auto here, because type is in new statement. =

[PATCH] D42983: [clang-tidy] Add readability-simd-intrinsics check.

2018-02-06 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. Please mention this check in docs/ReleaseNotes.rst (in alphabetical order). Comment at: clang-tidy/readability/SIMDIntrinsicsCheck.cpp:24 + +StringRef CheckPPC(StringRef Name) { + if (Name.startswith("vec_")) Please make functio

[PATCH] D42983: [clang-tidy] Add readability-simd-intrinsics check.

2018-02-06 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tidy/readability/SIMDIntrinsicsCheck.cpp:77 +void SIMDIntrinsicsCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + callExpr(callee(functionDecl(matchesName("^::(_mm_|_mm256_|_mm512_|vec_)"))),

[PATCH] D42983: [clang-tidy] Add readability-simd-intrinsics check.

2018-02-06 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: docs/ReleaseNotes.rst:97 +- New `readability-simd-intrinsics + `_ check Please move it new checks section. Repository: rCTE

[PATCH] D43089: clang: Add ARCTargetInfo

2018-02-08 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:8220 + +} // End anonymous namespace. Please change to // namespace https://reviews.llvm.org/D43089 ___ cfe-commits mailing list

[PATCH] D43120: [clang-tidy] New checker for exceptions that are created but not thrown

2018-02-09 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: docs/clang-tidy/checks/misc-throw-keyword-missing.rst:6 + +This check warns about the potentially missing `throw` keyword. If a temporary object is created, +but the object's type derives from (or the same as) a class that has '

[PATCH] D43120: [clang-tidy] New checker for exceptions that are created but not thrown

2018-02-09 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tidy/misc/ThrowKeywordMissingCheck.cpp:24 + + auto CtorInitializerList = + cxxConstructorDecl(hasAnyConstructorInitializer(anything())); Please don't use auto where type could not be easily deduced.

[PATCH] D43392: [clang-tidy] Add Fuchsia checker for visibility attributes

2018-02-16 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: docs/ReleaseNotes.rst:60 +- New `fuchsia-add-visibility + `_ check Please move into new checks section. Comment at

[PATCH] D43500: modernize-use-default-member-init: Remove trailing comma and colon.

2018-02-20 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp:15 +#include "../utils/LexerUtils.h" + Please place it on a top, since header belong to project. Comment at: clang-tidy/modernize/UseDefaultM

[PATCH] D43667: [clang-doc] Implement a YAML generator

2018-02-23 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. Please run Clang-format and Clang-tidy modernize over new code. Comment at: clang-doc/generators/Generators.h:29 + Generator(std::unique_ptr &IS, StringRef Root, StringRef Format) + : IS(IS), Root(Root), Format(Format){}; + virtual ~Genera

[PATCH] D43667: [clang-doc] Implement a YAML generator

2018-02-23 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. I may be mistaken, but Clang source code didn't use llvm namespace by default. Insetad you should include clang/Basic/LLVM.h and use llvm:: for rest of things. Comment at: clang-doc/tool/ClangDocMain.cpp:42 namespace { Ther

[PATCH] D41102: Setup clang-doc frontend framework

2018-02-23 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. Please run Clang-format and Clang-tidy modernize. Comment at: clang-doc/Representation.h:80 + : LineNumber(LineNumber), Filename(std::move(Filename)) {} + int LineNumber; + std::string Filename; Please separate constructor

[PATCH] D43341: [clang-doc] Implement reducer portion of the frontend framework

2018-02-23 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. Please run Clang-format and Clang-tidy modernize. Comment at: clang-doc/BitcodeReader.h:36 + public: + ClangDocBitcodeReader() {} + using RecordData = SmallVector; Please use = default; https://reviews.llvm.org/D43341

[PATCH] D43424: [clang-doc] Implement a (simple) Markdown generator

2018-02-23 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. Please run Clang-format and Clang-tidy modernize. Comment at: clang-doc/generators/Generators.h:46 +public: + MDGenerator(std::unique_ptr &IS, StringRef Root, StringRef Format) : Generator(IS, Root, Format) {}; + virtual ~MDGenerator() {}; ---

[PATCH] D43778: [clang-tidy] Adding RestrictIncludes check to Fuchsia module

2018-02-26 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tidy/fuchsia/CMakeLists.txt:8 OverloadedOperatorCheck.cpp + RestrictIncludesCheck.cpp StaticallyConstructedObjectsCheck.cpp Will be good idea to be have consistent name and documentation terminology:

[PATCH] D33470: [clang-tidy] Add misc-default-numerics

2017-05-23 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: docs/clang-tidy/checks/misc-default-numerics.rst:11 +Consider scenario: +1. Have `typedef long long BigInt` in source code +2. Use `std::numeric_limits::min()` May be code-block will be better? https://reviews.l

[PATCH] D33497: clang-tidy check for __func__/__FUNCTION__ in lambdas

2017-05-24 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. Please mention this check in docs/ReleaseNotes.rst (in alphabetical order). https://reviews.llvm.org/D33497 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-co

[PATCH] D33304: [WIP][clang-tidy] Add a new module Android and a new check for file descriptors.

2017-05-24 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. It's also necessary to mention new checks group in docs/clang-tidy/index.rst. Repository: rL LLVM https://reviews.llvm.org/D33304 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bi

[PATCH] D33497: [clang-tidy] check for __func__/__FUNCTION__ in lambdas

2017-05-24 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: docs/ReleaseNotes.rst:83 + + Finds uses of __func__ or __FUNCTION__ inside lambdas. + Please highlight __func__ and __FUNCTION__ with ``. https://reviews.llvm.org/D33497

[PATCH] D33497: [clang-tidy] check for __func__/__FUNCTION__ in lambdas

2017-05-24 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: docs/ReleaseNotes.rst:83 + + Finds uses of __func__ or __FUNCTION__ inside lambdas. + Eugene.Zelenko wrote: > Please highlight __func__ and __FUNCTION__ with ``. I meant double ``, not single `. https://re

[PATCH] D33537: [clang-tidy] Exception Escape Checker

2017-05-25 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. Please mention this check in docs/ReleaseNotes.rst (in alphabetical order). Comment at: docs/clang-tidy/checks/misc-exception-escape.rst:7 +Finds functions which should not throw exceptions: ++ Destructors ++ Copy constructors I

[PATCH] D33304: [clang-tidy] Add a new module Android and a new check for file descriptors.

2017-05-30 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tidy/utils/ExprToStr.cpp:23 + SM.getCharacterData(TE) - SM.getCharacterData(B)); +} +} // namespace utils Please add empty line. Comment at: clang-tidy/utils/ExprToStr.

[PATCH] D33722: [clang-tidy] Add checker for undelegated copy of base classes

2017-05-31 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. Please mention this check in docs/ReleaseNotes.rst (in alphabetical order). Comment at: docs/clang-tidy/checks/misc-undelegated-copy-of-base-classes.rst:29 + +The checker suggests a FixItHint in every scenario including multiple +missing initial

[PATCH] D33722: [clang-tidy] Add checker for undelegated copy of base classes

2017-05-31 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: docs/ReleaseNotes.rst:83 + + Finds copy constructors where the ctor don't call the constructor of the base class. + I think will be good idea to replace ctor with constructor. Or re-phrase sentence to avoid rep

[PATCH] D33825: [clang-tidy] signal handler must be plain old function check

2017-06-02 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tidy/cert/CERTTidyModule.cpp:64 +"cert-msc54-cpp"); // C checkers // DCL checker -> check. Comment at: docs/ReleaseNotes.rst:73 + + Checks if a signal handler is not a p

[PATCH] D33841: [clang-tidy] redundant keyword check

2017-06-02 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. Please mention this check in docs/ReleaseNotes.rst (in alphabetical order). Comment at: docs/clang-tidy/checks/readability-redundant-keyword.rst:6 + +This checker removes the redundant `extern` and `inline` keywords from code. +

[PATCH] D33841: [clang-tidy] redundant keyword check

2017-06-02 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. In https://reviews.llvm.org/D33841#771944, @Prazek wrote: > Feature request: removing "void" from int main(void) This will duplicate modernize-redundant-void-arg. Repository: rL LLVM https://reviews.llvm.org/D33841

[PATCH] D34206: [clang-tidy] Add "MakeSmartPtrFunction" option to modernize-make-shared/unique checks.

2017-06-14 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. It'll be good idea to run modernize-make-unique on LLVM/Clang/etc for llvm::make_unique. https://reviews.llvm.org/D34206 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/l

[PATCH] D41326: [clang-tidy] Added diagnostics about incorrect usage of NOLINT comment

2017-12-16 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:169 + } +private: + SmallVector CheckNames; Please separate with empty line. Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:170 +private: +

[PATCH] D41363: [clang-tidy] Adding Fuchsia checker for overloaded operators

2017-12-18 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: docs/ReleaseNotes.rst:141 + + Warns if an operator is overloaded, except for the copy and move operators. + assignment operators? Comment at: docs/clang-tidy/checks/fuchsia-overloaded-operator.

[PATCH] D41546: [clang-tidy] Adding Fuchsia checker for statically constructed objects

2017-12-22 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: docs/ReleaseNotes.rst:146 + + Warns if statically-stored objects are created, unless constructed with `constexpr`. + Please highlight constexpr with ``, not `. Comment at: docs/clang-tidy/chec

[PATCH] D41648: [clang-tidy] implement cppcoreguidelines macro rules

2018-01-01 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp:13 +#include "clang/Lex/PPCallbacks.h" + +namespace clang { Please include string and STLExtras.h. Comment at: docs/clang-tidy/checks/cppcoreguid

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-01-01 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tidy/bugprone/UnusedReturnValueCheck.cpp:13 +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; Please include cassert, Regex.h, raw_ostream.h, SmallString.h. ===

[PATCH] D41740: [clang-tidy] Adding a new bugprone check for streaming objects of type int8_t or uint8_t

2018-01-04 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tidy/bugprone/StreamInt8Check.cpp:1 +//===--- StreamInt8Check.cpp - clang-tidy--===// +// Please add real description, use space after it and shorten to 80 symbols. ===

[PATCH] D41815: [clang-tidy] implement check for goto

2018-01-10 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: docs/ReleaseNotes.rst:63 + + The usage of ``goto`` has been discouraged for a long time and is diagnosed + with this check. I think will be good idea to reformulate this statement and its copy in documentation.

[PATCH] D43847: [tidy] Add check: replace string::find(...) == 0 with absl::StartsWith

2018-02-27 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. Please add new module in docs/clang-tidy/index.rst and mention it in release notes. Comment at: clang-tidy/absl/AbslTidyModule.cpp:14 +#include "StringFindStartswithCheck.h" +using namespace clang::ast_matchers; + Please separat

[PATCH] D43847: [clang-tidy] Add check: replace string::find(...) == 0 with absl::StartsWith

2018-02-27 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. std::basic_string::starts_with() was suggested for C++20. May be will be good idea to generalize code to create absl and modernize checks? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43847 ___ cfe

[PATCH] D43870: [clang-tidy] Another batch of checks to rename from misc- to bugprone-.

2018-02-28 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: docs/ReleaseNotes.rst:60 +- The 'misc-undelegated-constructor' check was renamed to `bugprone-undelegated-constructor + `_

[PATCH] D43847: [clang-tidy] Add check: replace string::find(...) == 0 with absl::StartsWith

2018-03-01 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. In https://reviews.llvm.org/D43847#1023452, @hokein wrote: > In https://reviews.llvm.org/D43847#1021967, @aaron.ballman wrote: > > > I need a bit more context because I'm unfamiliar with `absl`. What is this > > module's intended use? > > > As `absl` has been open

[PATCH] D43847: [clang-tidy] Add check: replace string::find(...) == 0 with absl::StartsWith

2018-03-01 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. By the word, may be similar check for std::string::rfind() and std::string::ends_with() (does abseil have analog) should be added too? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43847 ___ cfe-com

[PATCH] D43847: [clang-tidy] Add check: replace string::find(...) == 0 with absl::StartsWith

2018-03-02 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. In https://reviews.llvm.org/D43847#1025833, @niko wrote: > Thanks everyone for your comments! I renamed the namespace and filenames to > 'abseil'. > > @Eugene.Zelenko, definitely interested in extending this to a C++20 modernize > check and adding `absl::EndsWith

[PATCH] D41102: Setup clang-doc frontend framework

2018-03-05 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-doc/BitcodeWriter.h:160 +class ClangDocBitcodeWriter { + public: + ClangDocBitcodeWriter(llvm::BitstreamWriter &Stream, Looks like Clang-format was applied incorrectly, because this is Google, not LLVM sty

[PATCH] D44173: [clang-tidy] Add "portability" module and rename readability-simd-intrinsics to portability-simd-intrinsics

2018-03-06 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. Please mention new module in Release Notes. I think new modules should be before new checks there. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44173 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D44173: [clang-tidy] Add "portability" module and rename readability-simd-intrinsics to portability-simd-intrinsics

2018-03-06 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: docs/ReleaseNotes.rst:60 +- New module `portability` and new `portability-simd-intrinsics + `_ check Module and check from it sh

[PATCH] D44295: [clang-tidy] Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods

2018-03-09 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tidy/bugprone/ParentVirtualCallCheck.cpp:26 + +bool IsParentOf(const CXXRecordDecl *Parent, const CXXRecordDecl *ThisClass) { + assert(Parent); Please make this function static and remove anonymous namespac

[PATCH] D44295: [clang-tidy] Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods

2018-03-09 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tidy/bugprone/ParentVirtualCallCheck.cpp:12 + +#include +#include Please run Clang-format and remove empty line between headers. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44295 _

[PATCH] D44346: [clang-tidy] Add Fuchsia checker for temporary objects

2018-03-09 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tidy/fuchsia/ZxTemporaryObjectsCheck.cpp:24 + std::string QualifiedName = Node.getQualifiedNameAsString(); + return llvm::any_of(Names, + [&](StringRef Name) { return QualifiedName == Name; }); ---

[PATCH] D44295: [clang-tidy] Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods

2018-03-11 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tidy/bugprone/ParentVirtualCallCheck.cpp:47 +const CXXRecordDecl *ThisClass) { + + assert(GrandParent != nullptr); Please remove empty line. Comment at: clang-tidy

[PATCH] D44143: [clang-tidy] Create properly seeded random generator check

2018-03-13 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: docs/ReleaseNotes.rst:63 + + Detects inappropriate seeding of C++ random generators and C srand function. + Please replace srand with ``srand ()``. Comment at: docs/clang-tidy/checks/cert-msc5

[PATCH] D44346: [clang-tidy] Add Zircon module to clang-tidy

2018-03-13 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: docs/ReleaseNotes.rst:77 +- New `zircon-temporary-objects + `_ check Please sort new checks in alphabetical order. https://review

[PATCH] D44602: [clang-tidy] readability-function-size: add VariableThreshold param.

2018-03-17 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: docs/ReleaseNotes.rst:122 +- Added `VariableThreshold` to `readability-function-size + `_ check Will be good idea to add //option/

[PATCH] D43424: [clang-doc] Implement a (simple) Markdown generator

2018-06-20 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-doc/MDGenerator.cpp:33 + case AccessSpecifier::AS_none: +return ""; + } return {}; https://reviews.llvm.org/D43424 ___ cfe-commits mai

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-09 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. GCC has -Wignored-qualifiers for long time, so may be better to implement it in Clang? Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:66 + llvm::Optional Loc = findConstToRemove(Def, Result); + if (!Loc) return; + DiagnosticBuil

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-11 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: docs/clang-tidy/checks/readability-const-value-return.rst:7 +Checks for functions with a ``const``-qualified return type and recommends +removal of the ``const`` keyword. Such use of ``const`` is superfluous, and +prevents valuab

[PATCH] D53170: [clang-doc] Switch to default to all-TUs executor

2018-10-11 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp:205 + llvm::cl::OptionCategory &Category) { + auto OptionsParser = + CommonOptionsParser::create(argc, argv, Category, llvm::cl::ZeroOrMore); --

[PATCH] D52771: [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)

2018-10-12 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: docs/ReleaseNotes.rst:113 + +- New alias :doc:`cppcoreguidelines-non-private-member-variables-in-classes + ` Please move new alias after new checks. Repository: rCTE Clang Tools Extra https://reviews.llvm.o

[PATCH] D48866: [clang-tidy] Add incorrect-pointer-cast checker

2018-10-12 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tidy/misc/IncorrectPointerCastCheck.cpp:80 + } +} // namespace misc + Somehow, misc namespace is closed twice. Comment at: docs/ReleaseNotes.rst:60 +- New :doc:`misc-incorrect-pointer-c

[PATCH] D53339: Add the abseil-duration-factory-float clang-tidy check

2018-10-16 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:24 +// Returns an integer if the fractional part of a `FloatingLiteral` is 0. +llvm::Optional +TruncateIfIntegral(const FloatingLiteral &FloatLiteral) { Please use s

[PATCH] D53339: [clang-tidy] Add the abseil-duration-factory-float check

2018-10-16 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. By the word, why this check could not be generalized to any function/method which have floating-point and integer variants? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53339 ___ cfe-commits mailin

[PATCH] D53382: [clang-doc] Update documentation

2018-10-17 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. Will be good idea to mention improvements in Release Notes. Comment at: clang-tools-extra/docs/clang-doc.rst:20 Use -= + Isn't it should be same length as heading? Same in other places. https://reviews.llvm.org/D5338

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-26 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:54 + +namespace { + lebedev.ri wrote: > Extend the namespace /\ above, so that function is also covered? Anonymous namespaces are only for class/struct declarations

[PATCH] D53771: [clang-tidy] Avoid C arrays check

2018-10-26 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. May be this check belongs to modernize? Comment at: docs/clang-tidy/checks/misc-avoid-c-arrays.rst:6 + +Find C-style array delarations and recommend to use ``std::array<>``. +All types of C arrays are diagnosed. Finds.

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

2018-10-26 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: docs/ReleaseNotes.rst:142 + + Diagnoses local variable declarations declaring more than one variable and + tries to refactor the code to one statement per declaration. May be Finds or Detects like other checks?

[PATCH] D53771: [clang-tidy] Avoid C arrays check

2018-10-27 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. Will be good idea to add aliases at least for existing modules. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53771 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-b

[PATCH] D53771: [clang-tidy] Avoid C arrays check

2018-10-28 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: docs/clang-tidy/checks/cppcoreguidelines-avoid-c-arrays.rst:6 +cppcoreguidelines-avoid-c-arrays += + Please make same length as header. Co

[PATCH] D53830: [clang-tidy]: Abseil: new check 'abseil-upgrade-duration-conversions'

2018-10-29 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp:22 +ast_matchers::MatchFinder *Finder) { + // For the arithmetic calls, we match only the uses of the templated operators + // where the template parameter is not a buil

[PATCH] D56563: [clang-tidy] add options documentation to readability-identifier-naming checker

2019-01-11 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: docs/clang-tidy/checks/readability-identifier-naming.rst:34 + +When defined, the checker will ensure abstract class names conform to the +selected casing. MyDeveloperDay wrote: > Eugene.Zelenko wrote: > >

[PATCH] D54945: This commit adds a chapter about clang-tidy integrations

2019-01-11 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. I would suggest to rename //contribution// to //Contributing// (see LLVM documentation) and //integrations// to //Integrations//. Will be also good idea to fix D55523 script warnings. CHANGES SINCE LAST ACTION https://reviews

[PATCH] D56644: [clang-tidy] readability-container-size-empty handle std::string length()

2019-01-13 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. It'll be worth to mention change in Release Notes (in changes list, in alphabetical order). Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56644/new/ https://reviews.llvm.org/D56644

[PATCH] D54945: This commit adds a chapter about clang-tidy integrations

2019-01-14 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. In D54945#1355920 , @MarinaKalashina wrote: > @Eugene.Zelenko > > > I would suggest to rename contribution to Contributing (see LLVM > > documentation) and integrations to Integrations. > > Sure, done. > > > Will be also g

[PATCH] D54945: This commit adds a chapter about clang-tidy integrations

2019-01-17 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. clang-tools-sphinx-docs bot is failing because of: Warning, treated as error: /home/buildbot/llvm-build-dir/clang-tools-sphinx-docs/llvm/src/tools/clang/tools/extra/docs/clang-tidy/Contributing.rst:61: ERROR: Unknown target name: "how to setup tooling for llvm".

[PATCH] D54945: This commit adds a chapter about clang-tidy integrations

2019-01-17 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. I fixed links. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54945/new/ https://reviews.llvm.org/D54945 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi

[PATCH] D54945: This commit adds a chapter about clang-tidy integrations

2019-01-17 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. By the word, I noticed that HTTP was used and replaced it with HTTPS in Contributing.rst. Will be good idea to do the same in other documentation. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54945/new/ https://reviews.llvm.org/D5

  1   2   3   4   5   6   7   8   9   10   >