[PATCH] D37060: [clang-tidy] Improve hicpp-exception-baseclass to handle generic code better

JonasToth marked 3 inline comments as done. JonasToth added inline comments. Comment at: clang-tidy/hicpp/ExceptionBaseclassCheck.cpp:41 + "'std::exception'") + << BadThrow->getSubExpr()->getType() << BadThrow->getSourceRange();

[PATCH] D37060: [clang-tidy] Improve hicpp-exception-baseclass to handle generic code better

JonasToth updated this revision to Diff 113252. JonasToth added a comment. - added inheritance cases, adjusted matcher is required https://reviews.llvm.org/D37060 Files: clang-tidy/hicpp/ExceptionBaseclassCheck.cpp test/clang-tidy/hicpp-exception-baseclass.cpp Index: test/clang-tidy/hicpp-

[PATCH] D37060: [clang-tidy] Improve hicpp-exception-baseclass to handle generic code better

JonasToth marked 3 inline comments as done. JonasToth added inline comments. Comment at: test/clang-tidy/hicpp-exception-baseclass.cpp:9 +class deep_hierarchy : public derived_exception {}; class non_derived_exception {}; aaron.ballman wrote: > JonasToth wrote

[PATCH] D36354: [clang-tidy] Implement type-based check for `gsl::owner`

JonasToth updated this revision to Diff 113265. JonasToth added a comment. - adjusted the diagnostic for member variables https://reviews.llvm.org/D36354 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp clang-tidy/cppcoreguide

[PATCH] D37060: [clang-tidy] Improve hicpp-exception-baseclass to handle generic code better

JonasToth marked 5 inline comments as done. JonasToth added inline comments. Comment at: test/clang-tidy/hicpp-exception-baseclass.cpp:9 +class deep_hierarchy : public derived_exception {}; class non_derived_exception {}; aaron.ballman wrote: > JonasToth wrote

[PATCH] D37060: [clang-tidy] Improve hicpp-exception-baseclass to handle generic code better

JonasToth updated this revision to Diff 113269. JonasToth marked an inline comment as done. JonasToth added a comment. - adjusted expected diagnostics - adjust diagnostics and remove private inheritance cases https://reviews.llvm.org/D37060 Files: clang-tidy/hicpp/ExceptionBaseclassCheck.cpp

[PATCH] D36354: [clang-tidy] Implement type-based check for `gsl::owner`

JonasToth added inline comments. Comment at: docs/clang-tidy/checks/cppcoreguidelines-owning-memory.rst:11 +The relevant sections in the `C++ Core Guidelines `_ are I.11, C.33, R.3 and GSL.Views +The

[PATCH] D36354: [clang-tidy] Implement type-based check for `gsl::owner`

JonasToth updated this revision to Diff 113272. JonasToth marked 5 inline comments as done. JonasToth added a comment. - fix typos and bad language https://reviews.llvm.org/D36354 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp

[PATCH] D36354: [clang-tidy] Implement type-based check for `gsl::owner`

JonasToth added a comment. language in the docuementation improved https://reviews.llvm.org/D36354 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36354: [clang-tidy] Implement type-based check for `gsl::owner`

JonasToth added a comment. ping https://reviews.llvm.org/D36354 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36354: [clang-tidy] Implement type-based check for `gsl::owner`

JonasToth added inline comments. Comment at: test/clang-tidy/cppcoreguidelines-owning-memory.cpp:39 + return new int(42); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: returning a 'gsl::owner<>' from a function but not declaring it; return type is 'int *' +} aa

[PATCH] D36354: [clang-tidy] Implement type-based check for `gsl::owner`

JonasToth updated this revision to Diff 114483. JonasToth marked 4 inline comments as done. JonasToth added a comment. - Merge branch 'master' - mention current problems with typedefs of owner<> and adjust test cases https://reviews.llvm.org/D36354 Files: clang-tidy/cppcoreguidelines/CMakeLis

