[PATCH] D45679: [clang-tidy] Add ExprMutationAnalyzer, that analyzes whether an expression is mutated within a statement.

JonasToth added a comment. @shuaiwang What are you working on next? Do you have an idea on how we will handle the pointee of a pointer? That would be very interesting for my const-correctness check. if you need help with anything, just say so. I would like to support Am 10.09.2018 um 18:49

[PATCH] D51220: [clang-tidy] run-clang-tidy fails using python 3.7

JonasToth added a comment. Unicode works both with python3 and python2 (but seemed to work before too). Otherwise LG Comment at: clang-tidy/tool/run-clang-tidy.py:169 failed_files.append(name) - -if is_py2: The patch does not apply clean to master

[PATCH] D50619: [clang-tidy] Handle unresolved expressions in ExprMutationAnalyzer

JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land. Ups, sorry i overlooked. I applied your changes to the current version of the const check, and everything seems fine. The false negative is gone. Repository: rCTE Clang Tools Extra

[PATCH] D51294: Fix Bug 38713: clang-format mishandles a short block after "default:" in a switch statement

This revision was automatically updated to reflect the committed changes. Closed by commit rL341284: Fix Bug 38713: clang-format mishandles a short block after default: in a… (authored by JonasToth, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D51294: Fix Bug 38713: clang-format mishandles a short block after "default:" in a switch statement

JonasToth added a comment. Commited for Owen in r341284. Repository: rC Clang https://reviews.llvm.org/D51294 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51381: [clang-tidy] fix check_clang_tidy to forbid mixing of CHECK-NOTES and CHECK-MESSAGES

This revision was automatically updated to reflect the committed changes. Closed by commit rL341039: [clang-tidy] fix check_clang_tidy to forbid mixing of CHECK-NOTES and CHECK… (authored by JonasToth, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM

[PATCH] D48714: [clang-tidy] fix PR37913, templated exception factory diagnosed correctly

JonasToth updated this revision to Diff 163274. JonasToth added a comment. - [Misc] migrate to CHECK-NOTES - fix outcommented check messages as well Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48714 Files: clang-tidy/hicpp/ExceptionBaseclassCheck.cpp

[PATCH] D51381: [clang-tidy] fix check_clang_tidy to properly mix CHECK-NOTES and CHECK-MESSAGES

JonasToth updated this revision to Diff 163273. JonasToth added a comment. - adjust check_clang_tidy to use either CHECK-NOTES or CHECK-MESSAGES but not both Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51381 Files: test/clang-tidy/check_clang_tidy.py Index:

[PATCH] D51381: [clang-tidy] fix check_clang_tidy to properly mix CHECK-NOTES and CHECK-MESSAGES

JonasToth added a comment. Yes, you are write. I would ensure in the script that `CHECK-MESSAGES XOR CHECK-NOTES` is used, to avoid the mistake i made. I can do it in this diff as well. Am 29.08.2018 um 20:29 schrieb Roman Lebedev via Phabricator: > lebedev.ri added a comment. > > In

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

JonasToth added a comment. Ok. Then we can commit for you guys, no problem :) Am 29.08.2018 um 16:34 schrieb Hugo Gonzalez via Phabricator: > hugoeg added a comment. > > In https://reviews.llvm.org/D50542#1217517, @JonasToth wrote: > >> Committed r340928. Thanks very much! >> >> Will you

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

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

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

JonasToth added a comment. Committed r340928. Thanks very much! Will you (and your team) upstream even more abseil checks? I think it might be reasonable to ask for commit rights. https://reviews.llvm.org/D50542 ___ cfe-commits mailing list

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

JonasToth added a comment. In https://reviews.llvm.org/D50542#1217511, @hugoeg wrote: > nit comment fixed DO you have commit rights or shall i commit for you? https://reviews.llvm.org/D50542 ___ cfe-commits mailing list

[PATCH] D48714: [clang-tidy] fix PR37913, templated exception factory diagnosed correctly

JonasToth added a comment. In https://reviews.llvm.org/D48714#1216989, @lebedev.ri wrote: > In https://reviews.llvm.org/D48714#1216537, @JonasToth wrote: > > > I had to revert the `CHECK-NOTES` change that @lebedev.ri introduced with > > his revision. It fails the test, i think there is an

[PATCH] D51381: [clang-tidy] fix check_clang_tidy to properly mix CHECK-NOTES and CHECK-MESSAGES

JonasToth added a comment. @lebedev.ri lets do it in the the other patch, to not split discussions. But what do you mean by `You would still have to duplicate the check-lines for error: though.`? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51381

[PATCH] D51381: [clang-tidy] fix check_clang_tidy to properly mix CHECK-NOTES and CHECK-MESSAGES

JonasToth created this revision. JonasToth added reviewers: alexfh, aaron.ballman, lebedev.ri, hokein. Herald added subscribers: cfe-commits, xazax.hun. This patch adjusts the check_clang_tidy.py script to tolerate warnings in the codepath that checks for notes. Checking for warnings itself

[PATCH] D48714: [clang-tidy] fix PR37913, templated exception factory diagnosed correctly

JonasToth updated this revision to Diff 162950. JonasToth added a comment. - [Test] use CHECK-NOTES again based on the fix in check_clang_tidy Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48714 Files: clang-tidy/hicpp/ExceptionBaseclassCheck.cpp

[PATCH] D48714: [clang-tidy] fix PR37913, templated exception factory diagnosed correctly

JonasToth added a subscriber: lebedev.ri. JonasToth added a comment. I had to revert the `CHECK-NOTES` change that @lebedev.ri introduced with his revision. It fails the test, i think there is an inconsistency or so in the check-clang-tidy script. I will try to figure out whats the issue.

[PATCH] D48714: [clang-tidy] fix PR37913, templated exception factory diagnosed correctly

JonasToth updated this revision to Diff 162945. JonasToth added a comment. - [Misc] comment the matcher to better understand it - [Fix] adjust the test cases to properly function now Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48714 Files:

[PATCH] D51332: [clang-tidy] Replace deprecated std::ios_base aliases

JonasToth added inline comments. Comment at: clang-tidy/modernize/DeprecatedIosBaseAliasesCheck.cpp:34 + // provide any benefit to other languages, despite being benign. + if (!getLangOpts().CPlusPlus) +return; Which standard supplies the replacement

[PATCH] D51332: [clang-tidy] Replace deprecated std::ios_base aliases

JonasToth added a comment. Hi andobence, thank you for the contribution. The check looks very good! Please add it to the release notes and synchronize the first line of the doc with the short sentence in the release notes describing what this check does. Comment at:

[PATCH] D51297: [docs] Create a guide for Vim users on how to setup Clangd

JonasToth added a comment. Very nice :) I will try to get it running for vim 8.0 on ubuntu 18.04 (hopefully this week!) and share if i had success :) Am 27.08.2018 um 16:57 schrieb Kirill Bobyrev via Phabricator: > kbobyrev added a comment. > > In https://reviews.llvm.org/D51297#1214321,

