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

JonasToth marked an inline comment as done. JonasToth added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp:608 +} + +template 0x8000- wrote: > JonasToth wrote: > > 0x8000- wrote: >

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

JonasToth marked an inline comment as done. JonasToth added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp:608 +} + +template 0x8000- wrote: > 0x8000- wrote: > > JonasToth wrote: >

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

JonasToth added a comment. In D54943#1804183 , @0x8000- wrote: > This patch does not build on top of current tree: > > /usr/bin/ld: > lib/libclangTidyCppCoreGuidelinesModule.a(ConstCorrectnessCheck.cpp.o): in > function >

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

JonasToth marked 2 inline comments as done. JonasToth added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp:608 +} + +template JonasToth wrote: > 0x8000- wrote: > > Please insert the

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

JonasToth marked 2 inline comments as done. JonasToth added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp:608 +} + +template 0x8000- wrote: > Please insert the this test code here: >

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

JonasToth added a comment. In D54395#1803552 , @thakis wrote: > This breaks tests on Windows: http://45.33.8.238/win/5061/step_8.txt > > Please take a look, and if takes a while to investigate, please revert in the > meantime. (Probably needs the

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

JonasToth marked 2 inline comments as done. JonasToth added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:5224 +/// Matches QualType nodes that are of function pointer type. +/// I will remove those later. They are not used.

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

JonasToth updated this revision to Diff 236101. JonasToth added a comment. - ignore implicit casts in memberExpr(hasObjectExpression()) - implement functionality for function pointers Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54943/new/

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

JonasToth added a comment. In D54943#1800182 , @0x8000- wrote: > F11163406: 0001-Add-extra-tests.patch > shows a couple of false positives. I added your tests, but some did not show false positives anymore. I

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

This revision was automatically updated to reflect the committed changes. Closed by commit rGcf48101200ee: [clang-tidy] implement utility-function to add const to variables (authored by JonasToth). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

JonasToth added a comment. Thank you for all the review over the time this took to land! :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54395/new/ https://reviews.llvm.org/D54395 ___ cfe-commits

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

JonasToth marked an inline comment as done. JonasToth added inline comments. Comment at: clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp:1055 + EXPECT_NE(runCheckOnCode(Cat(S), nullptr, "input.m"), +Cat("Object const*target;")); +

[PATCH] D72121: [clang-tidy] Fix readability-identifier-naming missing member variables

JonasToth added a comment. In D72121#1803242 , @njames93 wrote: > Ahh clang format script must have run, I'll undo that. I've not tried running > it against the whole of llvm as I don't know what the easy way to do that is, > especially given that llvm

[PATCH] D72121: [clang-tidy] Fix readability-identifier-naming missing member variables

JonasToth added a comment. Thank you for fixing these bugs! Did you try running the check of clang/llvm again, to see if there are still missing cases of this? Comment at: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:575 -if (Decl->isFileVarDecl()

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

JonasToth marked 6 inline comments as done. JonasToth added inline comments. Comment at: clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp:1006 +} + +} // namespace test aaron.ballman wrote: > JonasToth wrote: > > aaron.ballman wrote: > > > Can you also

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

JonasToth updated this revision to Diff 235938. JonasToth added a comment. - add proper objc test, that fail mostly Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54395/new/ https://reviews.llvm.org/D54395 Files:

[PATCH] D71973: [clang-tidy] Add bugprone-suspicious-memory-comparison check

JonasToth added a comment. LG from my side, besides the small nits. Please let @aaron.ballman have a look as well. thanks for the patch :) Comment at: clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.cpp:45 + + // check if comparing padding in base

[PATCH] D71973: [clang-tidy] Add bugprone-suspicious-memory-comparison check

JonasToth added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.cpp:1 +// RUN: %check_clang_tidy %s bugprone-suspicious-memory-comparison %t + gbencze wrote: > JonasToth wrote: > > We should

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

JonasToth marked an inline comment as done. JonasToth added inline comments. Comment at: clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp:66 + StringRef S = "MyInt target = 0;"; + auto Cat = [](StringRef S) { return (T + S).str(); }; + aaron.ballman

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

JonasToth updated this revision to Diff 235838. JonasToth marked 3 inline comments as done. JonasToth added a comment. - address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54395/new/ https://reviews.llvm.org/D54395 Files:

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

JonasToth updated this revision to Diff 235705. JonasToth added a comment. - clarify FIXME problem Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54395/new/ https://reviews.llvm.org/D54395 Files:

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

JonasToth updated this revision to Diff 235704. JonasToth added a comment. - adjust skipLParens code slightly to remove redundant checks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54395/new/ https://reviews.llvm.org/D54395 Files:

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

