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

JonasToth marked an inline comment as done. JonasToth added inline comments. Comment at: clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp:147 + // Example: `int i = 10`, `int i` (will be used if program is correct) + const auto LocalValDecl = varDecl(unless(anyOf( +

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

JonasToth marked an inline comment as done. JonasToth added inline comments. Comment at: clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp:183 + // TODO Implement automatic code transformation to add the 'const'. + diag(Variable->getLocStart(), "variable %0 of type %1 can

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

JonasToth updated this revision to Diff 159390. JonasToth marked 4 inline comments as done. JonasToth added a comment. - address review issues, todos/fixmes and diag nit Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45444 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt

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

JonasToth added a comment. In https://reviews.llvm.org/D45444#1189262, @aaron.ballman wrote: > However, I'm wondering how this should integrate with other const-correctness > efforts like `readability-non-const-parameter`? I think this check/functionality will kinda replace the `readability-n

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

JonasToth added a comment. I would like to have this patch in, because i currently have a 1MB output file, and the missing synchronization really destroys cleaning with grep and sed :( Comment at: clang-tidy/tool/run-clang-tidy.py:167 +output, err = proc.communicate() +

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

JonasToth added a comment. I forgot: Did you measure how much of a time penalty this brings? Just out of curiosity, correct script is more important then fast script :) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49851 ___ cfe-c

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

JonasToth added a comment. Lets say running it over whole LLVM for one check? I will apply this path locally, because i have to analyze llvm right now anyway. For me this looks good, but @alexfh or @aaron.ballman should accept it. https://reviews.llvm.org/D49851

[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 cfe-commits@lists.llvm

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

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

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

[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' [cppcoreguidelines-c

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

[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 -config="{CheckOp

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

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

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

[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: clang-tidy/cppcoreguidelines/CMakeLis

[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 http://lists.llvm.org/cgi-bin/mailman/listinf

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

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

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

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

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

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

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

[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 &Result) { + const auto *decl = Result.Nodes.getNodeAs

[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] 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 test/clang-ti

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

[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] 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: docs/clang-tidy/ch

[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 Characters[SizeIndicator::value

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

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

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

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

[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' + J

[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] 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 > + match(withEnclosingCompound(decl

[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. @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 Tool

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

[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' + h

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

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

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

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

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

[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* clang::CX

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

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

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

[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? (ht

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

JonasToth added a comment. ping. is there something obviously wrong with this check? 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] D38688: [clang-tidy] Misc redundant expressions checker updated for macros

JonasToth added inline comments. Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:357 + ConstExpr = Result.Nodes.getNodeAs(CstId); + return ConstExpr && ConstExpr->isIntegerConstantExpr(Value, *Result.Context); +} xazax.hun wrote: > I think you could ju

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

JonasToth updated this revision to Diff 118911. JonasToth added a comment. ping: I don't understand the lack of feedback. This check should not be a frontend warning, since it warns for perfectly valid code. It is supposed to give stronger guarantees for developers requiring this. - rebased to

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

JonasToth added inline comments. Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:85 +diag(ElseIfWithoutElse->getLocStart(), + "potential uncovered codepath found; add an ending else branch"); +return; JDevlieghere wrote: > I'm not a big

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

JonasToth updated this revision to Diff 118951. JonasToth marked 2 inline comments as done. JonasToth added a comment. - major codechange https://reviews.llvm.org/D37808 Files: clang-tidy/hicpp/CMakeLists.txt clang-tidy/hicpp/HICPPTidyModule.cpp clang-tidy/hicpp/MultiwayPathsCoveredCheck.

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

JonasToth marked 6 inline comments as done. JonasToth added inline comments. Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:96 +// Only the default branch (we explicitly matched for default!) exists. +if (CaseCount == 1) { + diag(SwitchWithDefault->getLoc

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

JonasToth marked 4 inline comments as done. JonasToth added a comment. Improved check a lot. Hope reviewers now have an easier time reading it. https://reviews.llvm.org/D37808 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm

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

JonasToth added a comment. F5426271: llvm_lib_target_x86_paths Example output for `llvm/lib/Target/X86` Running it over the whole `llvm/lib` codebase generates a lot of warnings. Please note, that it seems to be common to write code like this: int Val; swi

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

JonasToth marked 4 inline comments as done. JonasToth added inline comments. Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:98 + // hold. + const std::size_t BitCount = [&T, &Context]() { +if (T->isIntegralType(Context)) JDevlieghere wrote: > Un

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

JonasToth updated this revision to Diff 119440. JonasToth marked an inline comment as done. JonasToth added a comment. - address review comments https://reviews.llvm.org/D37808 Files: clang-tidy/hicpp/CMakeLists.txt clang-tidy/hicpp/HICPPTidyModule.cpp clang-tidy/hicpp/MultiwayPathsCovere

[PATCH] D39027: [docs][refactor] Add a new tutorial that talks about how one can implement refactoring actions

JonasToth added inline comments. Comment at: docs/RefactoringActionTutorial.rst:90 +Let's call it something like ``FlattenIfStatements.cpp``. Don't forget to add +it to the ``CMakeLists.txt`` file that's located in the +``lib/Tooling/Refactoring`` directory. Will

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

JonasToth updated this revision to Diff 119553. JonasToth added a comment. - Improve docs, grammar https://reviews.llvm.org/D37808 Files: clang-tidy/hicpp/CMakeLists.txt clang-tidy/hicpp/HICPPTidyModule.cpp clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp clang-tidy/hicpp/MultiwayPathsCov

[PATCH] D33537: [clang-tidy] Exception Escape Checker

JonasToth added a comment. > I agree that we should not spend too much effort on making warnings from the > compiler and tidy disjunct. +1 There is an effort to treat clangs frontend warnings similar to clang-tidy checks within clang-tidy. This would allow to manage the overlap as well. Will t

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

JonasToth added a comment. simplified the if-else stuff 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 updated this revision to Diff 120251. JonasToth marked 8 inline comments as done. JonasToth added a comment. - Merge 'master' - address review comments https://reviews.llvm.org/D37808 Files: clang-tidy/hicpp/CMakeLists.txt clang-tidy/hicpp/HICPPTidyModule.cpp clang-tidy/hicpp/Mu

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

[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 http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-

[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] 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: > > clang-tidy/tool/

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

JonasToth added a comment. Please create the patch with full context (https://www.llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51220 ___ cfe-commits mailin

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

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

[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. > > - https://reviews.llvm.org/F704765

[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: lib/Format/Contin

[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: lib/Form

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

[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 clang-tidy/cppcoregui

[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 clang-tidy/cppcoreguidelines/CppCoreGuidelinesT

[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 clang-tidy/cppcoreguidelines/CppCoreGuideli

  1   2   3   4   5   6   7   8   9   10   >