[PATCH] D56926: [Documentation] Use HTTPS whenever possible in clang-tools-extra

JonasToth added a comment. In D56926#1363648 , @MyDeveloperDay wrote: > the closest I can see is > > https://www.perforce.com/resources/qac/high-integrity-cpp-coding-standard#statements > > which take you to section 6 of the standard, but I see no id or

[PATCH] D56926: [Documentation] Use HTTPS whenever possible in clang-tools-extra

JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land. very good idea. This reminds me, that i wanted to fix the hicpp links, as they restructured their website. thanks ;) Changes LGTM! Repository: rCTE Clang Tools Extra CHANGES SINCE

[PATCH] D56610: [clangd] A code action to qualify an unqualified name

JonasToth added a comment. In D56610#1363461 , @ilya-biryukov wrote: > In D56610#1363408 , @JonasToth wrote: > > > Is this for something like `add const`? > > If yes, there is clang-tidy effort on that, see > >

[PATCH] D56918: [clang-tidy] add reproducer for PR39949 into test-suite

This revision was automatically updated to reflect the committed changes. Closed by commit rL351569: [clang-tidy] add reproducer for PR39949 into test-suite (authored by JonasToth, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGES SINCE LAST ACTION

[PATCH] D56610: [clangd] A code action to qualify an unqualified name

JonasToth added a comment. Is this for something like `add const`? If yes, there is clang-tidy effort on that, see https://reviews.llvm.org/D54943 and https://reviews.llvm.org/D54395 for a similar effort. Would be best to share the code instead of reinventing it :) CHANGES SINCE LAST ACTION

[PATCH] D56918: [clang-tidy] add reproducer for PR39949 into test-suite

JonasToth created this revision. JonasToth added reviewers: alexfh, aaron.ballman, hokein, hwright. Herald added subscribers: cfe-commits, xazax.hun. The underlying issue is fixed in https://reviews.llvm.org/D56444 and this test ensures the issue does not creep back into our code-base.

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

JonasToth created this revision. JonasToth added reviewers: aaron.ballman, sammccall, rsmith. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, a.sidorin, baloghadamsoftware. Herald added a reviewer: george.karpenkov. This patch adds two unit-tests that are the result of

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

JonasToth added a comment. In D54141#1362924 , @MyDeveloperDay wrote: > > LLVM is very chatty as well, I don't consider LLVM to be a bad code-base. > > Take readability-braces-around-statements for example. > > Do we need a

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

JonasToth added a comment. I believe there is some sort of memory leak in the `run-clang-tidy` or so. I had similar experience :) Take this with a grain of salt, as I don't recall all details: `run-clang-tidy.py` just takes all output from clang-tidy and prints it. A lot is redundant due to

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

JonasToth updated this revision to Diff 182359. JonasToth added a comment. Herald added a reviewer: serge-sans-paille. - make the script more useable in my buildbot context - reduce the test-files - fix unicode issues I encountered while using Repository: rCTE Clang Tools Extra CHANGES SINCE

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

JonasToth updated this revision to Diff 182356. JonasToth added a comment. - avoid bitrot Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D45444/new/ https://reviews.llvm.org/D45444 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt

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

JonasToth updated this revision to Diff 182355. JonasToth added a comment. - accidentally wrong patch uploaded Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54943/new/ https://reviews.llvm.org/D54943 Files:

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

JonasToth updated this revision to Diff 182354. JonasToth added a comment. avoid bitrot Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54943/new/ https://reviews.llvm.org/D54943 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt

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

JonasToth updated this revision to Diff 182353. JonasToth added a comment. - avoid bitrot Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D40854/new/ https://reviews.llvm.org/D40854 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt

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

JonasToth updated this revision to Diff 182345. JonasToth added a comment. - generalize to DeclSpec::TQ - add dependent type-tests with `typename` Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54395/new/ https://reviews.llvm.org/D54395 Files:

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

JonasToth updated this revision to Diff 181877. JonasToth added a comment. - Merge branch 'master' into check_mixed_arithmetic, a lot has happened... Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D40854/new/ https://reviews.llvm.org/D40854 Files:

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

JonasToth added inline comments. Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:33 +const LangOptions ) { + // we start with the location of the closing parenthesis. + const TypeSourceInfo *TSI = F.getTypeSourceInfo(); Nit: s/we/We

[PATCH] D56585: [clang-tidy] Treat references to smart pointers correctly in use-after-move.

JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land. LGTM, is there a bug report for this issue? If yes please close that too :) Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56585/new/

[PATCH] D56563: [clang-tidy] add options documentation to readability-identifier-naming checker

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/D56563/new/ https://reviews.llvm.org/D56563 ___ cfe-commits mailing list

[PATCH] D56563: [clang-tidy] add options documentation to readability-identifier-naming checker

