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

2018-10-22 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 170472. JonasToth added a comment. - [Doc] add missing release notes Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41648 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp

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

2018-10-22 Thread Jonas Toth via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE344940: [clang-tidy] implement cppcoreguidelines macro rules (authored by JonasToth, committed by ). Changed prior to commit: https://reviews.llvm.org/D41648?vs=170472&id=170473#toc Repository: rCT

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

2018-10-22 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. Please add a short sentence to the release notes and adjust the documentation to that check to cover the changes. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53488 ___ cfe-commits mailing list cfe-comm

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

2018-10-22 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:58 + const QualType Rhs) { + assert(Lhs->isRealType()); // Either integer or floating point. + assert(Rhs->isFloatingType()); // Floating point only

[PATCH] D53339: [clang-tidy] Add the abseil-duration-factory-float check

2018-10-22 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:51 + hasArgument(0, + anyOf(cxxStaticCastExpr( +hasDestinationType(realFloatingPointType()), What about c-st

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

2018-10-22 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. In https://reviews.llvm.org/D53025#1270784, @ymandel wrote: > Correction: the code *catches* the trailing return cases, I just couldn't > find a way to *fix* them; specifically, I was not able to get the necessary > source ranges to re-lex the trailing return type.

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

2018-10-22 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. > Also, the template version (whether trailing or normal return) does not > trigger the matchers at all, from which I surmise that clang doesn't consider > the type const qualified when it is not materialized with an actual type. > I've noted as much in the comments,

[PATCH] D53339: [clang-tidy] Add the abseil-duration-factory-float check

2018-10-23 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land. @alexfh you did comment before, do you want to add more? I have no issues left. Please give alex the opportunity to react, but if he doesn't (he has a lot to do) you can commit in 3 days

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

2018-10-24 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:58 + const QualType Rhs) { + assert(Lhs->isRealType()); // Either integer or floating point. + assert(Rhs->isFloatingType()); // Floating point only

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

2018-10-24 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. Please add a short notice in the release notes for this change. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53488 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/ma

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

2018-10-24 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: docs/ReleaseNotes.rst:166 +- The :doc:`cppcoreguidelines-narrowing-conversions +` check. please concat the two parts, similar to the one above. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D5

[PATCH] D52690: [clang-tidy] NFC use CHECK-NOTES in tests for misc-misplaced-const

2018-10-24 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. ping :) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52690 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2018-10-24 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land. Did you run this code over a real-world code-base and did you find new stuff and/or false positives or the like? From my side LGTM unless other reviewers see outstanding issues. Reposi

[PATCH] D53339: [clang-tidy] Add the abseil-duration-factory-float check

2018-10-24 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:84 + + // Check for floats without fractional components + if (const auto *LitFloat = alexfh wrote: > JonasToth wrote: > > missing full stop > Clang-tidy (and clang) di

[PATCH] D53339: [clang-tidy] Add the abseil-duration-factory-float check

2018-10-24 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. I think accepted now? :) If you want I can commit for you and monitor the buildbot, if there are bigger problems I would come back to you. https://reviews.llvm.org/D53339 ___ cfe-commits mailing list cfe-commits@lists.llv

[PATCH] D53339: [clang-tidy] Add the abseil-duration-factory-float check

2018-10-24 Thread Jonas Toth via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL345167: [clang-tidy] Add the abseil-duration-factory-float check (authored by JonasToth, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D53339

[PATCH] D53339: [clang-tidy] Add the abseil-duration-factory-float check

2018-10-24 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. Thanks for the patch! Repository: rL LLVM https://reviews.llvm.org/D53339 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2018-10-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. ping :) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51949 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2018-10-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. That one looks interesting :) Am 25.10.18 um 17:13 schrieb Guillaume Chatelet via Phabricator: > gchatelet added a comment. > > In https://reviews.llvm.org/D53488#1275786, @hokein wrote: > >> In https://reviews.llvm.org/D53488#1275750, @gchatelet wrote: >> >>> In ht

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

2018-10-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. > I'm not sure I understand what you're trying to break in the check. If you're > thinking of whether the code trips up on the lexical issues, I'm pretty sure > that won't apply here. Once the const-ness is hidden behind a template, the > check doesn't try to fix it;

[PATCH] D53719: [clang-tidy] UppercaseLiteralSuffixCheck: bugfix: don't cripple substNonTypeTemplateParmExpr

2018-10-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. What do you mean by "general way to avoid"? Templates? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53719 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/lis

[PATCH] D53719: [clang-tidy] UppercaseLiteralSuffixCheck: bugfix: don't cripple substNonTypeTemplateParmExpr

2018-10-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land. > I'm basically asking "is the > `unless(hasParent(substNonTypeTemplateParmExpr()))` fix correct? or is there > something else i'm missing?" Ah ok, I am not aware of another way, but no

[PATCH] D53723: [clang-tidy] UppercaseLiteralSuffixCheck: bugfix: ignore implicit code

2018-10-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. Did you discover these in a real project? May I ask which one and add it to my BB? :) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53723 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://li

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

2018-10-26 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. `modernize` would be a fitting module, wouldn't it? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53771 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listin

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

2018-10-26 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: docs/clang-tidy/checks/list.rst:13 abseil-redundant-strcat-calls - abseil-string-find-startswith abseil-str-cat-append spurious change Comment at: docs/clang-tidy/checks/list.rst:154 hi

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

2018-10-26 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: test/clang-tidy/misc-avoid-c-arrays.cpp:14 + +template +class array { lebedev.ri wrote: > JonasToth wrote: > > Please add a case with templates, where `T` itself is the array type, maybe > > there are other fancy tem

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

2018-10-26 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/readability/IsolateDeclarationCheck.cpp:52 + const LangOptions &LangOpts) { + assert(Indirections >= 0 && "Indirections must be non-negative"); + if (Indirections == 0) -

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

2018-10-26 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 171346. JonasToth marked 16 inline comments as done. JonasToth added a comment. - address review comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51949 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/Isolate

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

2018-10-26 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 171361. JonasToth added a comment. - [Fix] wrong condition in matcher coming from incorrect code change Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51949 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/IsolateDe

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

2018-10-29 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp:84 "cppcoreguidelines-c-copy-assignment-signature"); +CheckFactories.registerCheck( +"cppcoreguidelines-avoid-c-arrays"); please conserv

[PATCH] D53817: [clang-tidy] cppcoreguidelines-macro-usage: print macro names

2018-10-29 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. How does this patch change the behaviour for macros without location? They will be diagnosed at the beginning of the TU? It is probably better to ignore these kind of macros, as there is no way around them (compile time configuration can only be done with macros?!) an

[PATCH] D53817: [clang-tidy] cppcoreguidelines-macro-usage: print macro names

2018-10-29 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. >> They will be diagnosed at the beginning of the TU? >> It is probably better to ignore these kind of macros, as there is no way >> around them (compile time configuration can only be done with macros?!) and >> therefore warnings for theses macros are false positives

[PATCH] D53817: [clang-tidy] cppcoreguidelines-macro-usage: print macro names

2018-10-29 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. > I personally haven't yet seen this check firing on *predefined* macros. I did specifically check the compiler defined macros I could see in the AST and they were not diagnosed from the beginning. I would guess, only the compiler-flag macros are passed in the PPCallba

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

2018-10-29 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 171538. JonasToth added a comment. - Merge branch 'master' into experiment_isolate_decl - adjust doc slighty to comment Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51949 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/reada

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

2018-10-29 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. This is usually not done in the LLVM codebase, as its not a pointer/reference. Am 29.10.18 um 20:29 schrieb Alexander Zaitsev via Phabricator: > ZaMaZaN4iK added inline comments. > > > Comment at: clang-tidy/readability/IsolateDeclarationCheck.cpp:11

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

2018-10-30 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. Hi astrelni, my 2cents. Please upload the patch will full context (i believe `diff -U 9`, but check the man pages if that doesn't work :D) Comment at: clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp:32 + + // a *= b; a /= b; + Finder->ad

[PATCH] D53817: [clang-tidy] cppcoreguidelines-macro-usage: print macro names

2018-10-30 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst:30 + +.. option:: IgnoreLocationless + I think the another option name would fit better: how about `IgnoreCommandLineMacros` or the like? Repository: rCTE C

[PATCH] D53817: [clang-tidy] cppcoreguidelines-macro-usage: print macro names

2018-10-30 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land. LGTM, @aaron.ballman do you have more comments? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53817 ___ cfe-commits maili

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

2018-10-30 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth requested changes to this revision. JonasToth added a comment. This revision now requires changes to proceed. because this now diverged quite a bit i will revert the lgtm for now and take a longer look at the new patch. The numbers you reported sound good. Comment at

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

2018-10-30 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: docs/ReleaseNotes.rst:77 - New :doc:`abseil-duration-factory-float ` check. JonasToth wrote: > that seems to be unrelated sry for noise, this was part of the history diffs! Repository: rCTE Clang Tools Extra

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

2018-10-30 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:25 // FIXME: Check double -> float truncation. Pay attention to casts: void NarrowingConversionsCheck::registerMatchers(MatchFinder *Finder) { I think this

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

2018-10-30 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. Sorry for responding so late, but i thought i get a shot to look at it. Here is the mail from Douglas Yung who was so kind to isolate the breakage and find a reproducer. If you have the chance please take a look at it. Hi Jonas, I have attached a bzipped prepro

[PATCH] D53882: [clang-tidy] Adding Zircon checker for std namespace

2018-10-30 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tools-extra/clang-tidy/zircon/NoStdNamespaceCheck.cpp:29 +AST_MATCHER(NamespaceAliasDecl, isAliasedToStd) { + if (const auto *AN = Node.getAliasedNamespace()) { +// If this aliases to an actual namespace, check if its std. I

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

2018-10-30 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:107 +diag(Def->getInnerLocStart(), "return type %0 is 'const'-qualified " + "hindering compiler optimizations") +<< Def->getReturnTyp

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

2018-10-30 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. Am 30.10.18 um 21:47 schrieb Aaron Ballman via Phabricator: > aaron.ballman added a comment. > > I think this is getting really close! One question: would it make more sense > to name this `readability-const-type-return` or > `readability-const-return-type` instead?

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

2018-10-30 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. Interesting, did you try out `gcc-4.8` and if it is reproducable with that one? This version of gcc is still supported and if the problem occurs with the libstdc++ shipped there, we need to consider it. > However, **`16.04 LTS (Xenial)`** at the time of writing this com

[PATCH] D53882: [clang-tidy] Adding Zircon checker for std namespace

2018-10-30 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tools-extra/clang-tidy/zircon/NoStdNamespaceCheck.cpp:71 +void NoStdNamespaceCheck::check(const MatchFinder::MatchResult &Result) { + if (const auto *D = Result.Nodes.getNodeAs("stdVar")) +diag(D->getBeginLoc(),

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

2018-10-30 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:149 + const QualType RhsType = Rhs->getType(); + if (Lhs->isValueDependent() || Rhs->isValueDependent() || + LhsType->isDependentType() || RhsType->isDependentType()) --

[PATCH] D53882: [clang-tidy] Adding Zircon checker for std namespace

2018-10-31 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:176 + + Warns when the `std` namespace is used, as its use is against Zircon's libc++ + policy for the kernel. alexfh wrote: > JonasToth wrote: > > s/its/it's/ > > > > Could `

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

2018-10-31 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:107 +diag(Def->getInnerLocStart(), "return type %0 is 'const'-qualified " + "hindering compiler optimizations") +<< Def->getReturnTyp

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

2018-10-31 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth marked an inline comment as done. JonasToth added inline comments. Comment at: clang-tidy/readability/IsolateDeclarationCheck.cpp:119 + const LangOptions &LangOpts) { + std::size_t DeclCount = std::distance(DS->decl_begin(), DS->decl_end()); + if (DeclCount

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

2019-01-14 Thread Jonas Toth via Phabricator via cfe-commits
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 Ex

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

2019-01-15 Thread Jonas Toth via Phabricator via cfe-commits
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 https://r

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

2019-01-15 Thread Jonas Toth via Phabricator via cfe-commits
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 le

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

2019-01-15 Thread Jonas Toth via Phabricator via cfe-commits
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] D56563: [clang-tidy] add options documentation to readability-identifier-naming checker

2019-01-15 Thread Jonas Toth via Phabricator via cfe-commits
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 http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-

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

2019-01-15 Thread Jonas Toth via Phabricator via cfe-commits
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 cfe-co

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

2019-01-15 Thread Jonas Toth via Phabricator via cfe-commits
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/ https:

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

2019-01-15 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:33 +const LangOptions &LangOpts) { + // we start with the location of the closing parenthesis. + const TypeSourceInfo *TSI = F.getTypeSourceInfo(); Nit: s/we/We =

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

2019-01-15 Thread Jonas Toth via Phabricator via cfe-commits
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] D54395: [clang-tidy] implement utility-function to add 'const' to variables

2019-01-17 Thread Jonas Toth via Phabricator via cfe-commits
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: cla

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

2019-01-17 Thread Jonas Toth via Phabricator via cfe-commits
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 clang-tidy/cpp

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

2019-01-17 Thread Jonas Toth via Phabricator via cfe-commits
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 clang-tidy/cppco

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

2019-01-17 Thread Jonas Toth via Phabricator via cfe-commits
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: clang-tidy/cppcoreguidelines/CMakeLists

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

2019-01-17 Thread Jonas Toth via Phabricator via cfe-commits
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 clang-tidy/cpp

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

2019-01-17 Thread Jonas Toth via Phabricator via cfe-commits
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] D56160: [clang-tidy] modernize-use-trailing-return check

2019-01-18 Thread Jonas Toth via Phabricator via cfe-commits
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 the

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

2019-01-18 Thread Jonas Toth via Phabricator via cfe-commits
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 llvm-elide-braces-for-small-st

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

2019-01-18 Thread Jonas Toth via Phabricator via cfe-commits
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 reduc

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

2019-01-18 Thread Jonas Toth via Phabricator via cfe-commits
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. Repositor

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

2019-01-18 Thread Jonas Toth via Phabricator via cfe-commits
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

2019-01-18 Thread Jonas Toth via Phabricator via cfe-commits
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 https://

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

2019-01-18 Thread Jonas Toth via Phabricator via cfe-commits
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] D56926: [Documentation] Use HTTPS whenever possible in clang-tools-extra

2019-01-18 Thread Jonas Toth via Phabricator via cfe-commits
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 LA

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

2019-01-18 Thread Jonas Toth via Phabricator via cfe-commits
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 n

[PATCH] D56966: [clang-tidy] misc-non-private-member-variables-in-classes: ignore implicit methods

2019-01-19 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp:22 AST_MATCHER(CXXRecordDecl, hasMethods) { - return std::distance(Node.method_begin(), Node.method_end()) != 0; + for (const auto &Method : Node.methods()) { +if (Meth

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

2019-01-21 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 182780. JonasToth added a comment. - use only the small reproducer Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56917/new/ https://reviews.llvm.org/D56917 Files: unittests/Analysis/ExprMutationAnalyzerTest.cpp Index:

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

2019-01-21 Thread Jonas Toth via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL351743: [clang] add tests to ExprMutAnalyzer that reproduced a crash in ASTMatchers (authored by JonasToth, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGES SINCE LA

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

2019-01-23 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. ping :) Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54395/new/ https://reviews.llvm.org/D54395 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/

[PATCH] D57100: [clang-tidy] refactor bugprone-exception-escape analysis into class

2019-01-23 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth created this revision. JonasToth added reviewers: aaron.ballman, alexfh, hokein, baloghadamsoftware. Herald added subscribers: cfe-commits, rnkovacs, xazax.hun, mgorny. The check `bugprone-exception-escape` does an AST-based analysis to determine if a function might throw an exception an

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

2019-01-23 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. In D56160#1367074 , @bernhardmgruber wrote: > Thank you again @JonasToth for all your valueable input! I could almost > successfully run my check on the llvm/lib subfolder. I created a compilation > database from within Visual

[PATCH] D57100: [clang-tidy] refactor bugprone-exception-escape analysis into class

2019-01-23 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. In D57100#1367813 , @lebedev.ri wrote: > Two passing-by remarks: > > 1. I would *love* for this check to produce less cryptic reports :) It > currently does not output any details whatsoever. It's really unhelpful. I would do

[PATCH] D57100: [clang-tidy] refactor bugprone-exception-escape analysis into class

2019-01-23 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 183099. JonasToth removed a subscriber: lebedev.ri. JonasToth added a comment. - adjust doc to say semicolon separated list instead of comma Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57100/new/ https://rev

[PATCH] D57087: [clang-tidy] add OverrideMacro to modernize-use-override check

2019-01-23 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/modernize/UseOverrideCheck.cpp:133 +StringRef Correct = +HasFinal ? "'" + FinalMacro + "'" : "'" + OverrideMacro + "'"; Dangling? These values seem to be temporary and `StringRef` would bind to

[PATCH] D57100: [clang-tidy] refactor bugprone-exception-escape analysis into class

2019-01-23 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 183131. JonasToth added a comment. - [Fix] make class name correct Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57100/new/ https://reviews.llvm.org/D57100 Files: clang-tidy/bugprone/ExceptionEscapeCheck.cp

[PATCH] D57108: [clang-tidy] diagnose possibiltiy to add 'noexcept' in modernize-use-noexcept

2019-01-23 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth created this revision. JonasToth added reviewers: aaron.ballman, alexfh, hokein. Herald added subscribers: xazax.hun, mgorny. This patch utilizes the refactored `ExceptionAnalyzer` for `modernize-use-noexcept` to diagnose possibilties to introduce `noexcept` if there is no exception spec

[PATCH] D57100: [clang-tidy] refactor bugprone-exception-escape analysis into class

2019-01-23 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth marked 3 inline comments as done. JonasToth added inline comments. Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:35 + std::vector FunctionsThatShouldNotThrowVec = + utils::options::parseStringList(RawFunctionsThatShouldNotThrow); FunctionsThatShouldN

[PATCH] D57100: [clang-tidy] refactor bugprone-exception-escape analysis into class

2019-01-23 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 183134. JonasToth marked an inline comment as done. JonasToth added a comment. - add license to exceptionanalyzer Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57100/new/ https://reviews.llvm.org/D57100 Files

[PATCH] D57108: [clang-tidy] diagnose possibiltiy to add 'noexcept' in modernize-use-noexcept

2019-01-23 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 183136. JonasToth added a comment. - rebase to current refactoring Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57108/new/ https://reviews.llvm.org/D57108 Files: clang-tidy/bugprone/ExceptionEscapeCheck.cp

[PATCH] D57100: [clang-tidy] refactor bugprone-exception-escape analysis into class

2019-01-24 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth marked 3 inline comments as done. JonasToth added inline comments. Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:35 + std::vector FunctionsThatShouldNotThrowVec = + utils::options::parseStringList(RawFunctionsThatShouldNotThrow); FunctionsThatShouldN

[PATCH] D57108: [clang-tidy] diagnose possibiltiy to add 'noexcept' in modernize-use-noexcept

2019-01-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. In D57108#1369161 , @lebedev.ri wrote: > Hmm, i think this diff is incorrect. Doesn't it also include the changes from > D57100 ? yup, needs constant rebasing, i will do a final rebase once th

[PATCH] D57100: [clang-tidy] refactor bugprone-exception-escape analysis into class

2019-01-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 183489. JonasToth marked an inline comment as done. JonasToth added a comment. - revert the option configuration change for comma/semicolon Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57100/new/ https://revi

[PATCH] D57100: [clang-tidy] refactor bugprone-exception-escape analysis into class

2019-01-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth marked 2 inline comments as done. JonasToth added a comment. Reverted the changes. Maybe the comma thingie will just stay as is ;) Comment at: clang-tidy/utils/ExceptionAnalyzer.h:25 +/// give the possibility of an exception. +class ExceptionAnalyzer { +public: --

[PATCH] D57100: [clang-tidy] refactor bugprone-exception-escape analysis into class

2019-01-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 183490. JonasToth added a comment. - revert doc change as well Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57100/new/ https://reviews.llvm.org/D57100 Files: clang-tidy/bugprone/ExceptionEscapeCheck.cpp

[PATCH] D57108: [clang-tidy] diagnose possibiltiy to add 'noexcept' in modernize-use-noexcept

2019-01-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 183491. JonasToth added a comment. - rebase to latest patch Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57108/new/ https://reviews.llvm.org/D57108 Files: clang-tidy/bugprone/ExceptionEscapeCheck.cpp cla

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

2019-01-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/abseil/DurationAdditionCheck.cpp:22 +void DurationAdditionCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + binaryOperator(hasOperatorName("+"), This whole file is structurally sim

[PATCH] D57113: [clang-tidy] openmp-use-default-none - a new module and a check

2019-01-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/openmp/UseDefaultNoneCheck.cpp:51 +///instead of the actual OpenMPClauseKind. +AST_MATCHER_P(OMPExecutableDirective, isAllowedToContainClause, + OpenMPClauseKind, CKind) { Why is this re

[PATCH] D57113: [clang-tidy] openmp-use-default-none - a new module and a check

2019-01-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: docs/clang-tidy/checks/openmp-use-default-none.rst:12 +being implicitly determined, and thus forces developer to be explicit about the +desired data scoping for each variable. + If I understand correctly the issue is m

[PATCH] D57113: [clang-tidy] openmp-use-default-none - a new module and a check

2019-01-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/openmp/UseDefaultNoneCheck.cpp:113 +void UseDefaultNoneCheck::registerMatchers(MatchFinder *Finder) { + // If OpenMP is not enabled, don't register the check, it won't find anything. + if (!getLangOpts().OpenMP) -

[PATCH] D57113: [clang-tidy] openmp-use-default-none - a new module and a check

2019-01-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: docs/clang-tidy/checks/openmp-use-default-none.rst:12 +being implicitly determined, and thus forces developer to be explicit about the +desired data scoping for each variable. + lebedev.ri wrote: > JonasToth wrote: > >

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