[PATCH] D69746: [analyzer] FixItHint: Apply and test hints with the Clang-Tidy's script

2020-02-24 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. LG CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69746/new/ https://reviews.llvm.org/D69746 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailma

[PATCH] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

2020-02-27 Thread Alexander Kornienko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGb26c88e3c6e0: [clang-tidy] Store all ranges in clang::tooling::Diagnostic (authored by compositeprimes, committed by alexfh). Changed prior to commit: https://reviews.llvm.org/D69782?vs=245717&id=247029

[PATCH] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

2020-02-27 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In D69782#1886766 , @compositeprimes wrote: > I don't have commit rights (this is my first llvm commit!), so could someone > help out? Thanks! Fixed include paths and a broken test and committed as b26c88e3c6e08f8f78ab4291bc85b

[PATCH] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

2020-02-27 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. Next step is to plumb this to clang-tidy. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69782/new/ https://reviews.llvm.org/D69782 ___ cfe-commits mailing list cfe-commits@lists

[PATCH] D68887: [clang-tidy] Copy the Ranges field from the Diagnostic when creating the ClangTidyError

2020-02-27 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68887/new/ https://reviews.llvm.org/D68887 ___ cfe-commits mailing list cfe-commits@lis

[PATCH] D68887: [clang-tidy] Copy the Ranges field from the Diagnostic when creating the ClangTidyError

2020-03-02 Thread Alexander Kornienko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG071002ffdb3f: [clang-tidy] Copy the Ranges field from the Diagnostic when creating the… (authored by compositeprimes, committed by alexfh). Changed prior to commit: https://reviews.llvm.org/D68887?vs=2

[PATCH] D75441: [clang-tidy] Add helper base check classes that only run on specific language versions

2020-03-03 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In D75441#1901071 , @gribozavr2 wrote: > > I think my preference is to go with isLanguageVersionSupported() and not > > use base classes. > > +1, I can see this working, but the introduction of new concepts does not > seem like it

[PATCH] D75538: [clang-tidy] Updated language supported restrictions on some checks

2020-03-03 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: clang-tools-extra/test/clang-tidy/checkers/modernize-use-nullptr-basic.cpp:1 -// RUN: %check_clang_tidy -std=c++98 %s modernize-use-nullptr %t -- -

[PATCH] D75184: [clang-tidy] Optional inheritance of file configs from parent directories 

2020-03-04 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. Internally we have something similar, but with unconditional inheritance and a way to include other configs. I was thinking about implementing this in the FileOptionsProvider, but de

[PATCH] D75538: [clang-tidy] Updated language supported restrictions on some checks

2020-03-04 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/modernize-use-nullptr-basic.cpp:1 -// RUN: %check_clang_tidy -std=c++98 %s modernize-use-nullptr %t -- -- -Wno-non-literal-null-conversion -// gribozavr2 wrote: > njames93 wrot

[PATCH] D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal

2019-11-14 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. While I have no objections against this patch, I wonder whether someone had a chance to ask GCC developers about this? Is it a conscious choice to suggest `override` when `final` is present? What's the argument for doing so? Repository: rCTE Clang Tools Extra CHANGES

[PATCH] D46027: [clang-tidy] Fix PR35824

2019-11-27 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. > This approach will also introduce false negatives. Could you add a test showing this with a FIXME comment? > A better approach would be to check if the null statement is the result of > folding an if constexpr. > The current AST API does

[PATCH] D69746: [analyzer] FixItHint: Apply and test hints with the Clang Tidy's script

2019-11-27 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In D69746#1756448 , @gribozavr2 wrote: > The only way I know people apply fixits is with the help of IDEs. This depends on the infrastructure available. Talking specifically about clang-tidy in our environment, I know of at least