[PATCH] D36354: [clang-tidy] Implement type-based check for `gsl::owner`

JonasToth added a comment. - Adressed some review comments. - Added testcases, where the `owner<>` annotation might be hidden by a `typedef` or `using`, like int `using heap_int = gsl::owner;` The check is currently not able to resolve a `typedef` to `owner<>` correctly, and my knowledge in cla

[PATCH] D33826: [clang-tidy] avoid pointer cast to more strict alignment check

JonasToth added a comment. There is an exception to the general rule (EXP36-C-EX2), stating that the result of `malloc` and friends is allowed to be casted to stricter alignments, since the pointer is known to be of correct alignment. Could you add a testcase for this case, i think there would c

[PATCH] D36354: [clang-tidy] Implement type-based check for `gsl::owner`

JonasToth updated this revision to Diff 114848. JonasToth added a comment. - address review comments, especially improve diagnostic for return types, that should be owner but arent https://reviews.llvm.org/D36354 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreguidelin

[PATCH] D36354: [clang-tidy] Implement type-based check for `gsl::owner`

JonasToth updated this revision to Diff 114849. JonasToth added a comment. - fix typos https://reviews.llvm.org/D36354 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp clang

[PATCH] D36354: [clang-tidy] Implement type-based check for `gsl::owner`

JonasToth updated this revision to Diff 114851. JonasToth added a comment. - fix, that deleted destructors are caught as problem as well - clarify one comment, duplicated avoid was misleading https://reviews.llvm.org/D36354 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppco

[PATCH] D36354: [clang-tidy] Implement type-based check for `gsl::owner`

JonasToth added a comment. commited in https://reviews.llvm.org/rL313043 https://reviews.llvm.org/D36354 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36354: [clang-tidy] Implement type-based check for `gsl::owner`

JonasToth reopened this revision. JonasToth added a comment. This revision is now accepted and ready to land. This Patch broke the buildbot for vs-2015. I will revert, when i figured out how to do this in svn :/ It does not emit a warning for line 311: here the log > C:\b\slave\clang-x86-windows

[PATCH] D36354: [clang-tidy] Implement type-based check for `gsl::owner`

JonasToth updated this revision to Diff 114881. JonasToth added a comment. - make matcher for ctor initialization more readable https://reviews.llvm.org/D36354 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp clang-tidy/cppcor

[PATCH] D36354: [clang-tidy] Implement type-based check for `gsl::owner`

JonasToth added a comment. Recommited with fix in https://reviews.llvm.org/rL313067 https://reviews.llvm.org/D36354 ___ 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

JonasToth added a comment. I added a clarifying comment for some outcommented code still in the patch. I would like to have a bit of discussion on it. Maybe some parts of the check could be warning-area, i just implemented it here to have it on one place. Comment at: clang-ti

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

JonasToth updated this revision to Diff 115037. JonasToth marked 3 inline comments as done. JonasToth added a comment. - fix the first issues i found https://reviews.llvm.org/D37808 Files: clang-tidy/hicpp/CMakeLists.txt clang-tidy/hicpp/HICPPTidyModule.cpp clang-tidy/hicpp/MultiwayPathsC

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

JonasToth added a comment. fixed first stuff, sorry for noise. 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

JonasToth added a comment. In https://reviews.llvm.org/D37808#869602, @Eugene.Zelenko wrote: > I think will be good idea to extend -Wswitch diagnostics. Ok. But it will introduce new warnings to llvm codebase itself. I prepare some example output i found right now. https://reviews.llvm.org/D

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

JonasToth added a comment. In https://reviews.llvm.org/D37808#869823, @lebedev.ri wrote: > What about GNU extension `case 1 ... 3:` ? Strictly speaking, the coding standard (which should be enforced by the patch) requires strict ISO C++11, therefore this extension is not considered directly. D

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