JonasToth updated this revision to Diff 235703. JonasToth added a comment. - improve test situation and fix a crash coming from invalid source locations Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54395/new/ https://reviews.llvm.org/D54395

[PATCH] D36051: Move from a long list of checkers to tables

JonasToth added a comment. In D36051#1799785 , @sylvestre.ledru wrote: > @malcolm.parsons really? Oh, where is it implemented? > >

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

JonasToth updated this revision to Diff 235611. JonasToth added a comment. - fix brace Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54395/new/ https://reviews.llvm.org/D54395 Files:

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

JonasToth updated this revision to Diff 235610. JonasToth marked 14 inline comments as done. JonasToth added a comment. - create fresh branch with latest diff of the revision - fix review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

JonasToth added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp:34-41 +static bool isArrayType(QualType QT) { return isa(QT.getTypePtr()); } +static bool isReferenceType(QualType QT) { + return isa(QT.getTypePtr()); +} +static bool

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

JonasToth added a comment. There are many comments not marked as done. could you please do that with addressed issues? It is hard to otherwise see whats still left. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp:110 +

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

JonasToth abandoned this revision. JonasToth added a comment. Herald added a subscriber: whisperity. Abonding this revision in favor of: https://reviews.llvm.org/D54943 It doesn't make sense to have both open at the same time, as the const-transformation happens in its own patch and no heavy

[PATCH] D61989: [clang-tidy] enable modernize-concat-nested-namespaces on header files

JonasToth added a comment. Sorry for not responding to the review. I was very busy with other things and overlooked the mails :( Comment at: clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:82 ast_matchers::MatchFinder *Finder) { - if

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

JonasToth added a comment. Herald added subscribers: wuzish, whisperity. You need tests for this check. Please harden them against macro-interference as well. I think the transformation should happen as function call that return `Optional` and if any failure happens the diagnostic can still be

[PATCH] D65912: [clang-tidy] Add new check for math constants

JonasToth added a comment. Herald added a subscriber: whisperity. In D65912#1623375 , @ZaMaZaN4iK wrote: > > ... then I have a script that runs clang-tidy over all the compilation > > units in a compilation database > > Can you share this script please?

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

JonasToth added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp:110 + "making it const") + << MemberVariable; // FIXME: Add fix-it hint to MemberVariable +} missing

[PATCH] D71973: [clang-tidy] Add bugprone-suspicious-memory-comparison check

JonasToth added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.cpp:19 + +static constexpr llvm::StringLiteral CallExprName = "call"; + I think you can remove the constant and hardcode it in the matcher. that

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

JonasToth added a comment. Thank you for working on this! In D71980#1798680 , @njames93 wrote: > In D71980#1798517 , @lebedev.ri > wrote: > > > Missing tests; please upload patches with full context (`-U9`);

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

JonasToth added a comment. In D57662#1797607 , @derek wrote: > @zinovy.nis @alexfh @JonasToth @MyDeveloperDay > > Hi folks, please correct me if I'm wrong but it appears that an effect of > this change is that this script will no longer exit non-zero if

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

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] D71607: [clang-tidy] Add unsigned subtraction warning, with suggestion to convert to unsigned literals.

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] D45444: [clang-tidy] implement new check for const-correctness

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

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

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

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

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

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

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,

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

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] D70390: [clang-tidy] new performance-no-automatic-move check.

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(),

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

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

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

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] D70390: [clang-tidy] new performance-no-automatic-move check.

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

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

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:

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

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

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

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

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

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

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

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:

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

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(); > [...

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

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

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

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

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

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] D70144: clang-tidy: modernize-use-equals-default avoid adding redundant semicolons

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 ___

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

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/

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

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] D70144: clang-tidy: modernize-use-equals-default avoid adding redundant semicolons

JonasToth added a comment. In D70144#1745881 , @poelmanc wrote: > In D70144#1745583 , @JonasToth wrote: > > > In D70144#1745532 , > > @malcolm.parsons wrote: > > > > >

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

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

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

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

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

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

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

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() +

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

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

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

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

[PATCH] D68694: [clang-tidy] hicpp-signed-bitwise: Do not show "use of a signed integer operand with a binary bitwise operator" for positive integer operands

JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land. LGTM. But i was inactive for a long time, if @aaron.ballman accepts as well you can commit instantly. Otherwise please let the other people that commented some time to react. :)

[PATCH] D68694: [clang-tidy] hicpp-signed-bitwise: Do not show "use of a signed integer operand with a binary bitwise operator" for positive integer operands

