[PATCH] D130793: [clang-tidy] adjust treating of array-of-pointers when 'AnalyzePointers' is deactivated

2022-08-19 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 454032. JonasToth added a comment. - remove bad change from diff Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130793/new/ https://reviews.llvm.org/D130793 Files: clang-tools-extra/clang-tidy/misc/ConstCor

[PATCH] D132244: [docs] improve documentation for misc-const-correctness

2022-08-19 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 454067. JonasToth added a comment. - mention C limitation Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132244/new/ https://reviews.llvm.org/D132244 Files: clang-tools-extra/docs/clang-tidy/checks/misc/con

[PATCH] D132244: [docs] improve documentation for misc-const-correctness

2022-08-26 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. @njames93 friendly ping, for https://reviews.llvm.org/D130793 as well :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132244/new/ https://reviews.llvm.org/D132244 ___ cfe-comm

[PATCH] D130793: [clang-tidy] adjust treating of array-of-pointers when 'AnalyzePointers' is deactivated

2022-09-17 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth marked an inline comment as done. JonasToth added a comment. ping @njames93 :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130793/new/ https://reviews.llvm.org/D130793 ___ cfe-commits mailing

[PATCH] D130181: [clang-tidy] Add readability-use-early-exits check

2022-09-19 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. In D130181#3769618 , @njames93 wrote: > In D130181#3769083 , @JonasToth > wrote: > >> ... > > Your concerns aren't actually that important. Because the transformations > only work on f

[PATCH] D130793: [clang-tidy] adjust treating of array-of-pointers when 'AnalyzePointers' is deactivated

2022-09-26 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 462838. JonasToth added a comment. - remove unproductive check-fixes line Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130793/new/ https://reviews.llvm.org/D130793 Files: clang-tools-extra/clang-tidy/misc

[PATCH] D130793: [clang-tidy] adjust treating of array-of-pointers when 'AnalyzePointers' is deactivated

2022-09-26 Thread Jonas Toth via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe66345d54d5f: [clang-tidy] adjust treating of array-of-pointers when 'AnalyzePointers' is… (authored by JonasToth). Repository: rG LLVM Github Mon

[PATCH] D54943: [clang-tidy] implement new check 'misc-const-correctness' to add 'const' to unmodified variables

2022-05-29 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 432774. JonasToth added a comment. - addded `CHECK-FIXES` in clang-tidy tests - merged latest main into branch CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54943/new/ https://reviews.llvm.org/D54943 Files: clang-tools-extra/clang-tidy/misc/CMa

[PATCH] D127036: [clang-tidy] Warn about arrays in `bugprone-undefined-memory-manipulation`

2022-06-04 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/UndefinedMemoryManipulationCheck.cpp:27 void UndefinedMemoryManipulationCheck::registerMatchers(MatchFinder *Finder) { - const auto NotTriviallyCopyableObject = - hasType(ast_matchers::hasC

[PATCH] D54943: [clang-tidy] implement new check 'misc-const-correctness' to add 'const' to unmodified variables

2022-06-07 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. ping :) @njames93 I added more `CHECK-FIXES` and `CHECK-FIXES-NOT` statements in the tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54943/new/ https://reviews.llvm.org/D54943 ___ cfe-commits mailing list cfe

[PATCH] D54943: [clang-tidy] implement new check 'misc-const-correctness' to add 'const' to unmodified variables

2022-06-08 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.h:35 +TransformPointersAsValues( +Options.get("TransformPointersAsValues", false)) {} + njames93 wrote: > It may be worth adding some validati

[PATCH] D69780: [clang-format] DO NOT COMMIT - Demo of East/West Const Fixer on clang-format

2019-11-04 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. Yeah! nice work :) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69780/new/ https://reviews.llvm.org/D69780 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

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

2019-11-13 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! Did you check on a real code-base that suffers from the issue, if that works as expected? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D70144: clang-tidy: modernize-use-equals-default avoid adding redundant semicolons