JonasToth added a comment. Looking up to the Visitor, i will do this next. Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:27 + // Here are all the possible reasons: + enum Criteria : unsigned char { +None = 0, i think clarifying w

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

JonasToth added a comment. I did read through it now. In general, the code is sound with my understanding of the complexity metric and there is a almost one to one wording to the document, which is nice. Since we chatted in IRC directly, i would give a short summary to avoid forgetting what we

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

JonasToth added inline comments. Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:27 + // Here are all the possible reasons: + enum Criteria : unsigned char { +None = 0, JonasToth wrote: > i think clarifying which language constructs

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

JonasToth added a comment. I built the patch locally, here is my output: The Unittest failed for me: `Neither CHECK-FIXES nor CHECK-MESSAGES found in the input` Script call: `/usr/bin/python2.7 /home/jonas/opt/llvm/tools/clang/tools/extra/test/../test/clang-tidy/check_clang_tidy.py /home/jo

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

JonasToth added a comment. I see, forgot that. The doc were the interesting part for me, I trust that you test the code correctlty :) Repository: rL LLVM https://reviews.llvm.org/D36836 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

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

JonasToth added a comment. In https://reviews.llvm.org/D37808#869810, @Eugene.Zelenko wrote: > In https://reviews.llvm.org/D37808#869612, @JonasToth wrote: > > > In https://reviews.llvm.org/D37808#869602, @Eugene.Zelenko wrote: > > > > > I think will be good idea to extend -Wswitch diagnostics. >

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

JonasToth added a comment. Informationen my part think that both switch and else should stay here for now. Running it over llvm gave really a lot of warnings, and that a rather good codebase. The check did not recognize logic, your example would be warned against. I don't see a way to implemen

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

JonasToth added a comment. For my part the current state is ok. @alexfh and @aaron.ballman but should do their review before committing. I would be interested in a exampleoutput for any real project. Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:102-1

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

JonasToth added inline comments. Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:102-114 +const std::array CognitiveComplexity::Msgs = {{ +// B1 + B2 + B3 +"+%0, including nesting penalty of %1, nesting level increased to %2", + +// B1 + B2 +

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

JonasToth added a comment. 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] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

JonasToth added a comment. Hi Roman, there are still some comments marked as `not done` from my review. Maybe other reviewers wait for there 'resolution', but from my side there are resolved. One more attempt to get the review rolling. Repository: rL LLVM https://reviews.llvm.org/D36836

[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 one

[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 Github

[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 fo

[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 la

[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 here?

[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] 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] 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] 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. Chang

[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 https://review

[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] 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 https://lists.

[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 SIN

[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. Changed

[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 https://lists.llvm.org/c

[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 mailin

[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: clang-tools-extra/clang-tidy/modernize/ConcatNes

[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 https://reviews

[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: clang-tools-extra/clang-tidy/modernize/ConcatNe

[PATCH] D55595: [clang-tidy] Share the forced linking code between clang-tidy tool and plugin

JonasToth added a comment. In D55595#1328154 , @steveire wrote: > FYI, CMake target property `INTERFACE_SOURCES` is designed to make this easy. > > For each module you would generate a file containing > > extern volatile int ${MODULE_NAME}ModuleAnchorSou

[PATCH] D54309: [AST] Allow limiting the scope of common AST traversals (getParents, RAV).

JonasToth added a comment. > Sounds like we might not correctly set the parent map for CXXConstructorDecl > then? I unfortunatly don't know where to start to look for. Could you give me a hint what to inspect to figure that out? Repository: rC Clang CHANGES SINCE LAST ACTION https://revi

[PATCH] D55595: [clang-tidy] Share the forced linking code between clang-tidy tool and plugin

JonasToth added a comment. In D55595#1328263 , @steveire wrote: > Can you say where else it is common in LLVM? I'm curious. Maybe those places > could be changed too. My "is common" is not quantified, I have seen it before :) From grepping "extern vola

[PATCH] D55245: [clang-tidy] Add the abseil-duration-subtraction check

JonasToth added inline comments. Comment at: test/clang-tidy/abseil-duration-subtraction.cpp:12 + // CHECK-FIXES: absl::ToDoubleSeconds(d - absl::Seconds(1)) + x = absl::ToDoubleSeconds(d) - absl::ToDoubleSeconds(d1); + // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtra

[PATCH] D55245: [clang-tidy] Add the abseil-duration-subtraction check

JonasToth added a comment. Did you rebase the check onto the new master with your refactorings in? Comment at: test/clang-tidy/abseil-duration-subtraction.cpp:12 + // CHECK-FIXES: absl::ToDoubleSeconds(d - absl::Seconds(1)) + x = absl::ToDoubleSeconds(d) - absl::ToDoubleSecon

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

JonasToth added a comment. > You also get into trouble because there are many header files that it add > LLVM_NODISCARD to but they don't include "Compiler.h" where LLVM is defined > (so you end up with lots of errors) > 3>C:\llvm\include\llvm/ADT/iterator_range.h(46): error C3646: 'IteratorT'

[PATCH] D55595: [clang-tidy] Share the forced linking code between clang-tidy tool and plugin

JonasToth added a comment. One thing: Could you please check the utility-scripts in clang-tidy that create new checks, if they need adjustments? Not sure if we have something in there that relies on the old structure. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://re

[PATCH] D55245: [clang-tidy] Add the abseil-duration-subtraction check

JonasToth accepted this revision. JonasToth added a comment. LGTM with the doc-nit. Comment at: docs/clang-tidy/checks/abseil-duration-subtraction.rst:33 +Note: As with other ``clang-tidy`` checks, it is possible that multiple fixes +may overlap (as in the case of nested expres

[PATCH] D55595: [clang-tidy] Share the forced linking code between clang-tidy tool and plugin

JonasToth accepted this revision. JonasToth added a comment. LGTM, chatted with steveire on IRC: blocking this for a better cmake based solution makes no sense. So your free to commit :) Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55595/new/ http

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

JonasToth added inline comments. Comment at: test/clang-tidy/modernize-use-nodiscard.cpp:126 +template +class Bar +{ MyDeveloperDay wrote: > curdeius wrote: > > JonasToth wrote: > > > I think the template tests should be improved. > > > What happens for `T empty(

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

JonasToth added inline comments. Comment at: test/clang-tidy/modernize-use-nodiscard.cpp:183 +// CHECK-MESSAGES-NOT: warning: +// CHECK-FIXES: bool f33(T&) const + No warning -> No fix -> You can ellide the `CHECK-FIXES` here and elsewhere. FileCheck

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

JonasToth added inline comments. Comment at: clang-tidy/readability/DuplicatedAccessSpecifiersCheck.cpp:51 + diag(ASDecl->getLocation(), "duplicated access specifier") + << MatchedDecl + << FixItHint::CreateRemoval(ASDecl->getSourceRange()); ---

[PATCH] D56025: [clang-tidy] add IgnoreMacros option to readability-uppercase-literal-suffix

JonasToth accepted this revision. JonasToth added a comment. LGTM, do you have commit rights? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56025/new/ https://reviews.llvm.org/D56025 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

JonasToth added a comment. Sorry for my absence the last week, holidays came in between. I will take a look at the code again and try to find a robuster way of template-type detection :) I do have an idea, but not sure if that works as expected (within this week). Sorry again! CHANGES SINCE L

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

