[PATCH] D118520: [clang-tidy] Output currently processing check and nodes on crash

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:191 + } + void setContext(const ASTContext &Ctx) { CurContext = &Ctx; } + void clearContext() { CurContext = nullptr; } Please separate with newline.

[PATCH] D118520: [clang-tidy] Output currently processing check and nodes on crash

Eugene.Zelenko added a comment. Sure, this is important improvement. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118520/new/ https://reviews.llvm.org/D118520 ___ cfe-commits mailing list cfe-commits@li

[PATCH] D118743: [clang-tidy] Add `modernize-use-inline-const-variables-in-headers` check

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/UseInlineConstVariablesInHeadersCheck.h:33 +/// declarations in header files. True by default. +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/ch

[PATCH] D120385: [clang-tidy][run-clang-tidy.py] Mutual exclusion of -config and -config-file

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py:239 help='Specify the path of .clang-tidy or custom config' - 'file: e.g. -config-file=/some/path/myTidyConfigFile' +

[PATCH] D120385: [clang-tidy][run-clang-tidy.py] Mutual exclusion of -config and -config-file

Eugene.Zelenko added a comment. I think will be good to merge these changes to D120331 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120385/new/ https://reviews.llvm.org/D120385

[PATCH] D120385: [clang-tidy][run-clang-tidy.py] Mutual exclusion of -config and -config-file

Eugene.Zelenko added a comment. About procedure :-) You could not approve patches yourself, this is task for reviewers. If content of this patch will be merged to another one, just abandon this one. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D12

[PATCH] D121019: [clang-tools-extra] Document clang tidy unit tests target

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:367 +You can build the Clang Tidy unit tests by building the ``ClangTidyTests`` target. +Test targets in LLVM and Clang are excluded from the "build all" style action of -