JonasToth added a comment. > Do you know who is responsible for it? Because i haven't worked with > documentation before and don't know what i need to do to update it. The files reside in `clang-tools-extra/docs/ReleaseNotes.rst` and

[PATCH] D68694: [clang-tidy] hicpp-signed-bitwise: Do not show "use of a signed integer operand with a binary bitwise operator" for positive integer operands

JonasToth added a comment. The new switch needs documentation as well, and maybe even a note in the release notes (as it is publicly discussed as issue?). Otherwise I am fine with the changes! Comment at:

[PATCH] D68694: [clang-tidy] hicpp-signed-bitwise: Do not show "use of a signed integer operand with a binary bitwise operator" for positive integer operands

JonasToth added a comment. In D68694#1701772 , @aaron.ballman wrote: > > In my opinion this check should be disabled in case of integer literals, > > since there are a lot of existing code (even in system libraries) where > > user can do nothing, e.g.:

[PATCH] D56661: [clang-tidy] Fix incorrect array name generation in cppcoreguidelines-pro-bounds-constant-array-index

JonasToth added a comment. Sorry for the long delay! I have one question about a slighty different code constellation. std::optional> my_array; for(int i = 0; i < 42; ++i) (*my_array)[i] = 42; Will this be fixed properly? for(int i = 0; i < 42; ++i) gsl::at(*my_array, i)

[PATCH] D64671: [clang-tidy] New check: misc-init-local-variables

JonasToth added a comment. In D64671#1606084 , @jpakkane wrote: > In D64671#1603427 , @alexfh wrote: > > > A general comment: "misc" is a sort of a heap of checks that otherwise > > don't have a good home. This

[PATCH] D64448: gsl::Owner/gsl::Pointer: Add implicit annotations for some std types

JonasToth added a comment. Just as Sidenote in case you were not aware: There is a check in clang-tidy for `gsl::owner` (https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-owning-memory.html) It will benefit from your annotation and can be simplified. Repository: rG LLVM

[PATCH] D61989: [clang-tidy] enable modernize-concat-nested-namespaces on header files

JonasToth added a comment. Could you please run this check over LLVM and give a short report of your finding? I would imagine that there is a lot of duplication, given the include-heavy nature of big c++ code bases. Comment at:

[PATCH] D62125: Run ClangTidy tests in all C++ language modes

JonasToth added a comment. Wow, a lot of work! I did not check all test files, but I saw that you explicitly enabled c++11,14,17 but not 98. Is there a reason for that? I think we should test that standard too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D61989: [clang-tidy] enable modernize-concat-nested-namespaces on header files

JonasToth added a comment. Hi shixiao, thank you for taking a look at this. Please extend the unit-tests (clang-tools-extra/test/clang-tidy) for this check, your revision description already includes a good start. Comment at:

[PATCH] D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403)

JonasToth added a comment. Given this revision is accepted and seems ready to go: Do you need someone to commit on your behalf? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55793/new/ https://reviews.llvm.org/D55793 ___ cfe-commits

[PATCH] D61850: Removed superfluous and slightly annoying newlines in run-clang-tidy's output.

JonasToth added a comment. Thans for the patch! Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61850/new/ https://reviews.llvm.org/D61850 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D61850: Removed superfluous and slightly annoying newlines in run-clang-tidy's output.