2019-11-13 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. Please mention this improvement in the release notes as well :) Comment at: clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp:306 + if (ApplyFix) { +// Peek ahead to see if there's a semicolon after Body->getSourceRange() +Opti

[PATCH] D70144: clang-tidy: modernize-use-equals-default avoid adding redundant semicolons

2019-11-13 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp:306 + if (ApplyFix) { +// Peek ahead to see if there's a semicolon after Body->getSourceRange() +Optional Token; poelmanc wrote: > JonasToth wro

[PATCH] D70144: clang-tidy: modernize-use-equals-default avoid adding redundant semicolons

2019-11-13 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. Are you using the lexer-util functions? Maybe i am just to tired, but they seem to be unused. Comment at: clang/include/clang/Lex/Token.h:101 + /// Single argument overload provides base case for recursive template below + bool isOneOf(tok::TokenKi

[PATCH] D70144: clang-tidy: modernize-use-equals-default avoid adding redundant semicolons

2019-11-14 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. In D70144#1745532 , @malcolm.parsons wrote: > Should `clang::format::cleanupAroundReplacements()` handle this instead? Of yes. Totally forgot that. That would probably the most elegant way :) Repository: rCTE Clang Tools E

[PATCH] D70144: clang-tidy: modernize-use-equals-default avoid adding redundant semicolons

2019-11-14 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. In D70144#1745881 , @poelmanc wrote: > In D70144#1745583 , @JonasToth wrote: > > > In D70144#1745532 , > > @malcolm.parsons wrote: > > > > > Shoul

[PATCH] D70265: [clang-tidy] Add clang tidy check I.2 to cppcoreguidelines

2019-11-14 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp:20 +namespace { +AST_MATCHER(VarDecl, isLocalVarDecl) { return Node.isLocalVarDecl(); } +} // namespace This matcher already exists

[PATCH] D70265: [clang-tidy] Add clang tidy check I.2 to cppcoreguidelines

2019-11-14 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. Hi vingeldal, thanks for you contribution and welcome to LLVM :) If you have any questions or the like, feel free to ask! Best Regards, Jonas Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70265/new/ https://reviews.llvm

[PATCH] D70144: clang-tidy: modernize-use-equals-default avoid adding redundant semicolons

2019-11-16 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. Hmm. I think this is fine, even though its not perfect. @aaron.ballman wdyt? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70144/new/ https://reviews.llvm.org/D70144 ___ cfe-c

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

2019-11-16 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 229705. JonasToth marked 2 inline comments as done. JonasToth removed a subscriber: mgehre. JonasToth added a comment. - [Misc] port patch to monorepo - add more test cases for the transformation Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

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

2019-11-16 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth marked 5 inline comments as done. JonasToth added inline comments. Herald added a subscriber: mgehre. Comment at: clang-tidy/utils/FixItHintUtils.cpp:35 +static bool isValueType(QualType QT) { return isValueType(QT.getTypePtr()); } +static bool isArrayType(QualType QT)

[PATCH] D70052: [clang-tidy] Add misc-mutating-copy check

2019-11-18 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. There is a `ExprMutAnalyzer` that is able to find mutation of expressions in general (even though it is kinda experimental still). Maybe that should be utilized somehow? I think the current implementation does not cover when the address is taken and mutation happens t

[PATCH] D45444: [clang-tidy] implement new check for const-correctness

2019-11-18 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. Herald added a subscriber: mgehre. In D45444#1685995 , @tsdgeos wrote: > Would this warn with stuff like > > double borderWidth; > [... code that doesn't use borderWidth ...] > borderWidth = border->getWidth(); > [... co

[PATCH] D45444: [clang-tidy] implement new check for const-correctness

2019-11-18 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 229818. JonasToth added a comment. - update license - rebase to master and patch for adding const Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D45444/new/ https://reviews.llvm.org/D45444 Files: clang-tools

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

2019-11-18 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/performance-no-automatic-move.rst:47 +std::vector&& obj = ...; +return std::move(obj); // calls StatusOr::StatusOr(std::vector&&) + } While checking this example it s

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

2019-11-18 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/performance-no-automatic-move.rst:47 +std::vector&& obj = ...; +return std::move(obj); // calls StatusOr::StatusOr(std::vector&&) + } lebedev.ri wrote: > courbet wrot

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

2019-11-19 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. IMHO these two should just not overlap. It makes sense, to have controversial or configurable stuff in clang-tidy. It should just be consistent with the warnings, as those are "always right" and clang-tidy can be opinionated/specialized. Repository: rG LLVM Github

[PATCH] D69764: [clang-format] Add Left/Right Const (East/West , Before/After) fixer capability

2019-11-19 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang/include/clang/Format/Format.h:1110 + /// Different const alignment styles. + enum ConstAlignmentStyle { +/// Don't change const to either East const or West const. Drive-By question: https://reviews.llvm.or

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

2019-11-19 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. In D70390#1751159 , @courbet wrote: > > IMHO these two should just not overlap. It makes sense, to have > > controversial or configurable stuff in clang-tidy. It should just be > > consistent with the warnings, as those are "al

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

2019-11-19 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. I know it has been a long time, but this patch is ready, i think :) i would appreciate some reviews ;) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54395/new/ https://reviews.llvm.org/D54395 __

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