[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

2019-11-28 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: clang/include/clang/Basic/CharInfo.h:94 +LLVM_READONLY inline bool isWhitespace(llvm::StringRef S) { + for (StringRef::const_iterator I = S.begin(), E = S.end(); I != E; ++I) { +if (!isWhitespace(*I)) I'd suggest to

[PATCH] D69855: [clang-tidy] Fix llvm-namespace-comment for macro expansions

2019-12-03 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. There's a problem with this patch. Consider the following code: // In some remote header: #define SOME_RANDOM_MACRO internal // In a completely unrelated file that transitively includes the header: namespace internal { void f(); } // namespace internal It m

[PATCH] D70974: [clang-tidy] Fix PR26274

2019-12-03 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh created this revision. alexfh added reviewers: gribozavr, aaron.ballman. Herald added a subscriber: xazax.hun. Herald added a project: clang. This commit fixes http://llvm.org/PR26274 in a simpler and more correct way than 4736d63f752f8d13f4c6a9afd558565c32119718

[PATCH] D70974: [clang-tidy] Fix PR26274

2019-12-04 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh updated this revision to Diff 232140. alexfh marked an inline comment as done. alexfh added a comment. - Added support for inline namespaces and namespace attributes, fixed a typo, added tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D7

[PATCH] D70974: [clang-tidy] Fix PR26274

2019-12-04 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In D70974#1769036 , @gribozavr2 wrote: > In D70974#1768902 , @aaron.ballman > wrote: > > > In D70974#1768871 , @gribozavr2 > > wrote: > > > > > I'm

[PATCH] D70974: [clang-tidy] Fix PR26274

2019-12-05 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh updated this revision to Diff 232294. alexfh marked 2 inline comments as done. alexfh added a comment. - Added a test with an empty attribute list. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70974/new/ https://reviews.llvm.org/D70974 Fil

[PATCH] D70974: [clang-tidy] Fix PR26274

2019-12-05 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp:79 + } else { // Any other kind of token is unexpected here. +return llvm::None; + } aaron.ballman wrote: > How well do these test case

[PATCH] D70974: [clang-tidy] Fix PR26274

2019-12-06 Thread Alexander Kornienko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGfac4e3c5f8a0: [clang-tidy] Fix PR26274 (authored by alexfh). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70974/new/ https://reviews.llvm.org/D70974 Files:

[PATCH] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

2019-12-10 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: include/clang/Tooling/DiagnosticsYaml.h:75 std::string BuildDirectory; +ArrayRef Ranges; }; We should serialize the ran

[PATCH] D75538: [clang-tidy] Updated language supported restrictions on some checks

2020-03-05 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/modernize-use-nullptr-basic.cpp:1 -// RUN: %check_clang_tidy -std=c++98 %s modernize-use-nullptr %t -- -- -Wno-non-literal-null-conversion -// njames93 wrote: > alexfh wrote: >

[PATCH] D76083: [clang-tidy] Expand the list of functions in bugprone-unused-return-value

2020-03-12 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp:57 + "::std::map::lower_bound;" + "::std::move;" + "::std::multimap::eq

[PATCH] D75911: [clang-tidy] Added hasAnyListedName matcher

2020-03-12 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/Matchers.h:53-55 +ast_matchers::internal::Matcher +hasAnyListedName(std::vector Names); + We could change all checks to use the other overload and drop this one (and the utils::options:

[PATCH] D75184: [clang-tidy] Optional inheritance of file configs from parent directories 

2020-03-19 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In D75184#1914788 , @njames93 wrote: > In D75184#1914705 , @DmitryPolukhin > wrote: > > > You are absolutely right about current behaviour. Thank you for catching > > this odd behaviour. I'

[PATCH] D73841: [clang-tidy] Fixed crash 44745 in readability-else-after-return

2020-02-01 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG. Thanks for the fix! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73841/new/ https://reviews.llvm.org/D73841

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

2020-02-01 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: clang-tools-extra/clang-tidy/rename_check.py:172-174 + lineMatcher = re.compile('Renamed checks') + nextSectionMatcher = re.compile('Improvements to include-fixer') + checkMatcher = re.compile('- The \'(.*)') What's th

[PATCH] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

2020-02-18 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. Thanks for the update! Please upload patches with full context. That makes navigating the code much easier during reviews. See https://llvm.org/docs/Phabricator.html A few more comments inline. Comment at: include/clang/Tooling/Core/Diagnostic.h:62-70

[PATCH] D73236: [clang-tidy] Add clang-tidy headers to clang distribution

2020-01-23 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG with an outstanding comment. Comment at: clang-tools-extra/clang-tidy/CMakeLists.txt:104 +PATTERN "*.h" +PATTERN ".svn" EXCLUDE +) Is this sti

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

2020-01-27 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. How is this different from `-Wmissing-prototypes`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73413/new/ https://reviews.llvm.org/D73413 ___ cfe-commits mailing list cfe-comm

[PATCH] D73464: [clang] Add TagDecl AST matcher

2020-01-27 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. A drive-by comment. Comment at: clang/lib/ASTMatchers/Dynamic/Registry.cpp:212 + REGISTER_MATCHER(tagDecl); + REGISTER_MATCHER(tagType); REGISTER_MATCHER(enumConstantDecl); There's already `tagType` registration on the line 497. And

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

2020-01-27 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. In D72527#1843427 , @Eugene.Zelenko wrote: > Just ping. Are you waiting for other reviewers or for me to "Accept Revision"? If the latter, "stamped"

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

2020-01-28 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In D72527#1843475 , @Eugene.Zelenko wrote: > As I wrote, I don't have GitHub commit access, so I need help with commit. Sorry, missed that part of the patch description. As per Nathan's comment, please rebase the patch. Reposi

[PATCH] D44694: [clang-tidy] Use :doc: for check links in Release Notes

2018-03-21 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. LG. Thanks! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44694 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commi

[PATCH] D43766: [clang-tidy][modernize-make-unique] Checks c++14 flag before using std::make_unique

2018-03-21 Thread Alexander Kornienko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL328101: [clang-tidy][modernize-make-unique] Checks c++14 flag before using std… (authored by alexfh, committed by ). Herald added subscribers: llvm-commits, klimek. Changed prior to commit: https://revi

[PATCH] D44231: [clang-tidy] Check for sizeof that call functions

2018-03-21 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG modulo other reviewers' open comments. https://reviews.llvm.org/D44231 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.ll

[PATCH] D42730: [clang-tidy]] Add check for use of types/classes/functions from header which are deprecated and removed in C++17

2018-03-21 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: clang-tidy/modernize/DeprecatedFunctionalCheck.cpp:48-54 + } else if (const auto *const Call = + Result.Nodes.getNodeAs("ptr_fun_call")) { +diag(Call->getLocStart(), Message) << "'std::ptr_fun'"; + } else if (const a

[PATCH] D42730: [clang-tidy]] Add check for use of types/classes/functions from header which are deprecated and removed in C++17