[PATCH] D101471: [clang-tidy] Add proper emplace checks to modernize-use-emplace

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp:25 + Names) { + const auto FullName = "::" + Node.getQualifiedNameAsString(); + Please don't use auto unless type is spelled explicitly in s

[PATCH] D101471: [clang-tidy] Add proper emplace checks to modernize-use-emplace

Eugene.Zelenko added a comment. Please mark fixed suggestions as done. Comment at: clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp:248 + const auto *Call = PushBackCall ? PushBackCall : EmplacyCall; + Please don't use auto unless type is spelled e

[PATCH] D101471: [clang-tidy] Add proper emplace checks to modernize-use-emplace

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp:29 + int Count = 0; + for (const auto &Character : FullName) { +if (Character == '<') { I'm not sure, but probably braces could be elided in `for` a

[PATCH] D100092: [clang-tidy] cppcoreguidelines-declare-loop-variable-in-the-initializer: a new check

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/DeclareLoopVariableInTheInitializerCheck.cpp:37 + bool VisitDeclRefExpr(DeclRefExpr *D) { +if (const VarDecl *To = dyn_cast(D->getDecl())) { + if (To == MatchedDecl &&

[PATCH] D101787: [clang-tidy] Aliasing: Add more support for lambda captures.

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/Aliasing.cpp:76 +const VarDecl *Var) { + const CXXMethodDecl *MD = dyn_cast(Func); + if (!MD) `const auto*` could be used becaus

[PATCH] D116478: [clang-tidy] A comma-separated list of the names of functions or methods to be considered as not having side-effects

Eugene.Zelenko added a comment. Please mention new option in Release Notes (in changes in existing checks section). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116478/new/ https://reviews.llvm.org/D116478 ___ cfe-commits mailing list cfe-c

[PATCH] D116478: [clang-tidy] A comma-separated list of the names of functions or methods to be considered as not having side-effects

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:141 ^^ +- Added a setting ``bugprone-assert-side-effect.FunctionExceptions`` for + a comma-separated list of the names of functions or methods to be considered

[PATCH] D7982: Add readability-duplicate-include check to clang-tidy

Eugene.Zelenko added a comment. Functionality-wise this check is superseded by Include What You Use . Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-duplicate-include.rst:6 + +Looks for duplicate

[PATCH] D116478: [clang-tidy] A comma-separated list of the names of functions or methods to be considered as not having side-effects

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:141 ^^ +- Added a setting ``bugprone-assert-side-effect.FunctionExceptions`` for + a comma-separated list of the names of functions or methods to be considered

[PATCH] D116577: [clang-tidy] Added "boost-use-range-based-for-loop" check

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/boost/UseRangeBasedForLoopCheck.cpp:60 + +auto Loc = MacroNameToken.getLocation(); +auto Diag = Check->diag(Loc, "use range-based for loop instead of %0") Please specify type e

[PATCH] D116875: [clang-tidy] Add performance-inefficient-array-traversal check

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/performance/InefficientArrayTraversalCheck.cpp:59 + // TODO: Add support for x [+-]= 1 and x = x [+-] 1 + hasIncrement(unaryOperator(hasUnaryOperand(ToDecl), +

[PATCH] D116875: [clang-tidy] Add performance-inefficient-array-traversal check

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/performance/InefficientArrayTraversalCheck.cpp:63 +hasLHS(ToDecl), +hasRHS(integerLiteral(equals(1)), +

[PATCH] D117205: [clang-tidy] Support custom fix hint for cppcoreguidelines-pro-bounds-constant-array-index

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:156 +- Add new option ``cppcoreguidelines-pro-bounds-constant-array-index.FixHint`` + to allow users to specify a different fix hint than ``gsl::at``. Please sort checks

[PATCH] D117205: [clang-tidy] Support custom fix hint for cppcoreguidelines-pro-bounds-constant-array-index

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:156 +- Add new option ``cppcoreguidelines-pro-bounds-constant-array-index.FixHint`` + to allow users to specify a different fix hint than ``gsl::at``. carlosgalvezp wrote

[PATCH] D117306: [clang-tidy] Add new check 'shared-ptr-array-mismatch'.

Eugene.Zelenko added a comment. I think this check belongs to `bugprone` module. How about `std::unique_ptr`? Comment at: clang-tools-extra/clang-tidy/misc/SharedPtrArrayMismatchCheck.cpp:1 +//===--- SharedPtrArrayMismatchCheck.cpp - clang-tidy +//--===// --

[PATCH] D98635: [libtooling][clang-tidy] Fix diagnostics not respecting and highlighting fed SourceRanges

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:78 +- libToolingCore and Clang-Tidy was refactored and now checks can produce + highlights (``^`` under fragments of the source code) in diagnostics. + Existing and new checks in the f

[PATCH] D99646: [clang-tidy] misc-std-stream-objects-outside-main: a new check

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/StdStreamObjectsOutsideMainCheck.cpp:61 +} // namespace clang \ No newline at end of file Please add newline. Comment at: clang-tools-extra/clang-tidy/misc/S

[PATCH] D99646: [clang-tidy] misc-std-stream-objects-outside-main: a new check

Eugene.Zelenko added a comment. It looks like new patch was not uploaded. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99646/new/ https://reviews.llvm.org/D99646 ___ cfe-commits mailing list cfe-commits

[PATCH] D99646: [clang-tidy] misc-std-stream-objects-outside-main: a new check

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/StdStreamObjectsOutsideMainCheck.h:30 +return LangOpts.CPlusPlus; + }; + Semicolon is not needed and should cause `-Wextra-semi` warnings. Repository: rG LLVM Github Mon

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/altera-id-dependent-backward-branch.rst:9 + +Based on the `Altera SDK for OpenCL: Best Practices Guide +

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp:200 + if (isa(Loop)) +return DO_LOOP; // loop_type = 0; + else if (isa(Loop)) Is loop ID is not enough? Why does comment with numerical

[PATCH] D99993: [clang-tidy] bugprone-argument-comment: ignore name mismatches for decls from system headers

Eugene.Zelenko added a comment. Why `--header-filter` command line option or `HeaderFilterRegex` configuration file option could not solve this problem? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D3/new/ https://reviews.llvm.org/D3

[PATCH] D100092: [clang-tidy] cppcoreguidelines-declare-loop-variable-in-the-initializer: a new check

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/DeclareLoopVariableInTheInitializerCheck.cpp:28 + + diag(MatchedForStmt->getBeginLoc(), + "Prefer to declare a loop variable in the initializer part of a " It

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp:1 +//===--- IdDependentBackwardBranchCheck.cpp - clang-tidy *- C++ -*-===// +// `*- C++ -*` is not necessary for `.cpp` files. CHA

[PATCH] D93861: [clang-tidy][NFC] Split up some headers

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyContext.h:9 -#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGTIDYDIAGNOSTICCONSUMER_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGTIDYDIAGNOSTICCONSUMER_H Should header g

[PATCH] D93940: [clang-tidy] Add a check for blocking types and functions.

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/concurrency/AsyncBlockingCheck.cpp:1 +//===--- AsyncBlockingCheck.cpp - clang-tidy ===// +// Please make length same as end of comment. C

[PATCH] D93940: [clang-tidy] Add a check for blocking types and functions.

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/concurrency-async-blocking.rst:7 +Checks for some synchronous functions and types that volunteerly preempt system +thread. Volunteer preemption of a system thread in asynchronous code (e.

[PATCH] D93940: [clang-tidy] Add a check for blocking types and functions.

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/concurrency/AsyncBlockingCheck.cpp:319 +assert(L); +const auto *EV = L->ensureEvaluatedStmt(); +if (EV && EV->Evaluated.getKind() == APValue::ValueKind::None) { Please don'

[PATCH] D94621: [clang-tidy] add concurrency-async-fs

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/concurrency-async-fs.rst:6 + +Search for filesystem accesses that might block current system thread. +Asynchronous code may deal with it in numerous ways, the most widespread --

[PATCH] D93940: [clang-tidy] Add a check for blocking types and functions.

Eugene.Zelenko added a comment. In D93940#2509325 , @segoon wrote: > Eugene.Zelenko, njames93, any comments on the patch? Sorry, you need to wait for approval from one of maintainer mentioned as reviewers. I check only documentation and for minor code is

[PATCH] D94621: [clang-tidy] add concurrency-async-fs

Eugene.Zelenko added a comment. In D94621#2509324 , @segoon wrote: > Eugene.Zelenko, njames93, any comments on the patch? Sorry, you need to wait for approval from one of maintainers mentioned as reviewers. I check only documentation and for minor code i

[PATCH] D94622: [clang-tidy] add concurrency-async-no-new-threads

Eugene.Zelenko added a comment. In D94622#2509323 , @segoon wrote: > Eugene.Zelenko, njames93, any comments on the patch? Sorry, you need to wait for approval from one of maintainers mentioned as reviewers. I check only documentation and for minor code i

[PATCH] D111208: [clang-tidy] Support globbing in NOLINT* expressions

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:72 + multiple warnings in the same line. - Added support for `NOLINTBEGIN` ... `NOLINTEND` comments to suppress Clang-Tidy warnings over multiple lines. Please separate

[PATCH] D111623: [clang-tidy] Update documentation of check bugprone-unused-return-value [NFC].

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-unused-return-value.rst:16 + By default the following functions are checked: + ``std::async, std::launder, std::remove, std::remove_if, std::unique, + std::unique_ptr::relea

[PATCH] D112409: [clang-tidy] Add check 'cert-err33-c'.

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/cert-err33-c.rst:7 +Warns on unused function return values. +This check corresponds to (a part of) CERT C Coding Standard rule `ERR33-C. +Detect and handle standard library errors -

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

Eugene.Zelenko added a comment. Thank you for implementing 26817 ! But shouldn't this check belong to `modernize` module? Please add documentation and mention new check in Release Notes. Comment at: clang-tools-extra/clang-tidy/re

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp:18 +namespace tidy { +namespace readability { +ContainerDataPointerCheck::ContainerDataPointerCheck(StringRef Name, compnerd wrote: > Eugene.

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:87 + + Finds cases where code could use ``data`` rather than the address of an element. + It'll be good idea to mention containers and that `data` is method. ===

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

Eugene.Zelenko added a comment. In D108893#2972634 , @compnerd wrote: > @Eugene.Zelenko I'd like to have @aaron.ballman weigh in on the naming, and > assuming that he's okay with `modernize.container-data-pointer`, I'll change > it to that. Sure, Aaron

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

Eugene.Zelenko added a comment. What about adding `modernize`/`bugprone` aliases? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108893/new/ https://reviews.llvm.org/D108893 ___ cfe-commits mailing list c

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

Eugene.Zelenko added a comment. In D108893#2999654 , @aaron.ballman wrote: > In D108893#2999645 , > @Eugene.Zelenko wrote: > >> What about adding `modernize`/`bugprone` aliases? > > I'd be fine if we wanted to a

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

Eugene.Zelenko added a comment. > So you'd like to see that extra functionality added now? (I don't think it > makes sense to have the check as-is in both `readability` and `modernize`.) There are plenty of other cross-module aliases al

<    5   6   7   8   9   10