[PATCH] D59870: [clang-tidy] Add MagnitudeBitsUpperLimit option to bugprone-too-small-loop-variable

2019-03-28 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. > I think it's the easiest way to specify the bits of the ineteger type to > limit the catches. In real life, I met with this overflow / infinite loop > problem with 16-bit short type, so I think the real use cases are 8 and 16 > bit integers. It seems intuitive to me

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

2019-03-28 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. I think the idea is good and implementation, too. If we iterate all checks anyway (probably?) we could think about adding a severity to the checks, too? I know that code-checker and code-compass have something like this to signal importance of problems (say bugprone a

[PATCH] D59870: [clang-tidy] Add MagnitudeBitsUpperLimit option to bugprone-too-small-loop-variable

2019-03-27 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. I think in general this is a good direction. What do you think about other heuristics? E.g. one could say that a vector will not contain more then X GB or similar. That is probably more complicated to figure out though. Comment at: docs/clang-tidy/c

[PATCH] D59650: [NFC] ExceptionEscapeCheck: small refactoring

2019-03-21 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/utils/ExceptionAnalyzer.cpp:226 +ExceptionAnalyzer::ExceptionInfo +ExceptionAnalyzer::analyzeBoilerplate(const T *Node) { + ExceptionInfo ExceptionList; lebedev.ri wrote: > Please bikeshed on the name. I do

[PATCH] D59650: [NFC] ExceptionEscapeCheck: small refactoring

2019-03-21 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. > Looks like pointless code duplication that is easily avoidable. True, but I would prefer the refactoring to be private then (so the template stuff with the boilerplate) and a simple overloading interface dispatching to the templated stuff. In the end I think the pub

[PATCH] D59650: [NFC] ExceptionEscapeCheck: small refactoring

2019-03-21 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. Why not having normal overloads? The analysis for `Stmt` is implemented with the private methods. Explicit template specialization is a bit overkill and so easily understood (but not too complex in this case either).l Repository: rCTE Clang Tools Extra CHANGES SIN

[PATCH] D59540: [clang-tidy] [PR41119] readability-identifier-naming incorrectly fixes lambda capture

2019-03-19 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. What happens on `[=]() ...`, direct capture `[&Columns]()...` and `[Columns]()...`? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59540/new/ https://reviews.llvm.org/D59540 ___ cfe-commits mailing list cfe-commit

[PATCH] D57108: [clang-tidy] diagnose possibiltiy to add 'noexcept' in modernize-use-noexcept

2019-03-18 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. In D57108#1433087 , @baloghadamsoftware wrote: > Ping! Any news regarding this patch? Its on the todolist, but i currently have many things to do in my real life, so no time for clang-tidy development :( Repository: rCTE

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

2019-03-15 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. ping :) Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54395/new/ https://reviews.llvm.org/D54395 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org

[PATCH] D56343: [clang-tidy] Refactor: Extract Class CheckRunner on check_clang_tidy.py

2019-03-15 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. From my side its LGTM, but I would let @serge-sans-paille accept, as he is probably more familiar with python then I am. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56343/new/ https://reviews.llvm.org/D56343 __

[PATCH] D59318: [clang-tidy] add an overload for diag method that allows users to provide a diagnostic name rather than using the check name when building a diagnostic.

2019-03-15 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. What is the reason you want this change to happen? I think this gives the chance to create inconsistencies which we should avoid. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59318/new/ https://reviews.llvm.org/D59318

[PATCH] D59103: [clang-tidy] New checker bugprone-incomplete-comparison-operator

2019-03-07 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. @lebedev.ri @kallehuttunen what do you think of putting this into context of the proposal (i believe its being standardized with c++20) of `operator==() = default` and the spaceship-operator. This check could serve as a basis to modernize the `operator==` to the defaul

[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check

2019-03-07 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. IMHO the check is close to being finished. please address the notes and mark them as done if finished with it. So its clear to see whats outstanding. In my opinion the user-facing documentation misses a "Limitations" sections that shows the artificial `goto` example, t

[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check

2019-03-07 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: test/clang-tidy/cppcoreguidelines-use-raii-locks.cpp:90 + for (auto i = 0; i < 3; i++) { +m.lock(); +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use RAII JonasToth wrote: > lewmpk wrote: > > JonasToth wrote: >

[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check

2019-03-03 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: test/clang-tidy/cppcoreguidelines-use-raii-locks.cpp:90 + for (auto i = 0; i < 3; i++) { +m.lock(); +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use RAII lewmpk wrote: > JonasToth wrote: > > I got another one

[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check

2019-03-02 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: test/clang-tidy/cppcoreguidelines-use-raii-locks.cpp:90 + for (auto i = 0; i < 3; i++) { +m.lock(); +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use RAII I got another one that I think defeats the analysis: `

[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check

2019-03-02 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: test/clang-tidy/cppcoreguidelines-use-raii-locks.cpp:4 +// Mock implementation of std::mutex +namespace std { +struct mutex { lewmpk wrote: > JonasToth wrote: > > Please add more tests > > > > What happens for this? >

[PATCH] D58819: [clang-format] clang-format off/on not respected when using C Style comments

2019-03-01 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang/lib/Format/Format.cpp:1792 +else if (Trimmed == "// clang-format on" || + Trimmed == "/* clang-format on */") FormattingOff = false; MyDeveloperDay wrote: > JonasToth wrote: > > Should we a

[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check

2019-03-01 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp:35 +void UseRaiiLocksCheck::registerMatchers(MatchFinder *Finder) { + // lock_guards require c++11 or later + if (!getLangOpts().CPlusPlus11) If we allow boost, pre

[PATCH] D58819: [clang-format] clang-format off/on not respected when using C Style comments

2019-03-01 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang/lib/Format/Format.cpp:1792 +else if (Trimmed == "// clang-format on" || + Trimmed == "/* clang-format on */") FormattingOff = false; Should we allow ``` /* clang-format off It is just

[PATCH] D57087: [clang-tidy] add OverrideMacro to modernize-use-override check

2019-02-28 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth accepted this revision. JonasToth added a comment. LGTM Comment at: clang-tidy/modernize/UseOverrideCheck.cpp:196 + ReplacementText = " " + OverrideSpelling + " "; +} } else if (GetText(Tokens.back(), Sources) == "ABSTRACT") {

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

2019-02-28 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. In D58731#1413888 , @MyDeveloperDay wrote: > @JonasToth I left a comment in the commit needed to fix the index.rst, which > I don't think your later review fixes, sphinx complained about the rst file > being an unreferenced oc

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

2019-02-28 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. Thank you for the patch! Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58731/new/ https://reviews.llvm.org/D58731 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.o

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

2019-02-28 Thread Jonas Toth via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL355093: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions (authored by JonasToth, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to com

[PATCH] D58764: [clang-tidy] ignore predefined expressions in cppcoreguidelines-pro-bounds-array-to-pointer-decay check

2019-02-28 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. You can abondon this. A short justification (with reference to the other revision) on the bug report would be great! Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58764/new/ https://reviews.llvm.org/D58764

[PATCH] D58609: [clang-tidy] bugprone-string-integer-assignment: Reduce false positives.

2019-02-28 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land. LGTM. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58609/new/ https://reviews.llvm.org/D58609 _

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

2019-02-28 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth marked an inline comment as done. JonasToth added inline comments. Comment at: unittests/clang-tidy/AddConstTest.cpp:15 + +template alexfh wrote: > What's the point of default values of template arguments here? Wups, relict of older version. Reposito

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

2019-02-28 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 188716. JonasToth marked 3 inline comments as done. JonasToth added a comment. - address review comments - rebase to master Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54395/new/ https://reviews.llvm.org/D54

[PATCH] D56160: [clang-tidy] modernize-use-trailing-return-type check

2019-02-28 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. From my side only the nits are left. Comment at: clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:94 + bool VisitDeclRefExpr(DeclRefExpr *S) { +const DeclarationName Name = S->getNameInfo().getName(); +if (!S->getQualifierLoc() && Name.isI

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

2019-02-28 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. In D58731#1413498 , @MyDeveloperDay wrote: > In D58731#1413476 , @lewmpk wrote: > > > I'm happy to land this ASAP but I don't have commit rights > > > So one of us could land it for you..

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

2019-02-27 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. I think this change is worth mentioning in the release notes as well (cte/docs/ReleaseNotes.rst) Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58731/new/ https://reviews.llvm.org/D58731

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

2019-02-27 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. Welcome to the LLVM community and thank you for the patch lewmpk! Please add unit tests for the changes you made to the check. They live in `clang-tools-extra/test/clang-tidy/modernize-`. It is common to add a additional test-file to ensure the configuration option

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

2019-02-27 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. Herald added a subscriber: jdoerfert. another ping. @alexfh and @aaron.ballman you commented on prior versions. Would be nice if you could take a (final) look at this patch! Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54

[PATCH] D56160: [clang-tidy] modernize-use-trailing-return check

2019-02-27 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. > It took me far too long to come up with an update. Honestly, I was quite > demotivated as it turned out preventing name collisions of unqualifed names > after the rewrite was more difficult than I thought. Especially because this > error occurs sparsely when I test

[PATCH] D58137: [clang-tidy] Add the abseil-time-subtraction check

2019-02-27 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land. I am happy now :) Thank you for the patch, LGTM Comment at: test/clang-tidy/abseil-time-subtraction.cpp:12 + + d = absl::Hours(absl::ToUnixHours(t) - x); + // CHECK-M

[PATCH] D58609: [clang-tidy] bugprone-string-integer-assignment: Reduce false positives.

2019-02-27 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp:54 + (RHS->EvaluateAsInt(RHSVal, Ctx, Expr::SE_AllowSideEffects) || + LHS->EvaluateAsInt(RHSVal, Ctx, Expr::SE_AllowSideEffects))) { +return true; unn

[PATCH] D58606: [clang-tidy] misc-string-integer-assignment: fix false positive

2019-02-27 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land. LGTM. is there a bug or similar? If yes please mention it somewhere in the summary or so and close it :) Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://review

[PATCH] D57883: [clang-tidy] refactor ExceptionAnalyzer further to give ternary answer

2019-02-26 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth marked an inline comment as done. JonasToth added inline comments. Comment at: clang-tidy/utils/ExceptionAnalyzer.h:112 +/// throw, if it's unknown or if it won't throw. +enum State Behaviour : 2; + bjope wrote: > (post-commit comments) > > I'v

[PATCH] D58137: [clang-tidy] Add the abseil-time-subtraction check

2019-02-23 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/abseil/TimeSubtractionCheck.cpp:97 +void TimeSubtractionCheck::check(const MatchFinder::MatchResult &Result) { + const auto *BinOp = Result.Nodes.getNodeAs("binop"); + std::string inverse_name = hwright wr

[PATCH] D57883: [clang-tidy] refactor ExceptionAnalyzer further to give ternary answer

2019-02-20 Thread Jonas Toth via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE354517: [clang-tidy] refactor ExceptionAnalyzer further to give ternary answer (authored by JonasToth, committed by ). Changed prior to commit: https://reviews.llvm.org/D57883?vs=187635&id=187662#toc

[PATCH] D57883: [clang-tidy] refactor ExceptionAnalyzer further to give ternary answer

2019-02-20 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 187635. JonasToth marked an inline comment as done. JonasToth added a comment. - be explicit about the State enumerator values Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57883/new/ https://reviews.llvm.org/

[PATCH] D57883: [clang-tidy] refactor ExceptionAnalyzer further to give ternary answer

2019-02-20 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth marked 3 inline comments as done. JonasToth added a comment. all comments resolved. I will land this now. Comment at: clang-tidy/utils/ExceptionAnalyzer.h:26-31 + enum class State : std::int8_t { +Throwing,///< The function can definitly throw given an AST. +

[PATCH] D57883: [clang-tidy] refactor ExceptionAnalyzer further to give ternary answer

2019-02-20 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 187634. JonasToth marked 2 inline comments as done. JonasToth added a comment. - [Refactor] move support classes into the analyzer - [Refactor] move bigger methods into implementation file - minor adjustments Repository: rCTE Clang Tools Extra CHANGES S

[PATCH] D58137: [clang-tidy] Add the abseil-time-subtraction check

2019-02-16 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/abseil/TimeSubtractionCheck.cpp:72 + +// Match the cases where we know that the result is a Duration and the first +// argument is a Time. Just knowing the type of the first operand is not Eugene.Z

[PATCH] D58137: [clang-tidy] Add the abseil-time-subtraction check

2019-02-16 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/abseil/DurationRewriter.cpp:92 + DurationScale Scale, const Expr &Node) { + const llvm::StringRef &InverseFunction = getTimeInverseForScale(Scale); + if (const auto *MaybeCallArg = selectFirst( --

[PATCH] D57966: [clang-tidy] add camelBackOrCase casing style to readability-identifier-naming to support change to variable naming policy (if adopted)

2019-02-14 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Herald added a subscriber: jdoerfert. Comment at: test/clang-tidy/readability-identifier-naming-camelback-or-case.cpp:97 +class aB_def +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style +{ why no test for fixing? R

[PATCH] D57883: [clang-tidy] refactor ExceptionAnalyzer further to give ternary answer

2019-02-14 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. ping :) Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57883/new/ https://reviews.llvm.org/D57883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org

[PATCH] D57662: [clang-tidy] Parallelise clang-tidy-diff.py

2019-02-14 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/tool/clang-tidy-diff.py:51 + + if not timeout is None: +watchdog = threading.Timer(timeout, proc.kill) `if timeout is not None` is more readable. Comment at: clang-tidy/tool/c

[PATCH] D57662: [clang-tidy] Parallelise clang-tidy-diff.py

2019-02-13 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. In D57662#1392509 , @MyDeveloperDay wrote: > Just a question.. If clang tidy is running with -fix in parallel, what stops > each clang-tidy invocation altering a common header at the same time? `clang-apply-replacements` does

[PATCH] D57852: [clang-tidy] Don't use assignment for value-initialized enums

2019-02-08 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57852/new/ https://reviews.llvm.org/D57852 _

[PATCH] D57883: [clang-tidy] refactor ExceptionAnalyzer further to give ternary answer

2019-02-08 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth marked 2 inline comments as done. JonasToth added a comment. Good idea :) Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57883/new/ https://reviews.llvm.org/D57883 ___ cfe-commits mailing list

[PATCH] D57883: [clang-tidy] refactor ExceptionAnalyzer further to give ternary answer

2019-02-08 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 185939. JonasToth added a comment. - [Refactor] move support classes into the analyzer - [Refactor] move bigger methods into implementation file Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57883/new/ https:/

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

2019-02-07 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. Herald added a project: clang. ping Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54395/new/ https://reviews.llvm.org/D54395 ___ cfe-commits mailing list cfe-commits@lists.llv

[PATCH] D57883: [clang-tidy] refactor ExceptionAnalyzer further to give ternary answer

2019-02-07 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth marked 2 inline comments as done. JonasToth added inline comments. Comment at: clang-tidy/utils/ExceptionAnalyzer.h:31-192 +enum class ExceptionState : std::int8_t { + Throwing,///< The function can definitly throw given an AST. + NotThrowing, ///< This function c

[PATCH] D57883: [clang-tidy] refactor ExceptionAnalyzer further to give ternary answer

2019-02-07 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth created this revision. JonasToth added reviewers: lebedev.ri, aaron.ballman, baloghadamsoftware, alexfh. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, a.sidorin, rnkovacs, xazax.hun. Herald added a project: clang. The analsis on the throwing behvaiour on functio

[PATCH] D57852: [clang-tidy] Don't use assignment for value-initialized enums

2019-02-07 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. How are the semantics for `enum class` in this case? Comment at: clang-tools-extra/test/clang-tidy/modernize-use-default-member-init-assignment.cpp:172 + // CHECK-FIXES: PositiveValueEnum() {} + Enum e; + // CHECK-MESSAGES: :[[@LINE-1]]:8: warnin

[PATCH] D57787: [clang-tidy] modernize-avoid-c-arrays: avoid main function (PR40604)

2019-02-06 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/modernize/AvoidCArraysCheck.cpp:35 + const clang::DeclContext *DC = Node.getDeclContext(); + const clang::FunctionDecl *FD = llvm::dyn_cast(DC); + if (!FD) lebedev.ri wrote: > lebedev.ri wrote: > > JonasT

[PATCH] D57787: [clang-tidy] modernize-avoid-c-arrays: avoid main function (PR40604)

2019-02-05 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/modernize/AvoidCArraysCheck.cpp:35 + const clang::DeclContext *DC = Node.getDeclContext(); + const clang::FunctionDecl *FD = llvm::dyn_cast(DC); + if (!FD) There is `FunctionDecl->castToDeclContext()` whi

[PATCH] D57665: [clang-tidy] Handle unions with existing default-member-init

2019-02-03 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land. LGTM, please close the bug report and notify for the fix with the request to check in the real-project if it actually fixes the issue. Thanks :) Repository: rG LLVM Github Monorepo C

[PATCH] D57571: [clang-tidy] A new OpenMP module

2019-02-01 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land. LGTM. Committing both revision together would be good :) Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57571/new/ https://reviews.llvm.org/

[PATCH] D57108: [clang-tidy] diagnose possibiltiy to add 'noexcept' in modernize-use-noexcept

2019-01-31 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 184477. JonasToth added a comment. - Merge branch 'master' into feature-introduce-noexcept Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57108/new/ https://reviews.llvm.org/D57108 Files: clang-tidy/moderniz

[PATCH] D57100: [clang-tidy] refactor bugprone-exception-escape analysis into class

2019-01-31 Thread Jonas Toth via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL352741: [clang-tidy] refactor bugprone-exception-escape analysis into class (authored by JonasToth, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm

[PATCH] D57100: [clang-tidy] refactor bugprone-exception-escape analysis into class

2019-01-31 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. In D57100#1377687 , @baloghadamsoftware wrote: > In D57100#1373440 , @JonasToth wrote: > > > @baloghadamsoftware Do you see any problems with the refactoring, > > especially the new `Fun

[PATCH] D57100: [clang-tidy] refactor bugprone-exception-escape analysis into class

2019-01-31 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 184475. JonasToth added a comment. - [Misc] rewrite parts of a comment that were unprecise. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57100/new/ https://reviews.llvm.org/D57100 Files: clang-tidy/bugpron

[PATCH] D57100: [clang-tidy] refactor bugprone-exception-escape analysis into class

2019-01-28 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. In D57100#1373430 , @baloghadamsoftware wrote: > Hello! I am glad to see that this check gets improved by the community. I > also think a "modernize" check which marks functions with `noexcept` is also > useful. Great to hea

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

2019-01-28 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. ping. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54395/new/ https://reviews.llvm.org/D54395 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/c

[PATCH] D57185: [clang-tidy] Add the abseil-duration-addition check

2019-01-28 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57185/new/ https://reviews.llvm.org/D57185 ___ cfe-commits mailing list cfe-com

[PATCH] D57113: [clang-tidy] openmp-use-default-none - a new module and a check

2019-01-27 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land. LGTM with the few language nits. The new matchers look better and are more matcher style, gj :) Comment at: clang-tidy/openmp/UseDefaultNoneCheck.cpp:126 + // Don't re

[PATCH] D57249: [clang-tidy] fix unit tests for dropped _Float16 support in X86

2019-01-25 Thread Jonas Toth via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL352231: [clang-tidy] fix unit tests for dropped _Float16 support in X86 (authored by JonasToth, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGES SINCE LAST ACTION

[PATCH] D57249: [clang-tidy] fix unit tests for dropped _Float16 support in X86

2019-01-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 183580. JonasToth marked an inline comment as done. JonasToth added a comment. - last nit, remove the disabled in hexadecimal Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57249/new/ https://reviews.llvm.org/D

[PATCH] D57249: [clang-tidy] fix unit tests for dropped _Float16 support in X86

2019-01-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 183579. JonasToth added a comment. - Revert "add REQUIRES: for the readability unit-tests where necessary" as its not necessary because only the frontend is run Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D5

[PATCH] D57249: [clang-tidy] fix unit tests for dropped _Float16 support in X86

2019-01-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 183578. JonasToth added a comment. - add REQUIRES: for the readability unit-tests where necessary Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57249/new/ https://reviews.llvm.org/D57249 Files: test/clang-

[PATCH] D57249: [clang-tidy] fix unit tests for dropped _Float16 support in X86

2019-01-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth created this revision. JonasToth added reviewers: aaron.ballman, erichkeane, lebedev.ri. Herald added subscribers: cfe-commits, kristof.beyls, xazax.hun, javed.absar. Because _Float16 was disabled for X86 targets the unit-tests started failing. Extract the pieces for _Float16 and run the

[PATCH] D57185: [clang-tidy] Add the abseil-duration-addition check

2019-01-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: test/clang-tidy/abseil-duration-addition.cpp:84 +#undef PLUS_FIVE +} hwright wrote: > JonasToth wrote: > > a view template test-cases would be good to have. > I'm not sure I know that terminology; do you have an exampl

[PATCH] D57185: [clang-tidy] Add the abseil-duration-addition check

2019-01-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/abseil/DurationAdditionCheck.cpp:22 +void DurationAdditionCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + binaryOperator(hasOperatorName("+"), hwright wrote: > JonasToth wrote: >

[PATCH] D57113: [clang-tidy] openmp-use-default-none - a new module and a check

2019-01-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: docs/clang-tidy/checks/openmp-use-default-none.rst:12 +being implicitly determined, and thus forces developer to be explicit about the +desired data scoping for each variable. + lebedev.ri wrote: > JonasToth wrote: > >

[PATCH] D57113: [clang-tidy] openmp-use-default-none - a new module and a check

2019-01-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/openmp/UseDefaultNoneCheck.cpp:113 +void UseDefaultNoneCheck::registerMatchers(MatchFinder *Finder) { + // If OpenMP is not enabled, don't register the check, it won't find anything. + if (!getLangOpts().OpenMP) -

[PATCH] D57113: [clang-tidy] openmp-use-default-none - a new module and a check

2019-01-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: docs/clang-tidy/checks/openmp-use-default-none.rst:12 +being implicitly determined, and thus forces developer to be explicit about the +desired data scoping for each variable. + If I understand correctly the issue is m

[PATCH] D57113: [clang-tidy] openmp-use-default-none - a new module and a check

2019-01-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/openmp/UseDefaultNoneCheck.cpp:51 +///instead of the actual OpenMPClauseKind. +AST_MATCHER_P(OMPExecutableDirective, isAllowedToContainClause, + OpenMPClauseKind, CKind) { Why is this re

[PATCH] D57185: [clang-tidy] Add the abseil-duration-addition check

2019-01-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/abseil/DurationAdditionCheck.cpp:22 +void DurationAdditionCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + binaryOperator(hasOperatorName("+"), This whole file is structurally sim

[PATCH] D57108: [clang-tidy] diagnose possibiltiy to add 'noexcept' in modernize-use-noexcept

2019-01-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 183491. JonasToth added a comment. - rebase to latest patch Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57108/new/ https://reviews.llvm.org/D57108 Files: clang-tidy/bugprone/ExceptionEscapeCheck.cpp cla

[PATCH] D57100: [clang-tidy] refactor bugprone-exception-escape analysis into class

2019-01-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 183490. JonasToth added a comment. - revert doc change as well Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57100/new/ https://reviews.llvm.org/D57100 Files: clang-tidy/bugprone/ExceptionEscapeCheck.cpp

[PATCH] D57100: [clang-tidy] refactor bugprone-exception-escape analysis into class

2019-01-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth marked 2 inline comments as done. JonasToth added a comment. Reverted the changes. Maybe the comma thingie will just stay as is ;) Comment at: clang-tidy/utils/ExceptionAnalyzer.h:25 +/// give the possibility of an exception. +class ExceptionAnalyzer { +public: --

[PATCH] D57100: [clang-tidy] refactor bugprone-exception-escape analysis into class

2019-01-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 183489. JonasToth marked an inline comment as done. JonasToth added a comment. - revert the option configuration change for comma/semicolon Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57100/new/ https://revi

[PATCH] D57108: [clang-tidy] diagnose possibiltiy to add 'noexcept' in modernize-use-noexcept

2019-01-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. In D57108#1369161 , @lebedev.ri wrote: > Hmm, i think this diff is incorrect. Doesn't it also include the changes from > D57100 ? yup, needs constant rebasing, i will do a final rebase once th

[PATCH] D57100: [clang-tidy] refactor bugprone-exception-escape analysis into class

2019-01-24 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth marked 3 inline comments as done. JonasToth added inline comments. Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:35 + std::vector FunctionsThatShouldNotThrowVec = + utils::options::parseStringList(RawFunctionsThatShouldNotThrow); FunctionsThatShouldN

[PATCH] D57108: [clang-tidy] diagnose possibiltiy to add 'noexcept' in modernize-use-noexcept

2019-01-23 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 183136. JonasToth added a comment. - rebase to current refactoring Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57108/new/ https://reviews.llvm.org/D57108 Files: clang-tidy/bugprone/ExceptionEscapeCheck.cp

[PATCH] D57100: [clang-tidy] refactor bugprone-exception-escape analysis into class

2019-01-23 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 183134. JonasToth marked an inline comment as done. JonasToth added a comment. - add license to exceptionanalyzer Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57100/new/ https://reviews.llvm.org/D57100 Files

[PATCH] D57100: [clang-tidy] refactor bugprone-exception-escape analysis into class

2019-01-23 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth marked 3 inline comments as done. JonasToth added inline comments. Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:35 + std::vector FunctionsThatShouldNotThrowVec = + utils::options::parseStringList(RawFunctionsThatShouldNotThrow); FunctionsThatShouldN

[PATCH] D57108: [clang-tidy] diagnose possibiltiy to add 'noexcept' in modernize-use-noexcept

2019-01-23 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth created this revision. JonasToth added reviewers: aaron.ballman, alexfh, hokein. Herald added subscribers: xazax.hun, mgorny. This patch utilizes the refactored `ExceptionAnalyzer` for `modernize-use-noexcept` to diagnose possibilties to introduce `noexcept` if there is no exception spec

[PATCH] D57100: [clang-tidy] refactor bugprone-exception-escape analysis into class

2019-01-23 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 183131. JonasToth added a comment. - [Fix] make class name correct Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57100/new/ https://reviews.llvm.org/D57100 Files: clang-tidy/bugprone/ExceptionEscapeCheck.cp

[PATCH] D57087: [clang-tidy] add OverrideMacro to modernize-use-override check

2019-01-23 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/modernize/UseOverrideCheck.cpp:133 +StringRef Correct = +HasFinal ? "'" + FinalMacro + "'" : "'" + OverrideMacro + "'"; Dangling? These values seem to be temporary and `StringRef` would bind to

[PATCH] D57100: [clang-tidy] refactor bugprone-exception-escape analysis into class

2019-01-23 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 183099. JonasToth removed a subscriber: lebedev.ri. JonasToth added a comment. - adjust doc to say semicolon separated list instead of comma Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57100/new/ https://rev

[PATCH] D57100: [clang-tidy] refactor bugprone-exception-escape analysis into class

2019-01-23 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. In D57100#1367813 , @lebedev.ri wrote: > Two passing-by remarks: > > 1. I would *love* for this check to produce less cryptic reports :) It > currently does not output any details whatsoever. It's really unhelpful. I would do

[PATCH] D56160: [clang-tidy] modernize-use-trailing-return check

2019-01-23 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. In D56160#1367074 , @bernhardmgruber wrote: > Thank you again @JonasToth for all your valueable input! I could almost > successfully run my check on the llvm/lib subfolder. I created a compilation > database from within Visual

[PATCH] D57100: [clang-tidy] refactor bugprone-exception-escape analysis into class

2019-01-23 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth created this revision. JonasToth added reviewers: aaron.ballman, alexfh, hokein, baloghadamsoftware. Herald added subscribers: cfe-commits, rnkovacs, xazax.hun, mgorny. The check `bugprone-exception-escape` does an AST-based analysis to determine if a function might throw an exception an

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

2019-01-23 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. ping :) Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54395/new/ https://reviews.llvm.org/D54395 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/

[PATCH] D56917: [clang] add tests to ExprMutAnalyzer that reproduced a crash in ASTMatchers

2019-01-21 Thread Jonas Toth via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL351743: [clang] add tests to ExprMutAnalyzer that reproduced a crash in ASTMatchers (authored by JonasToth, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGES SINCE LA

[PATCH] D56917: [clang] add tests to ExprMutAnalyzer that reproduced a crash in ASTMatchers

2019-01-21 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 182780. JonasToth added a comment. - use only the small reproducer Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56917/new/ https://reviews.llvm.org/D56917 Files: unittests/Analysis/ExprMutationAnalyzerTest.cpp Index:

[PATCH] D56966: [clang-tidy] misc-non-private-member-variables-in-classes: ignore implicit methods

2019-01-19 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp:22 AST_MATCHER(CXXRecordDecl, hasMethods) { - return std::distance(Node.method_begin(), Node.method_end()) != 0; + for (const auto &Method : Node.methods()) { +if (Meth

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