JonasToth added inline comments. Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:24 +// parameters via using alias not detected by ``isTemplateTypeParamType()``. +static bool isAliasedTemplateParamType(const QualType &ParamType) { + return (ParamType.getCanonicalType().ge

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

JonasToth added a comment. Hi Bernhard, thanks for you patch! You mentioned that this is your first contribution, if you didn't find these docs already they might help you with the LLVM source-code a bit: - https://llvm.org/docs/ProgrammersManual.html - https://llvm.org/docs/DeveloperPolicy.htm

[PATCH] D54309: [AST] Allow limiting the scope of common AST traversals (getParents, RAV).

JonasToth added a comment. new years ping on the crash-issue. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54309/new/ https://reviews.llvm.org/D54309 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://li

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

JonasToth added inline comments. Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:30 +// TODO: Find a better way of detecting a function template. +static bool isFunctionTemplate(const QualType &ParamType) { + // Try to catch both std::function and boost::function

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

JonasToth added a comment. Mostly nits left. I think the check is good to go afterwards :) Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:23 +static bool isAliasedTemplateParamType(QualType ParamType) { + return (ParamType.getCanonicalType()->isTemplateTypeParmType() |

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

JonasToth added a comment. Thanks for the patch. Is this revision dependent on D56303 (or other way around)? Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:464 bool Value, Str

[PATCH] D56303: [clang-tidy] Handle case/default statements when simplifying boolean expressions

JonasToth added inline comments. Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:386 - bool BoolValue = Bool->getValue(); + const bool BoolValue = Bool->getValue(); `const` on values is uncommon in clang-tidy code. Please keep that consisten

[PATCH] D56343: [clang-tidy] Refactor: Compose Method on check_clang_tidy.py

JonasToth added inline comments. Comment at: test/clang-tidy/check_clang_tidy.py:112 +process_output = e.output.decode() +print('%s failed:\n%s' % (' '.join(args), process_output)) +if raise_error: Its better to use `.format()` instead of `%` syntax

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

JonasToth accepted this revision. JonasToth added a comment. LGTM. If you have the time please rerun this latest (final?) version over LLVM or any other bigger project and check if there are any issues left and ensure the code still compiles after code-transformation. CHANGES SINCE LAST ACTIO

[PATCH] D56343: [clang-tidy] Refactor: Compose Method on check_clang_tidy.py

JonasToth added a comment. In D56343#1347352 , @LegalizeAdulthood wrote: > I really want to get these reviewed quickly. Otherwise, I will run out of > available time to complete them and get them submitted. I will give my best to be available this wee

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

JonasToth added inline comments. Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:464 bool Value, StringRef Id) { - auto SimpleThen = binaryOperator( - hasOperatorName("="), - hasLHS(declRefExpr(hasDecla

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

JonasToth added inline comments. Comment at: test/clang-tidy/readability-simplify-bool-expr-members.cpp:160 + // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: {{.*}} in ternary expression result + // CHECK-FIXES: {{^ m_b2 = i <= 20;$}} + LegalizeAdulthood wrote:

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

JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land. Did you check if the changes actually fix the problematic behaviour? Did you run it over any project and did the code-transformation break the code, false positives, ...? Otherwise LGTM

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

JonasToth added a comment. In D56323#1347489 , @LegalizeAdulthood wrote: > I managed to do some limited testing on the original source file from the bug > report in lldb and verified that the correct fix was applied there. I also > tried a few other fi

[PATCH] D56303: [clang-tidy] Handle case/default statements when simplifying boolean expressions

JonasToth added inline comments. Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:386 - bool BoolValue = Bool->getValue(); + const bool BoolValue = Bool->getValue(); aaron.ballman wrote: > LegalizeAdulthood wrote: > > JonasToth wrote: > > > `

[PATCH] D54309: [AST] Allow limiting the scope of common AST traversals (getParents, RAV).

JonasToth added a comment. @sammccall Thank you for analyzing and investigation! Keeping the assertion is of course good and I am not sure on how to proceed. Given the close branch for 8.0 we might opt for an hotfix that resolves the issue for now. I think the bug-report is the better place to

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

JonasToth added a comment. > - if the closing parenthesis of the function is inside a macro, no FixIt will > be created (I cannot relyably lex for subsequent CV and ref qualifiers and > maybe we do not want to make changes in macros) Usually macros are untouched because its impossible to transf

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

JonasToth added a comment. In D55433#1347553 , @MyDeveloperDay wrote: > libprotobuf still builds cleanly with just the one expected warning..despite > adding over 500 [[nodiscards]] > > F7800873: image.png > > I'll con

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

JonasToth added a comment. No problem, thats why we test on real projects first, because there is always something hidden in C++ :) Is there a test for the lambdas? Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:39 +AST_MATCHER(CXXMethodDecl, isConversionDecl) { + //

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

JonasToth added inline comments. Comment at: clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp:23 + +constexpr char kDisabledTestPrefix[] = "DISABLED_"; + Please use `llvm::StringLiteral` Comment at: clang-tidy/google/AvoidUnderscoreIn

[PATCH] D56444: [AST] RecursiveASTVisitor visits lambda classes when implicit visitation is on.

JonasToth added a comment. I still see the unit-test crashing for `ExprMutAnalyzer` (just apply these tests https://github.com/JonasToth/clang/blob/fix_crash/unittests/Analysis/ExprMutationAnalyzerTest.cpp and run `check-clang-unit`) The clang-tidy check does not crash anymore, but `readability

[PATCH] D56444: [AST] RecursiveASTVisitor visits lambda classes when implicit visitation is on.

JonasToth added a comment. In D56444#1350746 , @sammccall wrote: > In D56444#1350252 , @JonasToth wrote: > > > I still see the unit-test crashing for `ExprMutAnalyzer` (just apply the > > last two tests > > https:

[PATCH] D56444: [AST] RecursiveASTVisitor visits lambda classes when implicit visitation is on.

JonasToth added a comment. In D56444#1350897 , @steveire wrote: > The timing of this is unfortunate because the change will not be needed soon. > Refactoring is underway to extract a generic traverser from ASTDumper. Then > the parent/child traversal of

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

JonasToth added a comment. @LegalizeAdulthood your stuck on the commit right things right? If you want I can commit for you (maybe even later) as you said you are limited on time. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56323/new/ https://reviews.llvm.org/D56323 __

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

JonasToth accepted this revision. JonasToth added a comment. LGTM! You verified that your fixes, fix the issues in LLVM? But it looks good to go. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55433/new/ https://reviews.llvm.org/D55433 ___ c

[PATCH] D56444: [AST] RecursiveASTVisitor visits lambda classes when implicit visitation is on.

JonasToth added a comment. >> Is the suggestion to make that change, but then modify the semantics of the >> `functionDecl()` etc matchers to hide it? Or something else? > > My suggestion is to extract the traverser from ASTDumper first, then fix this > issue. I understand that you are working

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

JonasToth added a comment. > I do not have commit rights. I'm not sure what constitutes someone who can > commit, but let me contribute a little more before taking that step, I have > another idea for a checker I'd like to try after this one, I just wanted to > get one under my belt first. Se

[PATCH] D56444: [AST] RecursiveASTVisitor visits lambda classes when implicit visitation is on.

JonasToth added a comment. @sammccall I (hopefully) fixed the type-issue in my test-cases. They nevertheless fail both on me, on latest llvm+clang+your patch. Clang-tidy still the same, so maybe there is a difference in our builds? Do you build with assertions on? I work on linux, ubuntu 18.04,

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

This revision was automatically updated to reflect the committed changes. Closed by commit rL350760: [clang-tidy] Adding a new modernize use nodiscard checker (authored by JonasToth, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D5543

[PATCH] D56444: [AST] RecursiveASTVisitor visits lambda classes when implicit visitation is on.

JonasToth added a comment. In D56444#1353003 , @sammccall wrote: > In D56444#1351714 , @JonasToth wrote: > > > @sammccall I (hopefully) fixed the type-issue in my test-cases. They > > nevertheless fail both on me,

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