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

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] D57185: [clang-tidy] Add the abseil-duration-addition check

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] D57249: [clang-tidy] fix unit tests for dropped _Float16 support in X86

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] D57249: [clang-tidy] fix unit tests for dropped _Float16 support in X86

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

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

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

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] D57113: [clang-tidy] openmp-use-default-none - a new module and a check

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] D57185: [clang-tidy] Add the abseil-duration-addition check

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] D54395: [clang-tidy] implement utility-function to add 'const' to variables

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] D57100: [clang-tidy] refactor bugprone-exception-escape analysis into class

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] D57100: [clang-tidy] refactor bugprone-exception-escape analysis into class

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

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

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] D57108: [clang-tidy] diagnose possibiltiy to add 'noexcept' in modernize-use-noexcept

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] D53830: [clang-tidy]: Abseil: new check 'abseil-upgrade-duration-conversions'

JonasToth added inline comments. Comment at: clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp:158 + *Result.Context) + .empty()) { + diag(ArgExpr->getBeginLoc(), Message); astrelni wrote: > JonasToth wrote: > > You could ellide

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

JonasToth updated this revision to Diff 171947. JonasToth added a comment. - Merge branch 'master' into experiment_isolate_decl - remove unused matcher Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51949 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/I

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

This revision was automatically updated to reflect the committed changes. Closed by commit rL345735: [clang-tidy] new check 'readability-isolate-declaration' (authored by JonasToth, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D51949 F

[PATCH] D53830: [clang-tidy]: Abseil: new check 'abseil-upgrade-duration-conversions'

JonasToth added inline comments. Comment at: clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp:167 +// likely needed. +diag(ArgExpr->getBeginLoc(), Message); +return; Minor, but you can move `diag(...)` out the if condition like this: ``` auto Di

[PATCH] D53025: [clang-tidy] implement new check for const return types.

JonasToth added a comment. Thank you very much for contributing to LLVM/clang-tidy! More patches are always welcome ;) I found one issue we have overseen that should be fixed. Would you be so kind and do that before comitting? Comment at: test/clang-tidy/readability-const-re

[PATCH] D53025: [clang-tidy] implement new check for const return types.

JonasToth added inline comments. Comment at: test/clang-tidy/readability-const-return-type.cpp:6 + +#include + JonasToth wrote: > One nit: system headers are not allowed in clang-tidy tests. We do mock the > required content. Sorry that I overlooked that. > >

[PATCH] D53025: [clang-tidy] implement new check for const return types.

JonasToth added a comment. Don't mind my last nit, we are doing this slight modification. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53025 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mai

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

JonasToth added a comment. I am, but this revision relies on support on the analysis in the frontend. This seems currently stalled as the developer there seems busy with other stuff. Once i implement transofmration-capabilites i will evaluate if there are any false-positives that would break. Onc

[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

JonasToth added a comment. For general understanding: Couldn't this check be generalized to comparing integers of different sizes? We tried a 'dont-mix-int-types' check for arithmetic already, its complicated :) But this as a specialization of the category could be done easier (i think). What d

[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

JonasToth added a comment. But that reasoning would apply to `if` and `while` loops, as it might be an "always true". But you are right, this case is not generally problematic. long size = 30; short index = 100; // Opposite comparison if(index > size) { // } Repository:

[PATCH] D54036: [fix][clang-tidy] fix for r345961 that introduced a test failure on Windows builds

JonasToth closed this revision. JonasToth added a comment. Thank you very much for the patch! Committed in r345995. Repository: rL LLVM https://reviews.llvm.org/D54036 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/

[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

JonasToth added a comment. In https://reviews.llvm.org/D53974#1285585, @ztamas wrote: > Just to make it clear. I think this check is in a good shape now. I mean it > catches usefull issues and also does not produce too many false positives, as > I see. Yes, I did not want to stop the patch or

[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

JonasToth added inline comments. Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:20 + +static const char LoopName[] = "forLoopName"; +static const char loopVarName[] = "loopVar"; ztamas wrote: > JonasToth wrote: > > Please move these variable in the

[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

JonasToth added a comment. Regarding the warning discussion: It is ok to have this check here in clang-tidy first and let it mature. Once we can handle all kind of loops and do not produce false positives this logic can move into the frontend. But in my opinion the warning-version should handle

[PATCH] D53771: [clang-tidy] Avoid C arrays check

JonasToth added inline comments. Comment at: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp:84 "cppcoreguidelines-c-copy-assignment-signature"); +CheckFactories.registerCheck( +"cppcoreguidelines-avoid-c-arrays"); lebedev.ri wro

[PATCH] D53771: [clang-tidy] Avoid C arrays check

JonasToth added inline comments. Comment at: clang-tidy/modernize/AvoidCArraysCheck.cpp:44 + unless(anyOf(hasParent(varDecl(isExternC())), + hasAncestor(functionDecl(isExternC()) + .bind("typeloc"), JonasToth wr

[PATCH] D53771: [clang-tidy] Avoid C arrays check

JonasToth added inline comments. Comment at: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp:84 "cppcoreguidelines-c-copy-assignment-signature"); +CheckFactories.registerCheck( +"cppcoreguidelines-avoid-c-arrays"); lebedev.ri wro

[PATCH] D54061: [clang-tidy] run() doesn't update the SourceManager.

JonasToth added a comment. > Theoretically, we could replace `ClangTidyCheck::check` with > `ClangTidyCheck::run`, but I'm not sure it is worth, `ClangTidyCheck::check` > is a public API, and is widely-used (for all clang-tidy checks), replacing it > requires large changes (although it is one-l

[PATCH] D53488: [clang-tidy] Catching narrowing from double to float.

JonasToth added inline comments. Comment at: test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp:164 + while (i) { +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: narrowing conversion from 'int' to 'bool' [cppcoreguidelines-narrowing-conversions] + }

[PATCH] D53488: [clang-tidy] Catching narrowing from double to float.

JonasToth added inline comments. Comment at: test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp:164 + while (i) { +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: narrowing conversion from 'int' to 'bool' [cppcoreguidelines-narrowing-conversions] + }

[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

JonasToth added a comment. last nits from my side (for now :)). If the other reviews could take a look at it as well, would be great. I am uncertain about the english in some comments @aaron.ballman finds all these language bugs ;) Comment at: clang-tidy/bugprone/TooSmallLoopV

[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

JonasToth added inline comments. Comment at: docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst:10 + + .. code-block:: c++ + Eugene.Zelenko wrote: > JonasToth wrote: > > ztamas wrote: > > > JonasToth wrote: > > > > the `.. code-block:: c++` is usually n

[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

JonasToth added inline comments. Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:20 + +static const char LoopName[] = "forLoopName"; +static const char loopVarName[] = "loopVar"; JonasToth wrote: > ztamas wrote: > > JonasToth wrote: > > > Please mov

[PATCH] D54061: [clang-tidy] run() doesn't update the SourceManager.

JonasToth added a comment. In https://reviews.llvm.org/D54061#1288395, @sammccall wrote: > In https://reviews.llvm.org/D54061#1286956, @JonasToth wrote: > > > > Theoretically, we could replace `ClangTidyCheck::check` with > > > `ClangTidyCheck::run`, but I'm not sure it is worth, > > > `ClangTi

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth created this revision. JonasToth added reviewers: alexfh, aaron.ballman, hokein, sammccall, lebedev.ri. Herald added subscribers: cfe-commits, xazax.hun, mgorny. `run-clang-tidy.py` is the parallel executor for `clang-tidy`. Due to the common header-inclusion problem in C++/C diagnostics

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth updated this revision to Diff 172726. JonasToth added a comment. - spurious change in my git Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54141 Files: clang-tidy/tool/run-clang-tidy.py clang-tidy/tool/run_clang_tidy.py clang-tidy/tool/test_input/out_csa_cmake.

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth added inline comments. Comment at: clang-tidy/tool/run_clang_tidy.py:1 +run-clang-tidy.py This simlink is required for my unittests, I don't know how to add the added tests in the `lit` test-suite so there is no change there yet. A bit of guidance the

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth added a comment. > - The output of clang-tidy diagnostic is YAML, and YAML is not an > space-efficient format (just for human readability). If you want to save > space further, we might consider using some compressed formats, e.g. > llvm::bitcode. Given the reduced YAML result (5.4MB)

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth added a comment. Thank you for the comment! In https://reviews.llvm.org/D54141#1288809, @steveire wrote: > This feature seems like a good idea. I started writing it too some months > ago, but then I changed tactic and worked on distributing the refactor over > the network instead. As

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth added a comment. > Maybe my suggestion was not clear. The yaml file generated by clang-tidy > contains not only replacements, but all diagnostics, even without a fixit. > > So, running `clang-apply-replacements --issue-diags the_new_file.yaml` would > issue the warnings/fixit hints by

[PATCH] D54157: [clangd] [NFC] Fix clang-tidy warnings.

JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land. Fixing such warnings is ok without prior review. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54157 ___ cfe-commits mail

[PATCH] D53488: [clang-tidy] Catching narrowing from double to float.

JonasToth added a comment. I think the check is really close. If the other reviewers want to take a look at it again, now is a good moment :) Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:178 + return; +// Conversions to unsigned integer are we

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth added a comment. > Do you understand the proposal now? Yes better, I was under the impression that `clang-apply-replaments` is run on the end and the YAMLs are kept until then. Now its clear. I assume `--issue-diags` produce the same result as the normal diagnostic engine. That could

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth added a comment. In https://reviews.llvm.org/D54141#1288993, @Eugene.Zelenko wrote: > Reducing log file size is good idea, but I think will be also good idea to > count duplicates. This will allow to concentrate clean-up efforts on place > where most of warnings originate. Places th

[PATCH] D54168: [clang-tidy] Zircon fbl::move -> std::move

JonasToth added a comment. I think this check is ok in the current form, but does it really need to be in upstream clang-tidy? You said its for migration only, so it is not valuable for you for a long time either? Comment at: clang-tools-extra/clang-tidy/zircon/FblMoveCheck.h

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth added a comment. In https://reviews.llvm.org/D54141#1288930, @JonasToth wrote: > > Do you understand the proposal now? > > Yes better, I was under the impression that `clang-apply-replaments` is run > on the end and the YAMLs are kept until then. Now its clear. > I assume `--issue-dia

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth added a comment. > At least the clang-tidy quiet mode is trivial to implement. Maybe instead of > `--quiet` we could have `--stdout=` where `output_format` can > be one of `none`, `diag`, `yaml` and in the future possibly `json` (requested > here: http://lists.llvm.org/pipermail/cfe-d

[PATCH] D54222: [clang-tidy] Add a check to detect returning static locals in public headers

JonasToth added a comment. I agree with @Eugene.Zelenko that this check should rather live in `bugprone-` as the issue its diagnosing is not LLVM specific. It is ok to add an alias into the llvm module. Comment at: clang-tidy/llvm/ProblematicStaticsCheck.cpp:22 +void Problema

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth added a comment. > Could you please explain your motivation of catching clang-tidy stdout? > `--export-fixes` emits everything of `diagnostic` to YAML even the > `diagnostic` doesn't have fixes. I guess the reason is that you want code > snippets that you could show to users? If so, I

[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

JonasToth added a comment. LG from my side. Please wait for feedback from @aaron.ballman or @alexfh before committing. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53974 ___ cfe-commits mailing list cfe-commits@lists.llvm.org htt

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

JonasToth added inline comments. Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:178 + return; +// Conversions to unsigned integer are well defined and follow modulo 2 +// arithmetic. gchatelet wrote: > JonasToth wrote: > > I a

[PATCH] D54222: [clang-tidy] Add a check to detect returning static locals in public headers

JonasToth added inline comments. Comment at: clang-tidy/llvm/ProblematicStaticsCheck.cpp:33 + const auto *VD = Result.Nodes.getNodeAs("var"); + const auto *Return = Result.Nodes.getNodeAs("return"); + diag(Return->getBeginLoc(), "address of static local variable %0 may not " -

[PATCH] D54257: [clang-tidy] **Prototype** use AllTUsExecutors

JonasToth added a comment. Very interesting. As I am not very familiar with all the internals I do have a few questions :) Right now notes seem not be closely attached to their related warning. But within the there is a check, that a note is emitted after an error. Would it make sense to group

[PATCH] D54222: [clang-tidy] Add a check to detect returning static locals in public headers

JonasToth added inline comments. Comment at: clang-tidy/llvm/ProblematicStaticsCheck.cpp:33 + const auto *VD = Result.Nodes.getNodeAs("var"); + const auto *Return = Result.Nodes.getNodeAs("return"); + diag(Return->getBeginLoc(), "address of static local variable %0 may not " -

[PATCH] D54281: [clang-tidy] fix PR39583 - ignoring ParenCast for string-literals in pro-bounds-array-to-pointer-decay

JonasToth created this revision. JonasToth added reviewers: aaron.ballman, alexfh, hokein. Herald added subscribers: cfe-commits, kbarton, xazax.hun, nemanjai. The fix to the issue that `const char* p = ("foo")` is diagnosed as decay is to ignored the ParenCast. Resolves PR39583 Repository: rC

[PATCH] D54281: [clang-tidy] fix PR39583 - ignoring ParenCast for string-literals in pro-bounds-array-to-pointer-decay

JonasToth added inline comments. Comment at: clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp:65 + unless(isInsideOfRangeBeginEndStmt()), + unless(hasSourceExpression(ignoringParenCasts(stringLiteral() .bind("cast"), ---

[PATCH] D54281: [clang-tidy] fix PR39583 - ignoring ParenCast for string-literals in pro-bounds-array-to-pointer-decay

JonasToth updated this revision to Diff 173311. JonasToth added a comment. - use ignoringParens instead Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54281 Files: clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp test/clang-tidy/cppcoreguidelines-pro-bound

[PATCH] D54307: [clang] overload ignoringParens for Expr

JonasToth created this revision. JonasToth added a reviewer: aaron.ballman. Herald added a subscriber: cfe-commits. This patch allows fixing PR39583. Repository: rC Clang https://reviews.llvm.org/D54307 Files: docs/LibASTMatchersReference.html include/clang/ASTMatchers/ASTMatchers.h li

[PATCH] D54307: [clang] overload ignoringParens for Expr

JonasToth added inline comments. Comment at: include/clang/ASTMatchers/ASTMatchers.h:814 /// would match the declaration for fp. -AST_MATCHER_P(QualType, ignoringParens, - internal::Matcher, InnerMatcher) { +AST_MATCHER_P_OVERLOAD(QualType, ignoringParens, internal:

[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land. LGTM. Did you run this check in its final form against a bigger project? These results would be interesting. Do you have commit access? Repository: rCTE Clang Tools Extra https://rev

[PATCH] D54307: [clang] overload ignoringParens for Expr

JonasToth added inline comments. Comment at: include/clang/ASTMatchers/ASTMatchers.h:814 /// would match the declaration for fp. -AST_MATCHER_P(QualType, ignoringParens, - internal::Matcher, InnerMatcher) { +AST_MATCHER_P_OVERLOAD(QualType, ignoringParens, internal:

[PATCH] D54307: [clang] overload ignoringParens for Expr

JonasToth updated this revision to Diff 173391. JonasToth marked 3 inline comments as done. JonasToth added a comment. - add unit test Repository: rC Clang https://reviews.llvm.org/D54307 Files: docs/LibASTMatchersReference.html include/clang/ASTMatchers/ASTMatchers.h lib/ASTMatchers/D

[PATCH] D54307: [clang] overload ignoringParens for Expr

JonasToth added inline comments. Comment at: include/clang/ASTMatchers/ASTMatchers.h:814 /// would match the declaration for fp. -AST_MATCHER_P(QualType, ignoringParens, - internal::Matcher, InnerMatcher) { +AST_MATCHER_P_OVERLOAD(QualType, ignoringParens, internal:

[PATCH] D54307: [ASTMatchers] overload ignoringParens for Expr

JonasToth updated this revision to Diff 173422. JonasToth added a comment. - add unit test Repository: rC Clang https://reviews.llvm.org/D54307 Files: docs/LibASTMatchersReference.html include/clang/ASTMatchers/ASTMatchers.h lib/ASTMatchers/Dynamic/Registry.cpp unittests/ASTMatchers/

[PATCH] D54281: [clang-tidy] fix PR39583 - ignoring ParenCast for string-literals in pro-bounds-array-to-pointer-decay

JonasToth updated this revision to Diff 173423. JonasToth added a comment. - use ignoringParens instead Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54281 Files: clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp test/clang-tidy/cppcoreguidelines-pro-bound

[PATCH] D54307: [ASTMatchers] overload ignoringParens for Expr

This revision was automatically updated to reflect the committed changes. Closed by commit rC346554: [ASTMatchers] overload ignoringParens for Expr (authored by JonasToth, committed by ). Changed prior to commit: https://reviews.llvm.org/D54307?vs=173422&id=173424#toc Repository: rC Clang h

[PATCH] D54281: [clang-tidy] fix PR39583 - ignoring ParenCast for string-literals in pro-bounds-array-to-pointer-decay

JonasToth updated this revision to Diff 173425. JonasToth added a comment. - fix comment Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54281 Files: clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-poin

[PATCH] D54281: [clang-tidy] fix PR39583 - ignoring ParenCast for string-literals in pro-bounds-array-to-pointer-decay

This revision was automatically updated to reflect the committed changes. Closed by commit rL346555: [clang-tidy] fix PR39583 - ignoring ParenCast for string-literals in pro-bounds… (authored by JonasToth, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://rev

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-pp'

JonasToth added a comment. Hi vmiklos, thank you for working on this patch! I added a few comments. Comment at: clang-tidy/readability/ReadabilityTidyModule.cpp:83 "readability-misplaced-array-index"); +CheckFactories.registerCheck("readability-redundant-pp");

[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

JonasToth added a comment. > I'll run it on LibreOffice code again and we'll see. Perfect, if you have the time, running it against LLVM would be very interesting as well. >> Do you have commit access? > > Commit access? This is my first patch. If you plan to regularly contribute to LLVM you

[PATCH] D53771: [clang-tidy] Avoid C arrays check

JonasToth added inline comments. Comment at: clang-tidy/modernize/AvoidCArraysCheck.cpp:26 + const clang::Type *TypeNode = Node.getTypePtr(); + return (TypeNode != nullptr && + InnerMatcher.matches(*TypeNode, Finder, Builder)); Nit: these parens are su

[PATCH] D53771: [clang-tidy] Avoid C arrays check

JonasToth added a comment. from my side mostly ok. I think the typedef points should be clarified, but I dont see more issues. Comment at: test/clang-tidy/modernize-avoid-c-arrays.cpp:32 + +using k = int[4]; +// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not declare C-style

[PATCH] D53771: [clang-tidy] Avoid C arrays check

JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land. LGTM. Please give other reviewers a chance to take a look at it too. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53771

[PATCH] D53771: [clang-tidy] Avoid C arrays check

JonasToth added a comment. In https://reviews.llvm.org/D53771#1294244, @lebedev.ri wrote: > *Maybe* we should be actually diagnosing the each actual declaration, not > just the `typeloc`. > Else if you use one `typedef`, and `// NOLINT` it, you will silence it for > all the decls that use it.

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

JonasToth added a comment. > Done. Please also mark the inline comments as done. Otherwise its hard to follow if there are outstanding issues somewhere, especially if code moves around. https://reviews.llvm.org/D54349 ___ cfe-commits mailing list

[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

JonasToth added a comment. Thank you for checking these two projects! In https://reviews.llvm.org/D53974#1294375, @ztamas wrote: > I have the result after running the current version of the check on > LibreOffice. > > Found around 50 new issues were hidden by macros: > > https://cgit.freedesk

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

JonasToth added inline comments. Comment at: docs/clang-tidy/checks/readability-redundant-preprocessor.rst:34 + + #ifdef FOO + #ifndef FOO // inner ifndef is considered redundant The indendation for these examples (following as well) is not enough. Please use 2

[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

JonasToth added a comment. >> I would be very interested why these false positives are created. Is there a >> way to avoid them and maybe it makes sense to already add `FIXME` at the >> possible places the check needs improvements. > > voidForLoopFalsePositive() and voidForLoopFalsePositive2()

[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

JonasToth added a comment. > Yes, that's right, these are not full false positives, but the check's main > focus is on those loops which are runtime dependent. If a loop's upper bound > can be calculated in compile time then this loop should be caught by a > compiler warning based on the actual

[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

JonasToth added inline comments. Comment at: test/clang-tidy/bugprone-too-small-loop-variable.cpp:138 + bool cond = false; + for (short i = 0; i < (cond ? 0 : size); ++i) { +// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: loop variable has narrower type 'short' than iteration

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

JonasToth created this revision. JonasToth added reviewers: aaron.ballman, hokein, alexfh, shuaiwang, lebedev.ri. Herald added subscribers: cfe-commits, xazax.hun, mgorny. JonasToth added a dependent revision: D45444: [clang-tidy] implement new check for const-correctness. This patch extends the

[PATCH] D54399: Move ExprMutationAnalyzer to Tooling/Analysis (1/3)

JonasToth added a comment. Will you update the existing clang-tidy checks as well? Repository: rC Clang https://reviews.llvm.org/D54399 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

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

JonasToth updated this revision to Diff 173653. JonasToth added a comment. - Merge branch 'master' into check_const Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45444 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp

[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

This revision was automatically updated to reflect the committed changes. Closed by commit rL346665: [clang-tidy] new check: bugprone-too-small-loop-variable (authored by JonasToth, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D53974 Fi

[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

JonasToth added a comment. Committed r346665. Thank you very much for the patch! Repository: rL LLVM https://reviews.llvm.org/D53974 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commi

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

JonasToth added a comment. Only a few nits from me. Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:30 + WarnOnFloatingPointNarrowingConversion( + Options.get("WarnOnFloatingPointNarrowingConversion", 0)) {} + I would make t

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land. So, finally LGTM :) Please give @aaron.ballman a chance to comment, as he reviewed too. Thanks for your patch! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53488 _

[PATCH] D54262: [clang-tidy] Add `delete this` bugprone check (PR38741)

JonasToth added a comment. Ok, Thank you for looking into it. You can abondon the revision in phabricator (Selection above comment field at the bottom of the page) if you wish. Am 19.11.18 um 23:36 schrieb Mateusz Maćkowski via Phabricator: > m4tx added a comment. > > After thinking about the p

[PATCH] D54737: [clang-tidy] Add the abseil-duration-comparison check

JonasToth added a comment. Could you please upload the diff with full context? Especially the parts with refactoring are harder to judge if the code around is not visible. Thank you :) Comment at: clang-tidy/abseil/DurationComparisonCheck.cpp:25 +static llvm::Optional getScale

[PATCH] D54737: [clang-tidy] Add the abseil-duration-comparison check

JonasToth added inline comments. Comment at: test/clang-tidy/abseil-duration-comparison.cpp:89 + // CHECK-FIXES: d1 > d2; + + // Check against the LHS JonasToth wrote: > What would happen for a type, that can implicitly convert to a duration or > double/int. >

[PATCH] D54745: [clang-tidy] Don't generate incorrect fixes for class with deleted copy constructor in smart_ptr check.

JonasToth added a comment. Does make_unique require the copy constructor if it could move? And would the same argument apply to the move-constructors as the arguments are forwarded? What would happen in the obscure case of a public copy-constructor, but private move-constructor (not saying it ma

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

JonasToth updated this revision to Diff 174828. JonasToth added a comment. add tests for different template instantiations Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54395 Files: clang-tidy/performance/ForRangeCopyCheck.cpp clang-tidy/performance/UnnecessaryCopyInitiali

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

JonasToth added inline comments. Comment at: unittests/clang-tidy/AddConstTest.cpp:733 + StringRef T = "template void f(T v) \n"; + StringRef S = "{ T target = v; }"; + auto Cat = [&T](StringRef S) { return (T + S).str(); }; alexfh wrote: > It would be intere

[PATCH] D54745: [clang-tidy] Don't generate incorrect fixes for class with deleted copy constructor in smart_ptr check.

JonasToth added a comment. IMHO this patch is fine, but i think a language expert (not me :D) should take a look (@aaron.ballman ?) as its complicated :) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54745 ___ cfe-commits mailing

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

JonasToth updated this revision to Diff 174942. JonasToth added a comment. - Merge branch 'master' into check_const Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45444 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp

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