2018-03-21 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: clang-tidy/modernize/DeprecatedFunctionalCheck.cpp:48-54 + } else if (const auto *const Call = + Result.Nodes.getNodeAs("ptr_fun_call")) { +diag(Call->getLocStart(), Message) << "'std::ptr_fun'"; + } else if (const a

[PATCH] D44765: PR36643 Make clang-format support more generic TMarcos

2018-03-22 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. We can't just treat `anything("")` like the _T macro. There should be a whitelist configurable with an option. By default only _T should be handled. Repository: rC Clang http

[PATCH] D44778: [clang-format] Wildcard expansion on Windows.

2018-03-22 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh created this revision. alexfh added reviewers: klimek, djasper. Herald added a subscriber: mgorny. Add support for wildcard expansion in command line arguments on Windows. See https://docs.microsoft.com/en-us/cpp/c-language/expanding-wildcard-arguments Fixes https://bugs.llvm.org/show_bug.

[PATCH] D44765: PR36643 Make clang-format support more generic TMarcos

2018-03-22 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In https://reviews.llvm.org/D44765#1045394, @Typz wrote: > A generic (or at least extandable) approach to specifying macro behaviors was > introduced here: https://reviews.llvm.org/D33440 I believe, that patch solves a significantly different problem and it won't make

[PATCH] D44778: [clang-format] Wildcard expansion on Windows.

2018-03-22 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In https://reviews.llvm.org/D44778#1046251, @rnk wrote: > Use `llvm::sys::Process::GetArgumentVector`, which already does wildcard > expansion from what I can see. It works with Unicode command lines and isn't > affected by locale. I vaguely remember that windows conso

[PATCH] D44778: [clang-format] Wildcard expansion on Windows.

2018-03-22 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In https://reviews.llvm.org/D44778#1046301, @alexfh wrote: > In https://reviews.llvm.org/D44778#1046251, @rnk wrote: > > > Use `llvm::sys::Process::GetArgumentVector`, which already does wildcard > > expansion from what I can see. It works with Unicode command lines and

[PATCH] D33440: clang-format: better handle statement macros

2018-03-23 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: lib/Format/Format.cpp:647-648 LLVMStyle.SortUsingDeclarations = true; + LLVMStyle.StatementMacros.push_back("Q_UNUSED"); + LLVMStyle.StatementMacros.push_back("QT_REQUIRE_VERSION"); What's the reason to have these

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

2018-03-23 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: docs/clang-tidy/checks/misc-exception-escape.rst:1 +.. title:: clang-tidy - bugprone-exception-escape + This file should be renamed as well. https://reviews.llvm.org/D33537 ___

[PATCH] D44778: [clang-format] Wildcard expansion on Windows.

2018-03-23 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh updated this revision to Diff 139593. alexfh added a comment. - Use llvm::sys::Process::GetArgumentVector. Repository: rC Clang https://reviews.llvm.org/D44778 Files: tools/clang-format/ClangFormat.cpp Index: tools/clang-format/ClangFormat.cpp =

[PATCH] D44778: [clang-format] Wildcard expansion on Windows.

2018-03-23 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. I've not tested this on Windows, but I hope llvm::sys::Process::GetArgumentVector should have been tested by someone already. Repository: rC Clang https://reviews.llvm.org/D44778 ___ cfe-commits mailing list cfe-commits@

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

2018-03-23 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:24-25 + +const TypeVec _throws(const FunctionDecl *Func); +const TypeVec _throws(const Stmt *St, const

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

2018-03-23 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: docs/clang-tidy/checks/bugprone-parent-virtual-call.rst:26 + int foo() override {... A::foo()...} + warning: qualified name A::foo refers to a +member which wa

[PATCH] D37014: [clang-tidy] Add a checker to remove useless intermediate variables before return statements with comparisons

2018-03-23 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: clang-tidy/readability/UnnecessaryIntermediateVarCheck.cpp:28 + auto File = Context->getCurrentFile(); + auto Style = format::getStyle(*Context->g

[PATCH] D41456: [clang-tidy] readability-else-after-return: also diagnose noreturn function calls.

2018-03-23 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. The problem with this proposed functionality is that it's not always obvious at the call site whether a function is noreturn. exit() may be an easy case, but there may be much less o

[PATCH] D44765: PR36643 Make clang-format support more generic TMarcos

2018-03-23 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In https://reviews.llvm.org/D44765#1045789, @obfuscated wrote: > In https://reviews.llvm.org/D44765#1045373, @alexfh wrote: > > > We can't just treat `anything("")` like the _T macro. There should be a > > whitelist configurable with an option. By default only _T shou

[PATCH] D44778: [clang-format] Wildcard expansion on Windows.

2018-03-26 Thread Alexander Kornienko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC328495: [clang-format] Wildcard expansion on Windows. (authored by alexfh, committed by ). Changed prior to commit: https://reviews.llvm.org/D44778?vs=139593&id=139784#toc Repository: rC Clang https

[PATCH] D44765: PR36643 Make clang-format support more generic TMarcos

2018-03-26 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. Thank you! One more comment from me and I'll leave the rest of the review to Krasimir, who has a better idea of how the configuration options should be named etc.

[PATCH] D44231: [clang-tidy] Check for sizeof that call functions

2018-03-26 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. LG. Do you need someone to submit this for you? Comment at: test/clang-tidy/misc-sizeof-expression.cpp:17 +enum E { E_VALUE = 0 }; + aaron.ballman wrote: > Can you add a C++11 test case using `enum class`

[PATCH] D38455: [clang-tidy] new cppcoreguidelines-narrowing-conversions check.

2018-03-26 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. > I agree that more data would be useful, so I'll do an analysis of flagging > all (non-ceil/floor float/double)->integral conversions. Removing from my dashboard for now. Reposito

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

2018-03-26 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: docs/clang-tidy/checks/bugprone-parent-virtual-call.rst:26 + int foo() override {... A::foo()...} + warning: qual

[PATCH] D44893: [ASTMatchers] Add isAssignmentOperator matcher

2018-03-26 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG! Repository: rC Clang https://reviews.llvm.org/D44893 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin

[PATCH] D44906: [clang-tidy] Define __clang_analyzer__ macro for clang-tidy for compatibility with clang static analyzer

2018-03-27 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: clang-tidy/ClangTidy.cpp:490 +CommandLineArguments AdjustedArgs = Args; +AdjustedArgs.emplace_back("-D__clang_analyzer__"); +return AdjustedArgs; I wonder whether we should instead reuse the logic

[PATCH] D44906: [clang-tidy] Define __clang_analyzer__ macro for clang-tidy for compatibility with clang static analyzer

2018-03-28 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG Comment at: clang-tidy/ClangTidy.cpp:490 +CommandLineArguments AdjustedArgs = Args; +AdjustedArgs.emplace_back("-D__clang_analyzer__"); +return Adj

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

2018-03-28 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. A few more nits. Comment at: clang-tidy/bugprone/ParentVirtualCallCheck.cpp:114-117 + auto ParentIter = Parents.begin(); + std::string ParentsStr = "'" + getNameA

[PATCH] D45258: [clang-tidy] Return non-zero exit code for clang errors.

2018-04-04 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh created this revision. alexfh added a reviewer: ilya-biryukov. Herald added subscribers: xazax.hun, klimek. Updated tests broken by this change. Fixes https://bugs.llvm.org/show_bug.cgi?id=27628 Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45258 Files: clang-tidy/too

[PATCH] D44765: PR36643 Make clang-format support more generic TMarcos

2018-04-04 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: include/clang/Format/Format.h:1676 + /// \brief A vector of macros that should be interpreted as macros expanding + /// to a string literal encoding prefix instead of as function calls. "A list of macro names"?

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

2018-04-04 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: clang-tidy/bugprone/ParentVirtualCallCheck.cpp:73 +// 'typedef'ed grand-parent classes. +// Adopted from https://stackoverflow.com/a/23081770/107308

[PATCH] D37014: [clang-tidy] Add a checker to remove useless intermediate variables before return statements with comparisons

2018-04-04 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: clang-tidy/readability/UnnecessaryIntermediateVarCheck.h:27 +// Matches the next statement within the parent statement sequence. +AST_MATCHER_P(Stmt

[PATCH] D33844: [clang-tidy] terminating continue check

2018-04-05 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. Please move the check to bugprone- module. clang-tidy/rename_check.py script should do most of the job (you'll have to remove the unnecessary "renamed check ..." entry in the release notes). Comment at: clang-tidy/misc/TerminatingContinueCheck.cpp:27 +

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

2018-04-05 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:100 + +namespace { + > make anonymous namespaces as small as possible, and only use th

[PATCH] D40937: [clang-tidy] Infinite loop checker

2018-04-05 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: clang-tidy/bugprone/InfiniteLoopCheck.cpp:23-25 +static internal::Matcher loopEndingStmt() { + return stmt(anyOf(breakStmt(), returnStmt(), gotoStm

[PATCH] D45096: Allow the creation of human-friendly ASTDumper to arbitrary output stream

2018-04-05 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good to me. One optional comment. Comment at: include/clang/Frontend/ASTConsumers.h:39-41 std::unique_ptr CreateASTDumper(StringRef FilterString,

[PATCH] D45059: [clang-tidy] Add check to catch comparisons in TEMP_FAILURE_RETRY

2018-04-05 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. The TEMP_FAILURE_RETRY macro is specific to the GNU C library (and environments that attempt to mimic it). The generic bugprone- module is not the best place for this check. I sugges

[PATCH] D45096: Allow the creation of human-friendly ASTDumper to arbitrary output stream

2018-04-06 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. Another couple of nits. Comment at: docs/HowToSetupToolingForLLVM.rst:136 if (this->ASTDump.operator _Bool()) - return clang::CreateASTDumper(this->ASTDumpFilter); + return clang::CreateASTDumper(nullptr, this->ASTDumpFilter);

[PATCH] D45096: Allow the creation of human-friendly ASTDumper to arbitrary output stream

2018-04-06 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG. I'll commit the patch for you. Repository: rC Clang https://reviews.llvm.org/D45096 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D45096: Allow the creation of human-friendly ASTDumper to arbitrary output stream

2018-04-06 Thread Alexander Kornienko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL329391: Allow the creation of human-friendly ASTDumper to arbitrary output stream (authored by alexfh, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.l

[PATCH] D44188: Misc typos

2018-04-06 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. Thanks! I've looked through all the fixes. Most of them look reasonable. I've reverted a few files where I found something suspicious or outright wrong and will commit the rest of the patch after I ensure the tests pass. These files couldn'

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

2018-04-06 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG https://reviews.llvm.org/D44295 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

[PATCH] D44188: Misc typos

2018-04-06 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. Feel free to send the rest of the fixes as a separate patch. Repository: rL LLVM https://reviews.llvm.org/D44188 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-co

[PATCH] D45405: [clang-tidy] [modernize-use-auto] Add a threshold for minimal type name length to be replaced with 'auto'

2018-04-09 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: clang-tidy/modernize/UseAutoCheck.cpp:290 +: ClangTidyCheck(Name, Context), RemoveStars(Options.get("RemoveStars", 0)), + MinTypeNameLength(Options.get("MinTypeNameLength", 0)) {} Maybe make the default 5? Or

[PATCH] D45392: [clang-tidy] add new check to find out objc ivars which do not have prefix '_'

2018-04-09 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. I wonder whether the readability-identifier-naming check could be extended to support this use case instead of adding a new check specifically for underscores in ivar names? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45392 ___

[PATCH] D43779: [Tooling] [0/1] Refactor FrontendActionFactory::create() to return std::unique_ptr<>

2018-04-09 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In https://reviews.llvm.org/D43779#1061043, @lebedev.ri wrote: > Hmm. > Got back to this issue. > > Not reproducible with gcc-4.8.5 and gcc-4.9.3 in ubuntu 16.04 chroot. > *Reproducible* with gcc-4.8.4 and gcc-4.9.2 in debian oldstable (Jessie) > chroot. > So one might

[PATCH] D38455: [clang-tidy] new cppcoreguidelines-narrowing-conversions check.

2018-04-09 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In https://reviews.llvm.org/D38455#1061195, @courbet wrote: > ping Sorry for the long review due to the holidays. Generally, I would also like Aaron to take a look when he's back, since he had some concerns. While we're waiting, one minor comment from me. ==

[PATCH] D45405: [clang-tidy] [modernize-use-auto] Add a threshold for minimal type name length to be replaced with 'auto'

2018-04-09 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: clang-tidy/modernize/UseAutoCheck.cpp:290 +: ClangTidyCheck(Name, Context), RemoveStars(Options.get("RemoveStars", 0)), + MinTypeNameLengt

[PATCH] D45258: [clang-tidy] Return non-zero exit code for clang errors.

2018-04-09 Thread Alexander Kornienko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE329579: [clang-tidy] Return non-zero exit code for clang errors. (authored by alexfh, committed by ). Changed prior to commit: https://reviews.llvm.org/D45258?vs=140956&id=141656#toc Repository: rL

[PATCH] D45258: [clang-tidy] Return non-zero exit code for clang errors.

2018-04-09 Thread Alexander Kornienko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL329579: [clang-tidy] Return non-zero exit code for clang errors. (authored by alexfh, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D45258 Files

[PATCH] D45405: [clang-tidy] [modernize-use-auto] Add a threshold for minimal type name length to be replaced with 'auto'

2018-04-09 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: clang-tidy/modernize/UseAutoCheck.cpp:290 +: ClangTidyCheck(Name, Context), RemoveStars(Options.get("RemoveStars", 0)), + MinTypeNameLength(Options.get("MinTypeNameLength", 0)) {} zinovy.nis wrote: > alexfh wr

[PATCH] D40937: [clang-tidy] Infinite loop checker

2018-04-09 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: clang-tidy/bugprone/InfiniteLoopCheck.cpp:94-99 + std::unique_ptr TheCFG = + CFG::buildCFG(nullptr, FunctionBody, &ASTCtx, Options); + if (!T

[PATCH] D38455: [clang-tidy] new cppcoreguidelines-narrowing-conversions check.

2018-04-10 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In https://reviews.llvm.org/D38455#1062722, @courbet wrote: > In https://reviews.llvm.org/D38455#1062718, @JonasToth wrote: > > > > Sure. Is it OK to add a dependency from > > > clang-tidy/bugprone/BugproneTidyModule.cpp to stuff in > > > clang-tidy/cpp-coreguidelines ?

[PATCH] D44765: PR36643 Make clang-format support more generic TMarcos

2018-04-10 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: lib/Format/FormatTokenLexer.cpp:371 FormatToken *Macro = Tokens[Tokens.size() - 4]; - if (Macro->TokenText != "_T") + if (std::find(Style.TMacros.begin(), Style.TMacros.end(), Macro->TokenText) == + Style.TMacros.end()) ---

[PATCH] D45405: [clang-tidy] [modernize-use-auto] Add a threshold for minimal type name length to be replaced with 'auto'

2018-04-10 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good. Thank you! https://reviews.llvm.org/D45405 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mail

[PATCH] D45392: [clang-tidy] add new check to find out objc ivars which do not have prefix '_'

2018-04-10 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In https://reviews.llvm.org/D45392#1061960, @Wizard wrote: > In https://reviews.llvm.org/D45392#1061433, @alexfh wrote: > > > I wonder whether the readability-identifier-naming check could be extended > > to support this use case instead of adding a new check specifically

[PATCH] D45059: [clang-tidy] Add check to catch comparisons in TEMP_FAILURE_RETRY

2018-04-10 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG. Thanks! https://reviews.llvm.org/D45059 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinf

[PATCH] D45405: [clang-tidy] [modernize-use-auto] Add a threshold for minimal type name length to be replaced with 'auto'

2018-04-11 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In https://reviews.llvm.org/D45405#1063890, @zinovy.nis wrote: > Roman, I see you've fixed them. Thanks a lot! > I did not face with these errors on MSVS'201 so had no chance to fix early. No stress, but as Roman said, please watch the bots after committing a patch: h

[PATCH] D45392: [clang-tidy] add new check to find out objc ivars which do not have prefix '_'

2018-04-11 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. Thank you, that's much better! I'd also appreciate, if you could document the options this check supports in its .rst document (separately from this patch). A few more comments inli

[PATCH] D38171: [clang-tidy] Implement clang-tidy check aliases

2017-11-15 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In https://reviews.llvm.org/D38171#925929, @xazax.hun wrote: > In https://reviews.llvm.org/D38171#909346, @leanil wrote: > > > In https://reviews.llvm.org/D38171#901427, @xazax.hun wrote: > > > > > One problem to think about when we add all clang-diagnostic as "first or >

[PATCH] D38171: [clang-tidy] Implement clang-tidy check aliases

2017-11-15 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. And, btw, sorry for the long delay. I've been on travelling / on vacation for the last few weeks. https://reviews.llvm.org/D38171 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/

[PATCH] D153430: [Clang][Driver] Warn on invalid Arm or AArch64 baremetal target triple

2023-06-27 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. After the patch, LLVM still has a number of `aarch64-arm-none-eabi` usages in tests, which is also considered invalid now: https://gcc.godbolt.org/z/z8cY5j68M Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153430/new/ https:

[PATCH] D143840: [clang] Add the check of membership for the issue #58674 and improve the lookup process

2023-02-23 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In D143840#4147582 , @joanahalili wrote: > In D143840#4147234 , @joanahalili > wrote: > >> Heads up: We are experiencing a series of clang crashes because of this >> commit. >> >> What w

[PATCH] D27813: [clang-tidy] fix missing anchor for MPI Module

2016-12-15 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. LG, thank you! https://reviews.llvm.org/D27813 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27187: [clang-tidy] Do not move parameter if only DeclRefExpr occurs inside of a loop

2016-12-15 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. Thanks, this is fine to commit. https://reviews.llvm.org/D27187 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D22507: Clang-tidy - Enum misuse check

2016-12-19 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. A few more notes, all fine for a follow up. Comment at: clang-tidy/misc/SuspiciousEnumUsageCheck.cpp:202 + + const auto *LhsExpr = Result.Nodes.getNodeAs("lhsExpr"); + const auto *RhsExpr = Result.Nodes.getNodeAs("rhsExpr"); Looks like

<    3   4   5   6   7   8   9   10   11   12   >