2019-11-19 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 230048. JonasToth added a comment. - port patch to monorepo - Merge branch 'master' into feature_transform_const.patch - merge current master and new version for previous steps into this patch - fix compilation issues after type renaming - work on fixing sup

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

2019-11-20 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp:28 +void NoAutomaticMoveCheck::registerMatchers(MatchFinder *finder) { + const auto const_local_variable = + varDecl(hasLocalStorage(), unless(hasType(lValueRefere

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

2019-11-21 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 after the variable names are adjusted Comment at: clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp:32 + + const auto const_local_variable = +

[PATCH] D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros

2019-11-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. In D31130#1758453 , @fiesh wrote: > In D31130#1711841 , @lebedev.ri > wrote: > > > The description only says what the patch does, not why this is the desired > > behavior. > > Also, thi

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

2019-11-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. Maybe a short notice in the release notes wouldn't hurt. Otherwise LGTM *EDIT*: Aaron commented as well. Plz let him react before committing :) CHANGES SINCE LAST ACTION https://revie

[PATCH] D45444: [clang-tidy] implement new check for const-correctness

2019-12-03 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. In D45444#1766156 , @0x8000- wrote: > Thank you for rebasing on current master. > > I have ran it today on our code base and found three classes of false > positives: > > 1. Writing to a bitfield of a struct. The struct stil

[PATCH] D45444: [clang-tidy] implement new check for const-correctness

2019-12-05 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. > Are you going to keep updating it on > https://github.com/JonasToth/llvm-project/commits/feature_check_const ? Yup, thats my backup if something goes wrong and to synchronize my pcs. :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revi

[PATCH] D71607: [clang-tidy] Add unsigned subtraction warning, with suggestion to convert to unsigned literals.

2019-12-18 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. Somewhat related: https://reviews.llvm.org/D40854 This is a check that tried to enforce not mixing any signed/unsigned arithmetic. there was no feedback from the cppcoreguideline-ppl on how to proceed with edge cases and occassion where mixing is not avoidable (e.g. `

[PATCH] D70265: [clang-tidy] Add clang tidy check I.2 to cppcoreguidelines

2019-12-21 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp:26 + hasType(isConstQualified()), + hasType(referenceType()), // References can't be changed, only data +

[PATCH] D71980: [clang-tidy] Disable Checks on If constexpr statements in template Instantiations for BugproneBranchClone and ReadabilityBracesAroundStatements

2020-01-19 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. In D71980#1810086 , @njames93 wrote: > Is this good to land and if so anyone able to commit? I marked this patch for porting into the release. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D73052: [clang-tidy] RenamerClangTidy now renames dependent member expr when the member can be resolved

2020-01-20 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:282 +: DepMemberRef->getBaseType(); +CXXRecordDecl *Base = BaseType.getTypePtr()->getAsCXXRecordDecl(); +if (!Base) can `Base

[PATCH] D72553: [clang-tidy] Add performance-prefer-preincrement check

2020-01-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. Yeah, the libc++ failures are not your fault :) My 2 cents added. Comment at: clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.cpp:63 + + if (!getLangOpts().CPlusPlus || !TransformCxxOpCalls) +return; Why would y

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

2020-01-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. We need tests for that. There should be tests for the exporting-feature. Maybe they could provide a good starting point on adding tests. (not sure if there is unit-test code for isolated testing, but some tests do exist!) Repository: rG LLVM Github Monorepo CHANGE

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

2020-01-30 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. In D54943#1841086 , @AlexanderLanin wrote: > In D54943#1815772 , @0x8000- > wrote: > > > This generated 56 "const const" declarations, of which 25 were triple const! > This could b

[PATCH] D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test

2018-03-21 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 139293. JonasToth marked 4 inline comments as done. JonasToth added a comment. - adress review comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40737 Files: clang-tidy/hicpp/CMakeLists.txt clang-tidy/hicpp/HICPPTidyModule.cpp

[PATCH] D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test

2018-03-21 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:119 + if (!SwitchHasDefault && SwitchCaseCount == 0) { +diag(Switch->getLocStart(), "degenerated switch without labels"); +return; aaron.ballman wrote: > I think

[PATCH] D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test

2018-03-21 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 139304. JonasToth marked 3 inline comments as done. JonasToth added a comment. - add fixme for degenerated switch Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40737 Files: clang-tidy/hicpp/CMakeLists.txt clang-tidy/hicpp/HICPPTidyMo

[PATCH] D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test

2018-03-21 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:119 + if (!SwitchHasDefault && SwitchCaseCount == 0) { +diag(Switch->getLocStart(), "degenerated switch without labels"); +return; aaron.ballman wrote: > JonasTot

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

2018-03-22 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: test/clang-tidy/readability-function-size.cpp:207-212 +void variables_8() { + int a, b; + struct A { +A(int c, int d); + }; +} lebedev.ri wrote: > aaron.ballman wrote: > > lebedev.ri wrote: > > > aaron.ballman w

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

2018-03-22 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. I think this check could land in the `bugprone` module. Given this situation won't appear a lot in codebases, did you check other codebases than LLVM? Comment at: clang-tidy/misc/TerminatingContinueCheck.h:19 + +/// Checks if a 'continue' statement

[PATCH] D40854: [clang-tidy] WIP implement cppcoreguidelines check for mixed integer arithmetic

2018-03-24 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 139706. JonasToth added a comment. - update to trunk - [Doc] update to new doc style Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40854 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreguidelines/CppCoreGuideline

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

2018-03-24 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 139707. JonasToth added a comment. - Merge branch 'master' into check_macros_usage - [Doc] update to new doc style Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41648 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppc

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

2018-03-24 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: test/clang-tidy/readability-function-size.cpp:207-212 +void variables_8() { + int a, b; + struct A { +A(int c, int d); + }; +} lebedev.ri wrote: > aaron.ballman wrote: > > lebedev.ri wrote: > > > aaron.ballman w

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

2018-03-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. I checked several codebases and implemented a little helper script to see which kind of macros exist. Then I added some filter regex as configuration and tried again, here are the results: For https://github.com/nlohmann/json which is a very high quality codebase, the

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

2018-03-26 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 139793. JonasToth added a comment. - [Feature] implement CAPS_ONLY features, adjust docs Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41648 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreguidelines/CppCoreGuide

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

2018-03-26 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. Which checks do you have in mind? Am 26.03.2018 um 18:07 schrieb Eugene Zelenko via Phabricator: > Eugene.Zelenko added inline comments. > > > Comment at: docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst:24 > + > +.. option:: CheckCapsOnly

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

2018-03-27 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst:24 + +.. option:: CheckCapsOnly + Eugene.Zelenko wrote: > Will be good idea to generalize this option with other check which deal with > macros. I dont think th

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

2018-03-27 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: test/clang-tidy/misc-terminating-continue.cpp:7 +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: terminating 'continue' [misc-terminating-continue] + } while(false); + Quuxplusone wrote: > I initially expected to see

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

2018-03-30 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. You noted, that the C++ warning would catch this case, but does not get triggered in C. Would it be wort to generalize the concern and have a warning that catch the comparison, regardless of the macro? Do other major libraries define a similar macro but name it differ

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

2018-03-30 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. > I'm not quite sure how we would go about that with confidence. The code we'd > need to catch looks something like: > > typeof(foo() == y) x; > /* snip */ > bar(x == -1); // warning: comparison is pointless > > If we could tell that x's type was inferred from a com

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

2018-03-31 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. Alright. Thank you for clarification :) Comment at: clang-tidy/bugprone/ComparisonInTempFailureRetryCheck.cpp:75 + const auto &Assign = *Result.Nodes.getNodeAs("assign"); + const auto &RHS = *cast(Assign.getRHS()->IgnoreParenCasts()); + assert(RHS.

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

2018-04-02 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. LGTM, but @aaron.ballman, @alexfh or someone else should review it before comitting. https://reviews.llvm.org/D45059 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/c

[PATCH] D45414: [clang-tidy] add missing assignment operations in hicpp-signed-bitwise

2018-04-08 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth created this revision. JonasToth added reviewers: aaron.ballman, hokein, alexfh. Herald added subscribers: cfe-commits, xazax.hun, klimek. This patch resolves the bug https://bugs.llvm.org/show_bug.cgi?id=36963. - implement missing assignment operators for `hicpp-signed-bitwise` - add t

[PATCH] D45414: [clang-tidy] add missing assignment operations in hicpp-signed-bitwise

2018-04-08 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 141547. JonasToth added a comment. - fix std bitmask type shifting issue Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45414 Files: clang-tidy/hicpp/SignedBitwiseCheck.cpp docs/ReleaseNotes.rst test/clang-tidy/hicpp-signed-bitwise-

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

2018-04-09 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. Hi, my 2 cents: - On which codebases did you run the check? - did you consider looking for `implicitCastExpr`? You can capture all narrowing conversion with that and analyze them further. I think it is possible to warn for the subset mentioned in the guidelines. - yo

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

2018-04-09 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. That sounds good. > Removing from my dashboard for now. @aaron.ballman seems to be busy now, maybe you should add alexfh again and discuss the results. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D38455 _

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

2018-04-09 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. > Is there anything I need to do for the diff to change state ? I thought updating the code would automatically mark it "ready for review" again. I think all comments must be marked as done to be ready for review again. Usually the reviewer reacts to changed code, too.

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

2018-04-09 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:32 + Finder->addMatcher( + implicitCastExpr(hasImplicitDestinationType(isInteger()), + hasSourceExpression(IsFloatExpr), Do you pla

[PATCH] D27621: [clang-tidy] check to find declarations declaring more than one name

2018-04-09 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. Still work on this one? https://reviews.llvm.org/D27621 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2018-04-09 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:34 + hasSourceExpression(IsFloatExpr), + unless(hasParent(castExpr(.bind("cast"), + this); courbet wrote:

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

2018-04-09 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. Could you please add some tests that include user defined literals and there interaction with other literals. We should catch narrowing conversions from them, too. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D38455 __

[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness

2018-04-09 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth created this revision. JonasToth added reviewers: alexfh, aaron.ballman, lebedev.ri, hokein. Herald added subscribers: cfe-commits, kbarton, xazax.hun, mgorny, nemanjai, klimek. This check aims to determine values and references that could be declared as const, but are not. The first v

[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness

2018-04-09 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. @aaron.ballman, @lebedev.ri, @alexfh and everyone else, i would love to hear your opinion on my approach. i never implemented a similar check and i might miss some cases, that need to be considered. The current state is just a proof-of-concept to see how such a check

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

2018-04-10 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. > Sure. Is it OK to add a dependency from > clang-tidy/bugprone/BugproneTidyModule.cpp to stuff in > clang-tidy/cpp-coreguidelines ? Is there an existing example of this ? Take a look in the hicpp module, almost everything there is aliased :) Repository: rCTE Clan

[PATCH] D45414: [clang-tidy] add missing assignment operations in hicpp-signed-bitwise

2018-04-10 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/hicpp/SignedBitwiseCheck.cpp:92 + if (N.getNodeAs("std_type")) +diag(Location, "shifting a value of the standardized bitmask types"); + else aaron.ballman wrote: > How about: "shifting a value of bitma

[PATCH] D45414: [clang-tidy] add missing assignment operations in hicpp-signed-bitwise

2018-04-10 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/hicpp/SignedBitwiseCheck.cpp:92 + if (N.getNodeAs("std_type")) +diag(Location, "shifting a value of the standardized bitmask types"); + else aaron.ballman wrote: > JonasToth wrote: > > aaron.ballman wr

[PATCH] D45468: [clang-tidy] Adding Fuchsia checker for human-readable logging

2018-04-10 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/zircon/HumanReadableStatusCheck.cpp:68 +SourceRange FmtStringRange; +for (const auto *Arg : D->arguments()) { + if (const auto *ICE = dyn_cast(Arg)) { Did you consider using `forEachArgumentWith

[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness

2018-04-10 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. > It'll be good idea to have option to apply this check for pointer/references > only, or include built-in types/enums. Agreed. I aim at a `mark handles const`(default on), `mark values const`(default on for objects), `mark pointer (const int * >> const <<) const` (d

[PATCH] D45414: [clang-tidy] add missing assignment operations in hicpp-signed-bitwise

2018-04-11 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 141970. JonasToth added a comment. - remove shifting exception for standarized bitmask types Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45414 Files: clang-tidy/hicpp/SignedBitwiseCheck.cpp docs/ReleaseNotes.rst test/clang-tidy/h

[PATCH] D45414: [clang-tidy] add missing assignment operations in hicpp-signed-bitwise

2018-04-11 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth closed this revision. JonasToth added a comment. Commit in https://reviews.llvm.org/rCTE329789 Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45414 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.

[PATCH] D39367: [clang-tidy] Add support for operator new[] in check bugprone-misplaced-operator-in-strlen-in-alloc

2017-10-27 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.cpp:62 + const Expr *Alloc = Result.Nodes.getNodeAs("Alloc"); + if (!Alloc) Alloc = Result.Nodes.getNodeAs("Alloc"); const auto *StrLen = Result.Nodes.getNodeAs("StrLen");

[PATCH] D39367: [clang-tidy] Add support for operator new[] in check bugprone-misplaced-operator-in-strlen-in-alloc

2017-10-27 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. Should the release notes be modified as well? Repository: rL LLVM https://reviews.llvm.org/D39367 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

2017-10-31 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 121009. JonasToth added a comment. - improved docs and comments - remove some newlines - use ternary operator instead of smallvec https://reviews.llvm.org/D37808 Files: clang-tidy/hicpp/CMakeLists.txt clang-tidy/hicpp/HICPPTidyModule.cpp clang-tidy/

[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

2017-10-31 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 121011. JonasToth added a comment. - fix rebasing issues in docs https://reviews.llvm.org/D37808 Files: clang-tidy/hicpp/CMakeLists.txt clang-tidy/hicpp/HICPPTidyModule.cpp clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp clang-tidy/hicpp/MultiwayPa

[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

2017-10-31 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. @aaron.ballman could you take a (i think) finishing look on the check? Most issues should be resolved and i think its ready for the finishing line :) https://reviews.llvm.org/D37808 ___ cfe-commits mailing list cfe-commit

[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

2017-11-01 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 121134. JonasToth added a comment. - remove double whitespace https://reviews.llvm.org/D37808 Files: clang-tidy/hicpp/CMakeLists.txt clang-tidy/hicpp/HICPPTidyModule.cpp clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp clang-tidy/hicpp/MultiwayPaths

[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

2017-11-08 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. @aaron.ballman and @alexfh ping. https://reviews.llvm.org/D37808 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

2017-11-17 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 123345. JonasToth marked 18 inline comments as done. JonasToth added a comment. - rebase to master - address review comments from aaron - handle bools correctly https://reviews.llvm.org/D37808 Files: clang-tidy/hicpp/CMakeLists.txt clang-tidy/hicpp/HI

[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

2017-11-17 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:13 + +#include +#include aaron.ballman wrote: > Including is forbidden by our coding standard. However, it doesn't > appear to be used in the source file, either. Yes,

[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

2017-11-17 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 123347. JonasToth marked 12 inline comments as done. JonasToth added a comment. - address more comments, especially doc https://reviews.llvm.org/D37808 Files: clang-tidy/hicpp/CMakeLists.txt clang-tidy/hicpp/HICPPTidyModule.cpp clang-tidy/hicpp/Mult

[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

2017-11-17 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 123349. JonasToth added a comment. - - fix ReleaseNotes https://reviews.llvm.org/D37808 Files: clang-tidy/hicpp/CMakeLists.txt clang-tidy/hicpp/HICPPTidyModule.cpp clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp clang-tidy/hicpp/MultiwayPathsCovere

[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

2017-11-18 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 123468. JonasToth marked 3 inline comments as done. JonasToth added a comment. - fix nits https://reviews.llvm.org/D37808 Files: clang-tidy/hicpp/CMakeLists.txt clang-tidy/hicpp/HICPPTidyModule.cpp clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp cl

[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

2017-11-18 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. finalize https://reviews.llvm.org/D37808 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

2017-11-18 Thread Jonas Toth via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL318600: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches (authored by JonasToth). Repository: rL LLVM https://reviews.llvm.org/D37808 Files: clang-tools-extra/trunk/clang

[PATCH] D39027: [docs][refactor] Add a new tutorial that talks about how one can implement refactoring actions

2017-11-18 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: docs/RefactoringActionTutorial.rst:236 + class NestedIfSelectionRequirement final + : final CodeRangeSelectionRequirement { + public: i think the `final` after the colon should be `public` instead.

[PATCH] D39027: [docs][refactor] Add a new tutorial that talks about how one can implement refactoring actions

2017-11-19 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: docs/RefactoringActionTutorial.rst:98-99 + + #include "clang/Refactoring/RefactoringActions.h" + #include "clang/Refactoring/RefactoringActionRules.h" + The includes are missing `Tooling` -> `clang/Tooling/Re

[PATCH] D30896: [Clang-tidy] add check misc-prefer-switch-for-enums

2017-03-15 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. @jbcoe and @aaron.ballman i think an valid warning would be if there is another check in an `else if` statement. so singular `if(enum_value == Enum::Kind)` is still ok, but another branch checking on the enum is suspiscious. Repository: rL LLVM https://reviews.ll

[PATCH] D31128: Rename the safety module to be hicpp

2017-03-19 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth accepted this revision. JonasToth added a comment. thank you very much for dealing with this issue :) I will update the aliases. But maybe later this week since i have an exam. :) https://reviews.llvm.org/D31128 ___ cfe-commits mailing lis

<    8   9   10   11   12   13   14   15   >