This revision was automatically updated to reflect the committed changes. Closed by commit rL360883: [clang-tidy] Removed superfluous and slightly annoying newlines in run-clang… (authored by JonasToth, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits.

[PATCH] D56323: [clang-tidy] Handle member variables in readability-simplify-boolean-expr

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE360882: [clang-tidy] Handle member variables in readability-simplify-boolean-expr (authored by JonasToth, committed by ). Herald added a project: clang. Repository: rCTE Clang Tools Extra CHANGES

[PATCH] D56323: [clang-tidy] Handle member variables in readability-simplify-boolean-expr

JonasToth added a comment. Thank you for the patch and sorry for the long delay. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56323/new/ https://reviews.llvm.org/D56323 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D61850: Removed superfluous and slightly annoying newlines in run-clang-tidy's output.

JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land. LGTM then, i can commit for you again? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61850/new/ https://reviews.llvm.org/D61850

[PATCH] D61849: Do not list enabled checks when -quiet is given to run-clang-tidy.

JonasToth added a comment. Yes someone has to do it on behalf of you. I did so for you. After a view good patches you can get commit rights from chris lattner, until then the reviewer can commit for you. Thank you for the patch! Repository: rL LLVM CHANGES SINCE LAST ACTION

[PATCH] D61849: Do not list enabled checks when -quiet is given to run-clang-tidy.

This revision was automatically updated to reflect the committed changes. Closed by commit rL360869: [clang-tidy] Do not list enabled checks when -quiet is given to run-clang-tidy. (authored by JonasToth, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits.

[PATCH] D61850: Removed superfluous and slightly annoying newlines in run-clang-tidy's output.

JonasToth added a comment. In D61850#1500330 , @svenpanne wrote: > In D61850#1500298 , @JonasToth wrote: > > > [...] please ensure that e.g. clang-analyzer-* output looks proper as well > > and `-quiet` does, too.

[PATCH] D61850: Removed superfluous and slightly annoying newlines in run-clang-tidy's output.

JonasToth added a comment. I agree with the direction, please ensure that e.g. clang-analyzer-* output looks proper as well and `-quiet` does, too. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61850/new/ https://reviews.llvm.org/D61850

[PATCH] D61749: [clang-tidy] initial version of readability-static-const-method

JonasToth added inline comments. Comment at: clang-tools-extra/test/clang-tidy/readability-static-const-method.cpp:209 +void KeepLambdas() { + auto F = +[]() { return 0; }; + auto F2 = []() { return 0; }; mgehre wrote: > JonasToth wrote: > > Does it pass

[PATCH] D61849: Do not list enabled checks when -quiet is given to run-clang-tidy.

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/D61849/new/ https://reviews.llvm.org/D61849

[PATCH] D61749: [clang-tidy] initial version of readability-static-const-method

JonasToth added inline comments. Comment at: clang-tools-extra/test/clang-tidy/readability-static-const-method.cpp:209 +void KeepLambdas() { + auto F = +[]() { return 0; }; + auto F2 = []() { return 0; }; Does it pass here? I looks a bit weird, shouldnt the

[PATCH] D61749: [clang-tidy] initial version of readability-static-const-method

JonasToth added a subscriber: shuaiwang. JonasToth added a comment. Hey, I do like your check! I think it would be great to implement this analysis in `utils/` as roman suggested. Then we have bigger flexibility. The `ExprMutAnalyzer` was requested to be moved to `clang/Analysis/` as the CSA

[PATCH] D54757: [clang-tidy] new check: bugprone-branch-clone

JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land. Did you run the new version over a big project like llvm again? If yes LGTM from my side, if not please do that before committing to ensure no new/other problems creeped in. CHANGES

[PATCH] D61508: [clang-tidy] bugprone-header-guard : a simple version of llvm-header-guard

JonasToth added a comment. > My understanding of the directory layout guidance in the docs is that > different styles get different directories. Yes, but for "the same check" we use aliases instead. When aliasing checks in different categories it is possible to change default configuration

[PATCH] D61508: [clang-tidy] misc-header-guard : a simple version of llvm-header-guard

JonasToth added a comment. Hi trixirt and thanks for the patch! I would rather like to generalize the llvm check to allow different styles and then alias the general version with different configurations. Introducing this code duplication does not sound like a good idea to me. The

[PATCH] D61475: Update an information about ReSharper C++ and clang-tidy custom binary integration

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/D61475/new/ https://reviews.llvm.org/D61475

[PATCH] D61386: [clang-tidy] Add support writing a check as a Transformer rewrite rule.

JonasToth added a comment. I like the new framework! Comment at: clang-tools-extra/clang-tidy/utils/TransformerTidy.cpp:43 + if (Rule.Explanation) { +if (Expected Explanation = Rule.Explanation(Result)) { + Message = *Explanation; these if-else are

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

JonasToth added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp:102 "bugprone-too-small-loop-variable"); +CheckFactories.registerCheck( +"bugprone-unhandled-self-assignment"); please order this list

[PATCH] D60139: [clang-tidy] Add bugprone-placement-new-target-type-mismatch check

JonasToth added a comment. Hey Dennis, my 2cents on the check. I think it is very good to have! Did you check coding guidelines if they say something to this issue? (e.g. cppcoreguidelines, hicpp, cert) As we have modules for them it would be great to make aliases to this check if they demand

[PATCH] D60453: ClangTidy: Avoid mixing stdout with stderror when dealing with a large number of files.

JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land. How does that happen to be a problem? It feels that `stdout.write` and `stderr.write` should be unconnected? (if you know the reason I would like to know it too :)) Please note, that

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

JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst:33 + + Upper limit for the magnitue bits of the loop variable. If it's

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

JonasToth added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:124 + ` now supports + `MagnitudeBitsUpperLimit` option. + Please add the new default here as its a change in behaviour of the check. Comment at:

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