[PATCH] D51297: [docs] Create a guide for Vim users on how to setup Clangd

JonasToth added a comment. Very nice! Switching to clangd means that YCM is not necessary anymore? https://reviews.llvm.org/D51297 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

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

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

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

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

JonasToth updated this revision to Diff 162449. JonasToth added a comment. - get up to date Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45444 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp

[PATCH] D48714: [clang-tidy] fix PR37913, templated exception factory diagnosed correctly

JonasToth updated this revision to Diff 162448. JonasToth added a comment. - Merge branch 'master' into fix_exception Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48714 Files: clang-tidy/hicpp/ExceptionBaseclassCheck.cpp test/clang-tidy/hicpp-exception-baseclass.cpp

[PATCH] D50697: [clang-format] fix PR38557 - comments between "default" and ':' causes the case label to be treated as an identifier

This revision was automatically updated to reflect the committed changes. Closed by commit rC340624: [clang-format] fix PR38557 - comments between default and : causes the case… (authored by JonasToth, committed by ). Repository: rC Clang https://reviews.llvm.org/D50697 Files:

[PATCH] D50699: [clang-format] fix PR38525 - Extraneous continuation indent spaces with BreakBeforeBinaryOperators set to All

This revision was automatically updated to reflect the committed changes. Closed by commit rC340623: [clang-format] fix PR38525 - Extraneous continuation indent spaces with… (authored by JonasToth, committed by ). Repository: rC Clang https://reviews.llvm.org/D50699 Files:

[PATCH] D51220: [clang-tidy] run-clang-tidy fails using python 3.7

JonasToth added a comment. LG from my side, but please let @aaron.ballman or @alexfh approve first Am 24.08.2018 um 18:08 schrieb Andi via Phabricator: > Abpostelnicu added a comment. > > I can confirm tested on: > 2.7.15 > 3.7.0 > > On both it worked. > > -

[PATCH] D51220: [clang-tidy] run-clang-tidy fails using python 3.7

JonasToth added a comment. Could you please verify it is actually working? Am 24.08.2018 um 18:05 schrieb Andi via Phabricator: > Abpostelnicu added a comment. > > In https://reviews.llvm.org/D51220#1212463, @JonasToth wrote: > >> I don't know a lot about how to write portable code! >> >>

[PATCH] D51220: [clang-tidy] run-clang-tidy fails using python 3.7

JonasToth added a comment. I don't know a lot about how to write portable code! But wouldn't this be out usecase? http://python-future.org/compatible_idioms.html#file-io-with-open Using the `decode` is supposed to work in both pythons Repository: rCTE Clang Tools Extra

[PATCH] D49851: [clang-tidy] run-clang-tidy add synchronisation to the output

JonasToth added a comment. Please dont work in this revision but create a new one (as this one is already committed)! Am 24.08.2018 um 17:34 schrieb Andi via Phabricator: > Abpostelnicu updated this revision to Diff 162387. > > https://reviews.llvm.org/D49851 > > Files: > >

[PATCH] D50699: [clang-format] fix PR38525 - Extraneous continuation indent spaces with BreakBeforeBinaryOperators set to All

JonasToth added a comment. Owen, shall i commit for you? Repository: rC Clang https://reviews.llvm.org/D50699 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48714: [clang-tidy] fix PR37913, templated exception factory diagnosed correctly

JonasToth added a comment. ping @alexfh what is your take on the issue? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48714 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D51061: [clang-tidy] abseil-str-cat-append