JonasToth added a comment. That looks good. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56563/new/ https://reviews.llvm.org/D56563 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D56563: [clang-tidy] add options documentation to readability-identifier-naming checker

JonasToth added a comment. Thank you very much for working on this! See https://bugs.llvm.org/show_bug.cgi?id=34990 for the bug report, I mentionend this revision. TBH i did not read through all the document, but scrolled mostly starting from 25%, it looks good to me. Given its length, what do

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

JonasToth added a comment. In D55433#1357483 , @MyDeveloperDay wrote: > In D55433#1351707 , @JonasToth wrote: > > > > I do not have commit rights. I'm not sure what constitutes someone who > > > can commit, but

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

JonasToth added a comment. Thank you for working on this! Could you please run the check over LLVM or any other significant codebase once you have the fix implemented and report if the transformation still breaks code? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION

[PATCH] D56644: [clang-tidy] readability-container-size-empty handle std::string length()

JonasToth added a comment. Could you please run it over LLVM or another significant codebase and check if there are miss-transformations (run with fix and then compile (+ tests optimally))? You can use `clang-tidy/tool/run-clang-tidy.py` for a parallel runner. Repository: rCTE Clang Tools

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

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

[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

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.

[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

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

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

[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

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

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

[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

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

[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

[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 added inline comments. Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:464 bool Value, StringRef Id) { - auto SimpleThen = binaryOperator( - hasOperatorName("="), -

[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

[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

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

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

[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] 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 ) { + // Try to catch both std::function and boost::function

[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

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

[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 ) { + return

[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

[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

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

[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

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

[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

[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

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

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

[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

[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

[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

[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

[PATCH] D55561: Stop stripping comments from AST matcher example code

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/D55561/new/ https://reviews.llvm.org/D55561 ___ cfe-commits mailing list

[PATCH] D55541: Use the standard Duration factory matcher

JonasToth added a comment. In D55541#1326868 , @lebedev.ri wrote: > In D55541#1326867 , @JonasToth wrote: > > > Please remember to upload your patches with full context (i can highly > > recommend using `arc`, see

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

JonasToth added a comment. See PR39949 as well, as it seems to trigger the same or a similar problem. @ioeric and @klimek maybe could give an opinion, too? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54309/new/ https://reviews.llvm.org/D54309

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

JonasToth added inline comments. Comment at: clang-tidy/abseil/DurationRewriter.cpp:77 + getInverseForScale(Scale); + if (const auto *MaybeCallArg = selectFirst( + "e", hwright wrote: > JonasToth wrote: > > In Principle the `Node` could have

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

JonasToth added inline comments. Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:29 +AST_MATCHER(CXXMethodDecl, isOverloadedOperator) { + // Don't put [[nodiscard]] front of operators. + return Node.isOverloadedOperator(); s/front/in front/

[PATCH] D55508: [clang-tidy] insert release notes for new checkers alphabetically

JonasToth added a comment. In D55508#1325960 , @Eugene.Zelenko wrote: > By the word, will be good idea to have script which check alphabetical order > and use it during build. Sometimes alphabetical order may be violated after > merge with trunk. I

[PATCH] D55508: [clang-tidy] insert release notes for new checkers alphabetically

This revision was automatically updated to reflect the committed changes. Closed by commit rL348793: [clang-tidy] insert release notes for new checkers alphabetically (authored by JonasToth, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D55508: [clang-tidy] insert release notes for new checkers alphabetically

JonasToth added a comment. In D55508#1325834 , @MyDeveloperDay wrote: > In D55508#1325758 , @JonasToth wrote: > > > LGTM. Do you have commit access? > > > I do not I'm afraid I will commit for you. CHANGES

[PATCH] D55508: [clang-tidy] insert release notes for new checkers alphabetically

JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land. LGTM. Do you have commit access? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55508/new/ https://reviews.llvm.org/D55508 ___

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

JonasToth added a comment. Hi MyDeveloperDay, thanks for the patch! Mostly stylistic comments. Would it make sense to attach the attribute to the implementation of the functions too? This check is definitly useful, but noisy. Do you see a change of another heuristic that could be applied to

[PATCH] D55508: [clang-tidy] insert release notes for new checkers alphabetically

JonasToth added a comment. I think this patch is fine, AFAIK these utility scripts are not tested directly but are just adjusted if they dont work as expected :) Did you test it with a fake new-check? If it does what we expect its fine :) CHANGES SINCE LAST ACTION

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

JonasToth added inline comments. Comment at: clang-tidy/abseil/DurationRewriter.cpp:21 +struct DurationScale2IndexFunctor { + using argument_type = DurationScale; + unsigned operator()(DurationScale Scale) const { Are you using `argument_type`? Browser

[PATCH] D55508: [clang-tidy] insert release notes for new checkers alphabetically

JonasToth added inline comments. Comment at: clang-tidy/add_new_check.py:213 match = re.search('Improvements to clang-tidy', line) +match_next = re.search('Improvements to include-fixer', line) +match_checker = re.search('- New :doc:`(.*)', line)

[PATCH] D55409: [clang-tidy] check for using declarations not in an anonymous namespace when there exists one

JonasToth added a comment. Please upload with full context. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55409/new/ https://reviews.llvm.org/D55409 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D55410: [clang-tidy] check for flagging using declarations in headers

JonasToth added a comment. Please upload with full context. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55410/new/ https://reviews.llvm.org/D55410 ___ cfe-commits mailing list

[PATCH] D55411: [clang-tidy] check for flagging using declarations not in the inner most namespace

JonasToth added a comment. Please upload the patch with full context. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55411/new/ https://reviews.llvm.org/D55411 ___ cfe-commits mailing list

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

JonasToth added a comment. Hi Sam, this patch "broke" `ExprMutAnalyzer`, at least it creates an assertion failure that seems harmless. Your thought on it would be great! The assertion in: `ASTMatchFinder.cpp +680` is triggered in the Analysis to figure out if something is mutated, because

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

JonasToth added a comment. In D55245#1320546 , @hwright wrote: > Ping. > > I assume I've got the right reviewers here, but I've also been sending a > bunch of stuff your way lately, so if I'm overwhelming review capacity, > please let me know. Hi

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

JonasToth marked 2 inline comments as done. JonasToth added inline comments. Comment at: clang-tidy/utils/FixItHintUtils.cpp:35 +static bool isValueType(QualType QT) { return isValueType(QT.getTypePtr()); } +static bool isArrayType(QualType QT) { return isa(QT.getTypePtr()); }

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

JonasToth added a comment. I had to revert this patch because it broke (at least one) buildbot with an assertion-failure (http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/40436/steps/test/logs/stdio) Could you please take a look at it? I could not reproduce

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

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE348343: [clang-tidy] new check: bugprone-branch-clone (authored by JonasToth, committed by ). Changed prior to commit: https://reviews.llvm.org/D54757?vs=176408=176772#toc Repository: rCTE Clang

[PATCH] D55255: Fix a false positive in misplaced-widening-cast

JonasToth added a comment. Committed, Thank you for the patch! Was there a bug-report for this issue? If yes can you please close it/reference? Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55255/new/ https://reviews.llvm.org/D55255

[PATCH] D55255: Fix a false positive in misplaced-widening-cast

This revision was automatically updated to reflect the committed changes. Closed by commit rL348341: Fix a false positive in misplaced-widening-cast (authored by JonasToth, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

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

JonasToth added a comment. > A quick, tangentially related idea: Would it be useful to implement multiline > nolint sections (e.g. `//BEGINNOLINT` – `//ENDNOLINT` or something similar)? > This would be helpful for using clang-tidy on projects that contain some > automatically generated

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

JonasToth added a comment. > `int8` ? Did you mean `int8_t` or am I missing somthing ? Your right, but the solution I wrote did actually not work anyway.. I just specialized `std::hash<>` now. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54737/new/

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

JonasToth added a comment. I had to revert and recommitted in rCTE348169 . `std::unordered_map` does not work, as `std::hash` is not specialized for it. This behaviour seems to work for some compilers, but some not. I had to google myself a bit for the best

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL348161: [clang-tidy] Add the abseil-duration-comparison check (authored by JonasToth, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D54881: [clang-format] Prevent Clang-Format from editing leading whitespace on lines outside of the format range

JonasToth added a comment. Hi russell thank you for the patch! As I am not a clang-format reviewers these are only general things, and Nits anyway ;) Hope the reviewers I added will evaluate better. Comment at: lib/Format/WhitespaceManager.cpp:54 Tok.Decision = (Newlines

[PATCH] D55006: [clang] - Simplify tools::SplitDebugName

JonasToth added a comment. from my side no objections, mailing list did not react AFAIK (just in case your waiting for me until you recommit). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55006/new/ https://reviews.llvm.org/D55006 ___

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

JonasToth added a comment. In D54757#1316899 , @donat.nagy wrote: > I applied this check to the llvm + clang codebase, and I was surprised to see > that it produced about 600 results (for comparison, the clang-tidy checks > which are enabled by default

[PATCH] D55125: [clang-tidy] Fix a false positive in misc-redundant-expression check

JonasToth added inline comments. Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:605 + return strncmp(SM.getCharacterData(T1.getLocation()), + SM.getCharacterData(T2.getLocation()), T1.getLength()) == 0; +} This operation could overflow

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

JonasToth added a comment. In D55044#1312438 , @lebedev.ri wrote: > Thanks for working on this. > Semi-duplicate of D50852 Please see > discussion there. > It should not be an abseil-specific check, the prefix

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