JonasToth added inline comments. Comment at: clang-tidy/abseil/StrCatAppendCheck.cpp:21 +namespace { + +// Skips any combination of temporary materialization, temporary binding and I think you can elide the empty line around the matcher.

[PATCH] D51061: [clang-tidy] abseil-str-cat-append

JonasToth added inline comments. Comment at: test/clang-tidy/abseil-str-cat-append.cpp:91 + +void bar() { + std::string a, b; hugoeg wrote: > JonasToth wrote: > > What happens if `StrCat` is used e.g. in an `std::accumulate` call as the > > binary operator?

[PATCH] D51061: [clang-tidy] abseil-str-cat-append

JonasToth added inline comments. Comment at: clang-tidy/abseil/StrCatAppendCheck.cpp:28 + for (;;) { +if (const auto *MTE = dyn_cast(E)) { + E = MTE->getTemporary(); You can ellide the braces for the single stmt ifs Comment at:

[PATCH] D50617: [ASTMatchers] Let hasObjectExpression also support UnresolvedMemberExpr, CXXDependentScopeMemberExpr

JonasToth added a comment. The fix was effective, the assertion does not fire anymore on the `ItaniumDemangle.cpp` file Repository: rC Clang https://reviews.llvm.org/D50617 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D50862: [clang-tidy] Abseil: faster strsplit delimiter check

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

[PATCH] D50619: [clang-tidy] Handle unresolved expressions in ExprMutationAnalyzer

JonasToth added a comment. Sure, thanks for the fast fix :) Am 22.08.2018 um 00:04 schrieb Shuai Wang via Phabricator: > shuaiwang added a comment. > > In https://reviews.llvm.org/D50619#1207785, @JonasToth wrote: > >> @shuaiwang Unfortunatly the analysis does not pass and fails on an

[PATCH] D50619: [clang-tidy] Handle unresolved expressions in ExprMutationAnalyzer

JonasToth added a comment. @shuaiwang Unfortunatly the analysis does not pass and fails on an assertion → ~/opt/llvm/build/bin/clang-tidy -checks=-*,cppcoreguidelines-const-correctness ItaniumDemangle.cpp -- clang-tidy: ../tools/clang/include/clang/AST/ExprCXX.h:3581: clang::Expr*

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

JonasToth added a comment. In https://reviews.llvm.org/D50542#1205880, @hugoeg wrote: > Anymore changes I should make or issues I should be aware of? From my side not! You still have unresolved comments that cause the revision to not be `Ready To Review` for the main reviewers, maybe that

[PATCH] D50862: [clang-tidy] Abseil: faster strsplit delimiter check

JonasToth added inline comments. Comment at: clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp:54 + // Now replace the " with '. + auto Pos = Result.find_first_of('"'); + if (Pos == Result.npos) deannagarcia wrote: > JonasToth wrote: > > deannagarcia wrote:

[PATCH] D50862: [clang-tidy] Abseil: faster strsplit delimiter check

JonasToth added inline comments. Comment at: clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp:85 + + // absl::StrSplit(..., "x") -> absl::StrSplit(..., 'x') + // absl::ByAnyChar("x") -> 'x' Maybe you could make these comments into full sentences. I do think

[PATCH] D50883: [clang-tidy] Handle unique owning smart pointers in ExprMutationAnalyzer

JonasToth added a comment. I see, thank you :) Am 17.08.2018 um 18:55 schrieb Shuai Wang via Phabricator: > shuaiwang added a comment. > > In https://reviews.llvm.org/D50883#1203690, @JonasToth wrote: > >> I am suprised that this does not automatically follow from the general >> rules. At

[PATCH] D50883: [clang-tidy] Handle unique owning smart pointers in ExprMutationAnalyzer

JonasToth added a comment. I am suprised that this does not automatically follow from the general rules. At the end, smartpointers cant do anything else then 'normal' classes. The `operator+/->` were not handled before? The mutation of `SmartPtr x; x->mf();` should already be catched, not?

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

JonasToth added inline comments. Comment at: clang-tidy/abseil/NoInternalDepsCheck.cpp:21 +bool IsInAbseilFile(const SourceManager& manager, SourceLocation loc){ + if (loc.isInvalid()) { +return false; You can elide the braces for single stmt ifs

[PATCH] D49851: [clang-tidy] run-clang-tidy add synchronisation to the output

JonasToth added a comment. In https://reviews.llvm.org/D49851#1202764, @Abpostelnicu wrote: > Strangely I haven't had those kind of issues but since you propose those > modifications I would do one more modification, let's output everything to > stdout and not stderr by doing: > > if err: >

[PATCH] D50852: [clang-tidy] abseil-auto-make-unique

JonasToth added inline comments. Comment at: test/clang-tidy/abseil-auto-make-unique.cpp:73 + // Different type. No change. + std::unique_ptr z = make_unique(); + std::unique_ptr z2(make_unique()); JonasToth wrote: > lets consider a 3 level class hierarchy. >

[PATCH] D50862: [clang-tidy] Abseil: faster strsplit delimiter check

JonasToth added inline comments. Comment at: clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp:24 + +ast_matchers::internal::Matcher +constructExprWithArg(llvm::StringRef ClassName, you dont need the `ast_matchers` namespace as there is a `using namespace

[PATCH] D50852: [clang-tidy] abseil-auto-make-unique

JonasToth added inline comments. Comment at: clang-tidy/abseil/AutoMakeUniqueCheck.cpp:21 +void AutoMakeUniqueCheck::registerMatchers(MatchFinder* finder) { + if (!getLangOpts().CPlusPlus) return; + Please clang-format, `return` on next line.

[PATCH] D50852: [clang-tidy] abseil-auto-make-unique

JonasToth added a comment. In https://reviews.llvm.org/D50852#1202774, @lebedev.ri wrote: > 1. Please always upload all patches with full context. > 2. There already is `modernize-use-auto`. Does it handle this case? Then this > should be just an alias to that check. Else, i think it would be

[PATCH] D48714: [clang-tidy] fix PR37913, templated exception factory diagnosed correctly

JonasToth added inline comments. Comment at: test/clang-tidy/hicpp-exception-baseclass.cpp:191 +void templated_thrower() { throw T{}(); } +// CHECK-MESSAGES: [[@LINE-1]]:34: warning: throwing an exception whose type 'int' is not derived from 'std::exception' +

[PATCH] D50580: [clang-tidy] Abseil: no namespace check

JonasToth added a comment. That sounds good as well, just not in clang and best in `clang-tidy/utils` > `ASTMatchers.h` is not a reasonable place to put `asseil`-specific matchers. > We have `clang-tidy/utils/Matchers.h` for putting clang-tidy specific > matchers. I'm not sure whether it is

[PATCH] D50619: [clang-tidy] Handle unresolved expressions in ExprMutationAnalyzer

JonasToth added a comment. @shuaiwang i tried to apply this and check the clang-tidy part again, but it does not compile (log attached). I update clang to master, did you add a matcher or something like this? F6950472: error.log Repository: rCTE Clang

[PATCH] D50619: [clang-tidy] Handle unresolved expressions in ExprMutationAnalyzer

JonasToth added a comment. You are totally right. Am 16.08.2018 um 02:41 schrieb Shuai Wang via Phabricator: > shuaiwang added inline comments. > > > Comment at: unittests/clang-tidy/ExprMutationAnalyzerTest.cpp:309 > > +TEST(ExprMutationAnalyzerTest, CallUnresolved) { > +

[PATCH] D50619: [clang-tidy] Handle unresolved expressions in ExprMutationAnalyzer

JonasToth added a comment. I see, thank you for the clarification :) Am 15.08.2018 um 19:25 schrieb Shuai Wang via Phabricator: > shuaiwang added inline comments. > > > Comment at: unittests/clang-tidy/ExprMutationAnalyzerTest.cpp:410 > +

[PATCH] D50619: [clang-tidy] Handle unresolved expressions in ExprMutationAnalyzer

JonasToth added inline comments. Comment at: unittests/clang-tidy/ExprMutationAnalyzerTest.cpp:117 +TEST(ExprMutationAnalyzerTest, AssumedNonConstMemberFunc) { + auto AST = tooling::buildASTFromCode( I think you could add another test with `X x` (if you don't

[PATCH] D48714: [clang-tidy] fix PR37913, templated exception factory diagnosed correctly

JonasToth added inline comments. Comment at: test/clang-tidy/hicpp-exception-baseclass.cpp:191 +void templated_thrower() { throw T{}(); } +// CHECK-MESSAGES: [[@LINE-1]]:34: warning: throwing an exception whose type 'int' is not derived from 'std::exception' +

[PATCH] D50699: [clang-format] fix PR38525 - Extraneous continuation indent spaces with BreakBeforeBinaryOperators set to All

JonasToth added a comment. In https://reviews.llvm.org/D50699#1198813, @owenpan wrote: > Updated patch created by "svn diff --diff-cmd=diff -x -U99" The patch seems not to be in the correct path (the `lib/Format` thing is missing). Could you check that again? If you plan to add more

[PATCH] D50699: Extraneous continuation indent spaces with BreakBeforeBinaryOperators set to All

JonasToth added a comment. Please create the diffs with full context (git diff -U 999) and add an test-case. Repository: rC Clang https://reviews.llvm.org/D50699 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D50697: [clang-format] fix PR38557 - comments between "default" and ':' causes the case label to be treated as an identifier

JonasToth updated this revision to Diff 160550. JonasToth added a comment. Using git for the diff, add testcase Repository: rC Clang https://reviews.llvm.org/D50697 Files: lib/Format/UnwrappedLineParser.cpp unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp

[PATCH] D50697: [clang-format] fix PR38557 - comments between "default" and ':' causes the case label to be treated as an identifier

JonasToth created this revision. JonasToth added reviewers: hokein, krasimir, djasper, klimek. The Bug was reported and fixed by Owen Pan. Repository: rC Clang https://reviews.llvm.org/D50697 Files: UnwrappedLineParser.cpp Index: UnwrappedLineParser.cpp

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

JonasToth added inline comments. Comment at: test/clang-tidy/abseil-no-internal-deps.cpp:5 + +void DirectAcess(){ + absl::strings_internal::InternalFunction(); Please run clang-format over the test code as well. The braces here in `FriendUsage` miss a space.

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

JonasToth added inline comments. Comment at: test/clang-tidy/abseil-no-internal-deps.cpp:2 +// RUN: %check_clang_tidy %s abseil-no-internal-deps %t + + hugoeg wrote: > JonasToth wrote: > > hugoeg wrote: > > > hokein wrote: > > > > nit: please make sure the code

[PATCH] D50580: [clang-tidy] Abseil: no namespace check

JonasToth added inline comments. Comment at: clang-tidy/abseil/NoNamespaceCheck.cpp:23 + + Finder->addMatcher(namespaceDecl(hasName("absl")).bind("absl_namespace"), + this); hugoeg wrote: > deannagarcia wrote: > > aaron.ballman wrote: > > >

[PATCH] D50619: [clang-tidy] Handle unresolved expressions in ExprMutationAnalyzer

JonasToth added inline comments. Comment at: unittests/clang-tidy/ExprMutationAnalyzerTest.cpp:454 + + AST = + tooling::buildASTFromCode("template void f() { T x; x.y.z; }"); JonasToth wrote: > Is there already a test for a method from a templated type? >

[PATCH] D50619: [clang-tidy] Handle unresolved expressions in ExprMutationAnalyzer

JonasToth added a comment. Please add a test for the same usecase as in Sema. template struct SizeIndicator { constexpr int value = 8; }; template <> struct SizeIndicator { constexpr int value = 4; }; template void fooFunction() { char

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

JonasToth added inline comments. Comment at: clang-tidy/abseil/NoInternalDepsCheck.h:21 +/// against doing so. This check should not be run on internal Abseil files or +///Abseil source code. +/// double blank Comment at:

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

JonasToth added inline comments. Comment at: test/clang-tidy/abseil-no-internal-deps.cpp:2 +// RUN: %check_clang_tidy %s abseil-no-internal-deps %t + + hugoeg wrote: > hokein wrote: > > nit: please make sure the code follow LLVM code style, even for test code :)

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

JonasToth added a comment. Thank you very much :) Am 12.08.2018 um 00:17 schrieb Shuai Wang via Phabricator: > shuaiwang added a comment. > > In https://reviews.llvm.org/D45444#1196271, @JonasToth wrote: > >> Always the same with the templates ;) So uninstantiated templates should >> >>

[PATCH] D48714: [clang-tidy] fix PR37913, templated exception factory diagnosed correctly

JonasToth updated this revision to Diff 160242. JonasToth added a comment. - update to current master of clang introduce CHECK-NOTES - use new BeginLoc api Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48714 Files: clang-tidy/hicpp/ExceptionBaseclassCheck.cpp

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

JonasToth updated this revision to Diff 160239. JonasToth added a comment. - explicitly ignore lambdas in VarDecls Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45444 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp

[PATCH] D50580: [clang-tidy] Abseil: no namespace check

JonasToth added a comment. Could it happen that some template specializations or so need to land in `absl`? Comment at: clang-tidy/abseil/NoNamespaceCheck.cpp:28 +void NoNamespaceCheck::check(const MatchFinder::MatchResult ) { + const auto *decl =

[PATCH] D50447: [clang-tidy] Omit cases where loop variable is not used in loop body in performance-for-range-copy.

JonasToth added a comment. Could there even be a false positive? In a sense, the variable is never used, so it does not matter, not? Am 10.08.2018 um 22:17 schrieb Shuai Wang via Phabricator: > shuaiwang added a comment. > > In https://reviews.llvm.org/D50447#1194967, @JonasToth wrote: > >>

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

JonasToth added a comment. Always the same with the templates ;) So uninstantiated templates should just be ignored. I think it would be better to have it in the ExprMutAnalyzer, because that part can not decide on const-ness. Fixing it here would just circumvent the problem but not fix it,

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

JonasToth added a comment. No concerns except the small nits from my side. Comment at: clang-tidy/abseil/NoInternalDepsCheck.cpp:24 + + // TODO(hugoeg): refactor matcher to be configurable or just match on any internal access from outside the enclosing namespace. +

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

JonasToth added a comment. Thank you for your first contribution to LLVM btw :) Comment at: clang-tidy/abseil/NoInternalDepsCheck.cpp:24 + + TODO(hugoeg): refactor matcher to be configurable or just match on any internal access from outside the enclosing namespace. +

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

JonasToth added inline comments. Comment at: clang-tidy/abseil/NoInternalDepsCheck.cpp:24 + + Finder->addMatcher( + nestedNameSpecifierLoc(loc(specifiesNamespace(namespaceDecl( hugoeg wrote: > JonasToth wrote: > > Actually that one is generally useful.

[PATCH] D50389: [clang-tidy] new check for Abseil

JonasToth added inline comments. Comment at: clang-tidy/abseil/DurationDivisionCheck.cpp:50 + *result.SourceManager, result.Context->getLangOpts()), + ")"); +} deannagarcia wrote: > JonasToth wrote: > > This line looks odd, does it

[PATCH] D50389: [clang-tidy] new check for Abseil

JonasToth added a comment. LGTM with only the formatting question. But don't commit before any of the reviewers accepts (@alexfh @aaron.ballman usually have the last word) Comment at: clang-tidy/abseil/DurationDivisionCheck.cpp:50 + *result.SourceManager,

[PATCH] D36892: [clang-tidy] check_clang_tidy.py: support CHECK-NOTES prefix

JonasToth added a comment. LGTM Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D36892 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50389: [clang-tidy] new check for Abseil

JonasToth added inline comments. Comment at: clang-tidy/abseil/DurationDivisionCheck.cpp:29 + hasSourceExpression(ignoringParenCasts(cxxOperatorCallExpr( + hasOverloadedOperatorName("/"), argumentCountIs(2), + hasArgument(0, expr(IsDuration)),

[PATCH] D50389: [clang-tidy] new check for Abseil

JonasToth added inline comments. Comment at: clang-tidy/abseil/DurationDivisionCheck.cpp:32 + hasImplicitDestinationType(qualType(unless(isInteger(, + unless(hasParent(cxxStaticCastExpr(, + this); deannagarcia wrote: > JonasToth

[PATCH] D50447: [clang-tidy] Omit cases where loop variable is not used in loop body in performance-for-range-copy.

JonasToth added a comment. Do you think it is a bad idea? If the variable is not used it is ok to ignore it in this particular circumstance. Other warnings/check should deal with such a situation IMHO. Am 10.08.2018 um 10:29 schrieb Roman Lebedev via Phabricator: > lebedev.ri added a comment. >

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

JonasToth added inline comments. Comment at: clang-tidy/abseil/NoInternalDepsCheck.cpp:24 + + Finder->addMatcher( + nestedNameSpecifierLoc(loc(specifiesNamespace(namespaceDecl( Actually that one is generally useful. Accessing the `foo::internal` from

[PATCH] D50447: [clang-tidy] Omit cases where loop variable is not used in loop body in performance-for-range-copy.

JonasToth added a comment. LGTM, but alex or aaron should allow the commit ;) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50447 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

JonasToth updated this revision to Diff 159775. JonasToth added a comment. - fix bug with AnalyzeValues==false skipping whole check, adjust test code to 'const' instead of const Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45444 Files:

[PATCH] D50447: [clang-tidy] Add a whitelistClasses option in performance-for-range-copy check.

JonasToth added a comment. > But in our codebase, we have code intended to use like below, and it is in > base library, and is used widely. I think it makes sense to whitelist this > class in our internal configuration. > > for (auto _ : state) { > ... // no `_` being referenced in the

[PATCH] D50447: [clang-tidy] Add a whitelistClasses option in performance-for-range-copy check.

JonasToth added a comment. Given it is about the performance in loops and the optimizer?! can better work with the copy-and-don't-use it might make sense to just check if the variable is dereferenced in the scope of the loop (any declRefExpr exists). That is more userfriendly, too. Am

[PATCH] D49851: [clang-tidy] run-clang-tidy add synchronisation to the output

JonasToth added a comment. @alexfh or @aaron.ballman could you take a final look on this one? https://reviews.llvm.org/D49851 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50447: [clang-tidy] Add a whitelistClasses option in performance-for-range-copy check.

JonasToth added a comment. Is the type important, or specifics about the variable (e.g. the name?) The `_` variable names are sometimes used for RAII variables/lambdas that shall just do some cleanup, e.g. gsl::finally(). It might make sense to give `ExprMutAnalyzer` an ignore-mechanism. I

[PATCH] D50447: [clang-tidy] Add a whitelistClasses option in performance-for-range-copy check.

JonasToth added inline comments. Comment at: test/clang-tidy/performance-for-range-copy.cpp:1 -// RUN: %check_clang_tidy %s performance-for-range-copy %t -- -- -std=c++11 -fno-delayed-template-parsing +// RUN: %check_clang_tidy %s performance-for-range-copy %t

[PATCH] D50447: [clang-tidy] Add a whitelistClasses option in performance-for-range-copy check.

JonasToth added a comment. If whitelisting works, thats fine. But I agree with @lebedev.ri that a contribution/improvement to the ExprMutAnalyzer is the better option. This is especially the case, because multiple checks (and maybe even some other parts then clang-tidy) will utilize this

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

JonasToth added a comment. > Could you give a concrete example of this? vi llvm/lib/Demangle/ItaniumDemangle.cpp +1762 /home/jonas/opt/llvm/lib/Demangle/ItaniumDemangle.cpp:1762:7: warning: variable 'num' of type 'char [FloatData::max_demangled_size]' can be declared 'const'

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

JonasToth added a comment. For reference, here is the output for llvm/lib. F6896569: const_correctness_2_llvm_lib_references F6896567: const_correctness_2_llvm_lib_values_pointers Things i noticed: - lambdas are warned

[PATCH] D50389: [clang-tidy] new check for Abseil

JonasToth added inline comments. Comment at: clang-tidy/abseil/DurationDivisionCheck.cpp:37 +void DurationDivisionCheck::check(const MatchFinder::MatchResult& result) { + const auto* op = result.Nodes.getNodeAs("op"); + diag(op->getOperatorLoc(), JonasToth

[PATCH] D49851: [clang-tidy] run-clang-tidy add synchronisation to the output

JonasToth added a comment. No nothing. I think it is barely noticable in general. From prior long runs i notices the scrambled messages, but it was only a small fraction. So most of the time the threads dont seem to interfere, which makes the lock kinda low-cost. Am 07.08.2018 um 19:17 schrieb

[PATCH] D49851: [clang-tidy] run-clang-tidy add synchronisation to the output

JonasToth added a comment. I do run it over llvm/lib, there is no visible effect. from my experience maybe 1-5% of the lines are interleafed without the patch, too. https://reviews.llvm.org/D49851 ___ cfe-commits mailing list

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