[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

2017-02-07 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added inline comments. Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:54 +// FIXME use DiagnosticIDs::Level::Note +diag(NoExceptRange.getBegin(), "in a function declared no-throw here:", DiagnosticIDs::Note) +<<

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

2017-02-08 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added inline comments. Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:54 +// FIXME use DiagnosticIDs::Level::Note +diag(NoExceptRange.getBegin(), "in a function declared no-throw here:", DiagnosticIDs::Note) +<<

[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.

2017-02-06 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. In https://reviews.llvm.org/D29151#668197, @zaks.anna wrote: > > I tried to come up with some advice on where checks should go in > > http://clang.llvm.org/extra/clang-tidy/#choosing-the-right-place-for-your-check > > The guidelines stated on the clang-tidy page seem

[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.

2017-02-04 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. In https://reviews.llvm.org/D29151#665948, @alexfh wrote: > In https://reviews.llvm.org/D29151#662504, @zaks.anna wrote: > > > Before clang-tidy came into existence the guidelines were very clear. One > > should write a clang warning if the diagnostic: > > > > - can be

[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.

2017-01-31 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. In https://reviews.llvm.org/D29151#658484, @zaks.anna wrote: > The static analyzer is definitely the place to go for bug detection that > requires path sensitivity. It's also reasonably good for anything that needs > flow-sensitivity. Although, most flow-sensitive

[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.

2017-01-26 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. In https://reviews.llvm.org/D29151#656887, @Eugene.Zelenko wrote: > General question: isn't Static Analyzer is better place for such workflow > checks? Probably yes, but it is much harder to implement this there. Other problem is that it would be probably a part of

[PATCH] D22057: Prevent devirtualization of calls to un-instantiated functions.

2017-01-28 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. I have read the patch, but I don't have enough knowledge about C++ rules and fronted to accept it. Richard? Comment at: lib/Sema/Sema.cpp:684 + for (auto PII : Pending) +if (FunctionDecl *Func = dyn_cast(PII.first)) +

[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.

2017-01-25 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added inline comments. Comment at: clang-tidy/misc/InvalidatedIteratorsCheck.cpp:61 + cxxMemberCallExpr(has(memberExpr(hasDeclaration(cxxMethodDecl(hasAnyName( + "push_back", "emplace_back", "clear", +

[PATCH] D29839: [clang-tidy] New misc-istream-overflow check

2017-02-10 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. Nice check! :) Comment at: clang-tidy/misc/IstreamOverflowCheck.cpp:59-61 + if (ConstType) { +ArraySize = ConstType->getSize(); + } same here Comment at: clang-tidy/misc/IstreamOverflowCheck.cpp:78-80 +if

[PATCH] D29839: [clang-tidy] New misc-istream-overflow check

2017-02-11 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. In https://reviews.llvm.org/D29839#674301, @xazax.hun wrote: > Shouldn't this be a path sensitive check within the clang static analyzer > instead? So branches are properly handled and interprocedural analysis is > done. Do you have some examples? I would argue, that

[PATCH] D19105: Changes in clang after running http://reviews.llvm.org/D18821

2017-02-13 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek abandoned this revision. Prazek added inline comments. Comment at: lib/CodeGen/SplitKit.h:298 - typedef PointerIntPair ValueForcePair; + typedef PointerIntPair ValueForcePair; typedef DenseMap,

[PATCH] D29839: [clang-tidy] New misc-istream-overflow check

2017-02-11 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. In https://reviews.llvm.org/D29839#674315, @xazax.hun wrote: > In https://reviews.llvm.org/D29839#674307, @Prazek wrote: > > > In https://reviews.llvm.org/D29839#674301, @xazax.hun wrote: > > > > > Shouldn't this be a path sensitive check within the clang static analyzer

[PATCH] D28746: Mention ThinLTO with PGO in ReleaseNotes 4.0

2017-01-15 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. Shoud I add the same note to LLVM Release notes? https://reviews.llvm.org/D28746 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28746: Mention ThinLTO with PGO in ReleaseNotes 4.0

2017-01-15 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek updated this revision to Diff 84504. Prazek marked an inline comment as done. Prazek added a comment. (PGO) https://reviews.llvm.org/D28746 Files: docs/ReleaseNotes.rst Index: docs/ReleaseNotes.rst === ---

[PATCH] D28729: [clang-tidy] Add -enable-alpha-checks command

2017-01-15 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. In https://reviews.llvm.org/D28729#646758, @aaron.ballman wrote: > In https://reviews.llvm.org/D28729#646560, @alexfh wrote: > > > In https://reviews.llvm.org/D28729#646555, @aaron.ballman wrote: > > > > > In https://reviews.llvm.org/D28729#646548, @alexfh wrote: > > > >

[PATCH] D28746: Mention ThinLTO with PGO in ReleaseNotes 4.0

2017-01-15 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. In https://reviews.llvm.org/D28746#646875, @mehdi_amini wrote: > Oh I didn't notice that we were in the clang Release Notes, actually I've > only updated the LLVM ones in the past. > > So yes, I think this should be in LLVM as well, I don't know if it should > differ in

[PATCH] D28746: Mention ThinLTO with PGO in ReleaseNotes 4.0

2017-01-15 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. Maybe I should change ThinLTO for '-flto=thin' and also mention this improvement in LLVM release notes? BTW '-flto=thin' is not documented in UsersManual https://reviews.llvm.org/D28746 ___ cfe-commits mailing list

[PATCH] D28746: Mention ThinLTO with PGO in ReleaseNotes 4.0

2017-01-15 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek created this revision. Prazek added reviewers: tejohnson, mehdi_amini, hans. Prazek added a subscriber: cfe-commits. It would be good to also mention other improvements in ThinLTO for 4.0 release https://reviews.llvm.org/D28746 Files: docs/ReleaseNotes.rst Index:

[PATCH] D28729: [clang-tidy] Add -enable-alpha-checks command

2017-01-16 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek updated this revision to Diff 84550. Prazek marked an inline comment as done. Prazek added a comment. -enable-alpha-checks is now not visible for users, but it make my life as clang-tidy developer much easier. https://reviews.llvm.org/D28729 Files: clang-tidy/ClangTidy.cpp

[PATCH] D28727: Add -fstrict-vtable-pointers to UserManual

2017-01-16 Thread Piotr Padlewski via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Prazek marked an inline comment as done. Closed by commit rL292112: Add -fstrict-vtable-pointers to UsersManual (authored by Prazek). Changed prior to commit: https://reviews.llvm.org/D28727?vs=84492=84551#toc

[PATCH] D28727: Add -fstrict-vtable-pointers to UserManual

2017-01-16 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. I also have sent it on the branch Repository: rL LLVM https://reviews.llvm.org/D28727 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28729: [clang-tidy] Add -enable-alpha-checks command

2017-01-16 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. Does solution like this works for you? We don't officially support alpha checkers, but it is much easier to check if something is already implemented in static analyzer easily https://reviews.llvm.org/D28729 ___

[PATCH] D28606: Mention devirtualization in release notes

2017-01-16 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek closed this revision. Prazek added a comment. Pushed on branch https://reviews.llvm.org/D28606 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28746: Mention ThinLTO with PGO in ReleaseNotes 4.0

2017-01-16 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek closed this revision. Prazek added a comment. Pushed on branch. Please check if everything looks good when RC will be released. https://reviews.llvm.org/D28746 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D28606: Mention devirtualization in release notes

2017-01-15 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek updated this revision to Diff 84491. Prazek added a comment. Added link to documentation https://reviews.llvm.org/D28606 Files: docs/ReleaseNotes.rst Index: docs/ReleaseNotes.rst === --- docs/ReleaseNotes.rst +++

[PATCH] D28729: [clang-tidy] Add -enable-alpha-checks command

2017-01-14 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. So the problem I got is that every time I want to check if there is already a feature in clang-tidy/static-analyzer that solves my issue, I either have to deal with static-analyzer command line, which is horrible, or I have to modify and recompile the source code. The

[PATCH] D28768: [clang-tidy] Add check 'modernize-return-braced-init-list'

2017-01-16 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. Thanks for the check. Have you run it on llvm? Comment at: clang-tidy/modernize/ReturnBracedInitListCheck.cpp:27-32 + auto soughtConstructExpr = + cxxConstructExpr(unless(isListInitialization())).bind("ctor"); + + auto hasConstructExpr =

[PATCH] D28768: [clang-tidy] Add check 'modernize-return-braced-init-list'

2017-01-17 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek accepted this revision. Prazek added a comment. This revision is now accepted and ready to land. Do you have some results from running it on LLVM? If nothing crashes and all fixit are correct then LGTM. Comment at:

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

2017-01-16 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added inline comments. Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:52-53 + for (const auto : NoExceptRanges) { +// FIXME use DiagnosticIDs::Level::Note +diag(NoExceptRange.getBegin(), "In function declared no-throw here:") +<<

[PATCH] D28768: [clang-tidy] Add check 'modernize-return-braced-init-list'

2017-01-16 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added inline comments. Comment at: test/clang-tidy/modernize-return-braced-init-list.cpp:95 +} + +vector f6() { please also add test that contains for function Type foo(): return call(Type()); return OtherType(Type()); // implicit conversion It would

[PATCH] D28729: [clang-tidy] Add -enable-alpha-checks command

2017-01-20 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. Ok, I didn't know that it is this easy to run alpha checks from clang. https://reviews.llvm.org/D28729 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27166: [clang-tidy] Enhance modernize-use-auto to templated function casts

2016-12-08 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a reviewer: staronj. Prazek added a comment. gosh, I found it too late. Jakub started to work on the same problem and he have made initial check for it. One of the problems he got was multideclarations. Does it work for cases like? long a = lexical_cast(z), b = lexical_cast(y);

[PATCH] D27752: Use after move bug fixes

2016-12-14 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek created this revision. Prazek added reviewers: rsmith, mboehme. Prazek added a subscriber: cfe-commits. Herald added a subscriber: klimek. Bunch of fixed bugs in Clang after running misc-use-after-move in clang-tidy. https://reviews.llvm.org/D27752 Files: lib/Format/Format.cpp

[PATCH] D27166: [clang-tidy] Enhance modernize-use-auto to templated function casts

2016-12-14 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. Cool! Have you run this check on LLVM & Clang to check if it works? I just runned it and I got this weird behavior lib/Analysis/AssumptionCache.cpp:120 -for (const BasicBlock : cast(*I.first)) +for (const BasicBlock : auto(*I.first))

[PATCH] D27806: [clang-tidy] Add obvious-invalid-range

2016-12-15 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek marked an inline comment as done. Prazek added a comment. In https://reviews.llvm.org/D27806#623838, @malcolm.parsons wrote: > What about arguments 3 and 4 of `std::list::splice(It, list, It, It)`? I am aware of that, but I wanted to make a bunch of simple checks to fill obvious module

[PATCH] D27806: [clang-tidy] Add misc-invalid-range

2016-12-15 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek updated this revision to Diff 81600. Prazek added a comment. - Small fixes https://reviews.llvm.org/D27806 Files: clang-tidy/obvious/CMakeLists.txt clang-tidy/obvious/InvalidRangeCheck.cpp clang-tidy/obvious/InvalidRangeCheck.h clang-tidy/obvious/ObviousTidyModule.cpp

[PATCH] D27806: [clang-tidy] Add obvious-invalid-range

2016-12-15 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek updated this revision to Diff 81604. Prazek added a comment. - Small fixes https://reviews.llvm.org/D27806 Files: clang-tidy/obvious/CMakeLists.txt clang-tidy/obvious/InvalidRangeCheck.cpp clang-tidy/obvious/InvalidRangeCheck.h clang-tidy/obvious/ObviousTidyModule.cpp

[PATCH] D27767: NFC Changes from modernize-use-auto

2016-12-15 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added inline comments. Comment at: lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp:106 if (Optional P = Succ->getLocationAs()) +if (const auto *BO = P->getStmtAs()) { malcolm.parsons wrote: > The check missed this one. I don't think it is

[PATCH] D27813: [clang-tidy] fix missing anchor for MPI Module

2016-12-15 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek created this revision. Prazek added reviewers: alexfh, Alexander_Droste, hokein. Prazek added a subscriber: cfe-commits. Herald added a subscriber: JDevlieghere. MPIModule was not linked to plugins https://reviews.llvm.org/D27813 Files: clang-tidy/plugin/ClangTidyPlugin.cpp Index:

[PATCH] D27806: [clang-tidy] Add misc-invalid-range

2016-12-15 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek marked an inline comment as done. Prazek added inline comments. Comment at: clang-tidy/ClangTidy.cpp:296 const auto = - AnalyzerOptions::getRegisteredCheckers(/*IncludeExperimental=*/false); +

[PATCH] D27815: [clang-tidy] Add obvious module for obvious bugs

2016-12-15 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek created this revision. Prazek added reviewers: alexfh, malcolm.parsons. Prazek added a subscriber: cfe-commits. Herald added subscribers: JDevlieghere, mgorny. Module for checks for obvious bugs that probably won't be found in working code, but can be found while looking for an obvious bug

[PATCH] D27806: [clang-tidy] Add misc-invalid-range

2016-12-15 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek marked 2 inline comments as done. Prazek added inline comments. Comment at: docs/ReleaseNotes.rst:84 +- New `misc-invalid-range + `_ check malcolm.parsons wrote: > "invalid" <

[PATCH] D27806: [clang-tidy] Add obvious-invalid-range

2016-12-15 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek updated this revision to Diff 81603. Prazek marked 2 inline comments as done. Prazek added a comment. - Small fixes https://reviews.llvm.org/D27806 Files: clang-tidy/obvious/CMakeLists.txt clang-tidy/obvious/InvalidRangeCheck.cpp clang-tidy/obvious/InvalidRangeCheck.h

[PATCH] D27806: [clang-tidy] Add obvious-invalid-range

2016-12-15 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek marked 3 inline comments as done. Prazek added inline comments. Comment at: docs/ReleaseNotes.rst:84 +- New `misc-invalid-range + `_ check malcolm.parsons wrote: > Prazek wrote: >

[PATCH] D24487: [Clang] Fix some Clang-tidy modernize-use-bool-literals and Include What You Use warnings in AST; other minor fixes

2016-12-15 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added inline comments. Comment at: lib/ASTMatchers/Dynamic/Parser.cpp:617 -} // namespace dynamic -} // namespace ast_matchers -} // namespace clang +} // end namespace dynamic +} // end namespace ast_matchers I think check should not warn in these

[PATCH] D27806: [clang-tidy] Add misc-invalid-range

2016-12-15 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek updated this revision to Diff 81597. Prazek added a comment. - Moved to obvious https://reviews.llvm.org/D27806 Files: clang-tidy/ClangTidy.cpp clang-tidy/obvious/CMakeLists.txt clang-tidy/obvious/InvalidRangeCheck.cpp clang-tidy/obvious/InvalidRangeCheck.h

[PATCH] D27806: [clang-tidy] Add obvious-invalid-range

2016-12-15 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. Also please review https://reviews.llvm.org/D27815 to make it happen https://reviews.llvm.org/D27806 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27806: [clang-tidy] Add obvious-invalid-range

2016-12-15 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek updated this revision to Diff 81617. Prazek marked an inline comment as done. Prazek added a comment. - Small fixes https://reviews.llvm.org/D27806 Files: clang-tidy/obvious/CMakeLists.txt clang-tidy/obvious/InvalidRangeCheck.cpp clang-tidy/obvious/InvalidRangeCheck.h

[PATCH] D27806: [clang-tidy] Add obvious-invalid-range

2016-12-15 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek updated this revision to Diff 81606. Prazek added a comment. - Small fixes https://reviews.llvm.org/D27806 Files: clang-tidy/obvious/CMakeLists.txt clang-tidy/obvious/InvalidRangeCheck.cpp clang-tidy/obvious/InvalidRangeCheck.h clang-tidy/obvious/ObviousTidyModule.cpp

[PATCH] D27813: [clang-tidy] fix missing anchor for MPI Module

2016-12-15 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. In https://reviews.llvm.org/D27813#623978, @Alexander_Droste wrote: > Thanks for adding! I also forgot about that one once. It is simple bugfix so I guess I need small LGTM to push it https://reviews.llvm.org/D27813 ___

[PATCH] D27806: [clang-tidy] Add obvious-invalid-range

2016-12-15 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added inline comments. Comment at: docs/clang-tidy/checks/list.rst:68 misc-inefficient-algorithm + misc-invalid-range misc-macro-parentheses malcolm.parsons wrote: > misc. > add_new_check.py can fix it. How? I added new check with this script,

[PATCH] D27806: [clang-tidy] Add obvious-invalid-range

2016-12-15 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added inline comments. Comment at: docs/clang-tidy/checks/list.rst:68 misc-inefficient-algorithm + misc-invalid-range misc-macro-parentheses malcolm.parsons wrote: > Prazek wrote: > > malcolm.parsons wrote: > > > misc. > > > add_new_check.py can

[PATCH] D27806: [clang-tidy] Add obvious-invalid-range

2016-12-15 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added inline comments. Comment at: test/clang-tidy/misc-invalid-range.cpp:41 + auto = v; + std::move(v.begin(), v2.end(), v2.begin()); + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: call to algorithm with begin and end from different objects [misc-invalid-range]

[PATCH] D27815: [clang-tidy] Add obvious module for obvious bugs

2016-12-15 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. In https://reviews.llvm.org/D27815#624294, @Eugene.Zelenko wrote: > I'm not sure that this is good name for module. > > Singe reason for this is check for STL algorithms, may be //stl// module is > more correct destination? The reason for this module is that I want to

[PATCH] D27815: [clang-tidy] Add obvious module for obvious bugs

2016-12-15 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. In https://reviews.llvm.org/D27815#624322, @Eugene.Zelenko wrote: > There are many other checks or compiler warnings which could be classified as > obvious bugs. Sure, but unfortunatelly they won't be able to become warning because of false positive rate. In the

[PATCH] D27806: [clang-tidy] Add obvious-invalid-range

2016-12-16 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. In https://reviews.llvm.org/D27806#624675, @Eugene.Zelenko wrote: > It may be good idea to create LLVM check which will suggest to use > STLExtras.h wrappers instead of STL algorithms. Sure, I actually didn't know about STLExtras, but anyway it would be something LLVM

[PATCH] D27813: [clang-tidy] fix missing anchor for MPI Module

2016-12-16 Thread Piotr Padlewski via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL289930: [clang-tidy] fix missing anchor for MPI Module (authored by Prazek). Changed prior to commit: https://reviews.llvm.org/D27813?vs=81593=81723#toc Repository: rL LLVM

[PATCH] D27806: [clang-tidy] Add misc-invalid-range

2016-12-15 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek created this revision. Prazek added reviewers: alexfh, malcolm.parsons. Prazek added a subscriber: cfe-commits. Herald added subscribers: JDevlieghere, mgorny. Warns if algorithm is used with ``.begin()`` and ``.end()`` from different variables. https://reviews.llvm.org/D27806 Files:

[PATCH] D27806: [clang-tidy] Add obvious-invalid-range

2016-12-16 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek marked 2 inline comments as done. Prazek added inline comments. Comment at: clang-tidy/obvious/InvalidRangeCheck.cpp:20-36 +"std::for_each; std::find; std::find_if; std::find_end; " +"std::find_first_of; std::adjacent_find; std::count; std::count_if;" +

[PATCH] D27700: [clang-tidy] refactor ExprSequence out of misc-use-after-move check

2016-12-13 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added inline comments. Comment at: clang-tidy/misc/UseAfterMoveCheck.cpp:18 using namespace clang::ast_matchers; +using namespace clang::tidy::utils; + I don't think it is required Comment at: clang-tidy/utils/ExprSequence.cpp:180-182

[PATCH] D27700: [clang-tidy] refactor ExprSequence out of misc-use-after-move check

2016-12-13 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added inline comments. Comment at: clang-tidy/misc/UseAfterMoveCheck.cpp:18 using namespace clang::ast_matchers; +using namespace clang::tidy::utils; + Prazek wrote: > I don't think it is required ok I guess I am wrong https://reviews.llvm.org/D27700

[PATCH] D27700: [clang-tidy] refactor ExprSequence out of misc-use-after-move check

2016-12-13 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added inline comments. Comment at: clang-tidy/utils/ExprSequence.cpp:154 +return SyntheticStmtSourceMap.lookup(S); + else +return S; alexfh wrote: > nit: No `else` after return, please. Not sure if he should change it in this patch - it is just

[PATCH] D27700: [clang-tidy] refactor ExprSequence out of misc-use-after-move check

2016-12-13 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added inline comments. Comment at: clang-tidy/utils/ExprSequence.cpp:52 + +bool isDescendantOrEqual(const Stmt *Descendant, const Stmt *Ancestor, + ASTContext *Context) { staronj wrote: > Shouldn't isDescendantOrEqual be static or

[PATCH] D27166: [clang-tidy] Enhance modernize-use-auto to templated function casts

2016-12-13 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. In https://reviews.llvm.org/D27166#617696, @malcolm.parsons wrote: > In https://reviews.llvm.org/D27166#617621, @Prazek wrote:. > > > Does it work for cases like? > > > Yes; `replaceExpr()` checks that the variable has the same unqualified type > as the initializer and

[PATCH] D27166: [clang-tidy] Enhance modernize-use-auto to templated function casts

2016-12-14 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek accepted this revision. Prazek added a comment. This revision is now accepted and ready to land. LGTM, thanks for all the fixes. Can you leave FIXIT comment somewhere in the code near the FixitHint about the function pointers? https://reviews.llvm.org/D27166

[PATCH] D21298: [Clang-tidy] delete null check

2016-12-14 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added inline comments. Comment at: clang-tidy/readability/DeleteNullPointerCheck.cpp:33 + Finder->addMatcher( + ifStmt(allOf(hasCondition( + anyOf(PointerCondition, BinaryPointerCheckCondition)), I think allOf matcher is

[PATCH] D27166: [clang-tidy] Enhance modernize-use-auto to templated function casts

2016-12-14 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. There is still one more problem: /home/prazek/llvm/lib/Analysis/ScalarEvolution.cpp:2442:11: warning: use auto when initializing with a template cast to avoid duplicating the type name [modernize-use-auto] const auto **O = SCEVAllocator.Allocate(Ops.size());

[PATCH] D27166: [clang-tidy] Enhance modernize-use-auto to templated function casts

2016-12-14 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. In https://reviews.llvm.org/D27166#622108, @malcolm.parsons wrote: > In https://reviews.llvm.org/D27166#622103, @Prazek wrote: > > > There is still one more problem: > > > > /home/prazek/llvm/lib/Analysis/ScalarEvolution.cpp:2442:11: warning: use > > auto when

[PATCH] D27806: [clang-tidy] Add obvious-invalid-range

2016-12-16 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek marked an inline comment as done. Prazek added inline comments. Comment at: clang-tidy/obvious/InvalidRangeCheck.cpp:20-36 +"std::for_each; std::find; std::find_if; std::find_end; " +"std::find_first_of; std::adjacent_find; std::count; std::count_if;" +

[PATCH] D27815: [clang-tidy] Add obvious module for obvious bugs

2016-12-16 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. In https://reviews.llvm.org/D27815#625102, @aaron.ballman wrote: > I am really not keen on the name "obvious" for this module. What is obvious > to one person is not always obvious to another. Also, if the checks are > finding *obvious bugs*, then that suggests they

[PATCH] D27806: [clang-tidy] Add obvious-invalid-range

2016-12-16 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added inline comments. Comment at: clang-tidy/obvious/InvalidRangeCheck.cpp:20-36 +"std::for_each; std::find; std::find_if; std::find_end; " +"std::find_first_of; std::adjacent_find; std::count; std::count_if;" +"std::mismatch; std::equal; std::search;

[PATCH] D28727: Add -fstrict-vtable-pointers to UserManual

2017-01-14 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek created this revision. Prazek added a reviewer: hans. Prazek added a subscriber: cfe-commits. It would be good to merge it to 4.0 branch. https://reviews.llvm.org/D28727 Files: docs/UsersManual.rst Index: docs/UsersManual.rst

[PATCH] D28729: [clang-tidy] Add -enable-alpha-checks command

2017-01-14 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek updated this revision to Diff 84448. Prazek added a comment. reformat https://reviews.llvm.org/D28729 Files: clang-tidy/ClangTidy.cpp clang-tidy/ClangTidyOptions.cpp clang-tidy/ClangTidyOptions.h clang-tidy/tool/ClangTidyMain.cpp test/clang-tidy/enable-alpha-checks.cpp Index:

[PATCH] D28729: [clang-tidy] Add -enable-alpha-checks command

2017-01-14 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek created this revision. Prazek added a reviewer: alexfh. Prazek added a subscriber: cfe-commits. Herald added a subscriber: JDevlieghere. https://reviews.llvm.org/D28729 Files: clang-tidy/ClangTidy.cpp clang-tidy/ClangTidyOptions.cpp clang-tidy/ClangTidyOptions.h

[PATCH] D28606: Mention devirtualization in release notes

2017-01-12 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek created this revision. Prazek added a reviewer: hans. Prazek added a subscriber: cfe-commits. https://reviews.llvm.org/D28606 Files: docs/ReleaseNotes.rst Index: docs/ReleaseNotes.rst === --- docs/ReleaseNotes.rst +++

[PATCH] D28727: Add -fstrict-vtable-pointers to UserManual

2017-01-15 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek updated this revision to Diff 84489. Prazek added a comment. suggested fix https://reviews.llvm.org/D28727 Files: docs/UsersManual.rst Index: docs/UsersManual.rst === --- docs/UsersManual.rst +++ docs/UsersManual.rst @@

[PATCH] D28727: Add -fstrict-vtable-pointers to UserManual

2017-01-15 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek updated this revision to Diff 84492. Prazek added a comment. Removed [no-] so hyperlink would work https://reviews.llvm.org/D28727 Files: docs/UsersManual.rst Index: docs/UsersManual.rst === --- docs/UsersManual.rst +++

[PATCH] D28310: Add virtual functions getter

2017-01-04 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek created this revision. Prazek added a reviewer: rjmccall. Prazek added a subscriber: cfe-commits. Small refactor https://reviews.llvm.org/D28310 Files: include/clang/AST/VTableBuilder.h lib/CodeGen/ItaniumCXXABI.cpp Index: lib/CodeGen/ItaniumCXXABI.cpp

[PATCH] D27210: [clang-tidy] misc-string-compare. Adding a new check to clang-tidy

2016-12-18 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. Do you need some help with implementing the other fixit, or you just need some extra time? It seems to be almost the same as your second fixit Comment at: clang-tidy/misc/StringCompareCheck.cpp:69-70 +diag(Matched->getLocStart(), + "do not

[PATCH] D27752: [clang] Use after move bug fixes

2016-12-23 Thread Piotr Padlewski via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL290424: Use after move bug fixes (authored by Prazek). Changed prior to commit: https://reviews.llvm.org/D27752?vs=81369=82408#toc Repository: rL LLVM https://reviews.llvm.org/D27752 Files:

[PATCH] D27806: [clang-tidy] Add obvious-invalid-range

2016-12-23 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added inline comments. Comment at: clang-tidy/obvious/InvalidRangeCheck.cpp:62 + auto CallsAlgorithm = hasDeclaration( + functionDecl(Names.size() > 0 ? hasAnyName(Names) : anything())); + alexfh wrote: > Does this check make sense without the

[PATCH] D28034: [ASTMatchers] Add hasInClassInitializer traversal matcher for FieldDecl.

2016-12-22 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added inline comments. Comment at: docs/LibASTMatchersReference.html:2442 }; -fieldDecl(isBitField()) +fieldDecl(hasBitWidth(2)) matches 'int a;' and 'int c;' but not 'int b;'. Fix not connected to patch? Comment at:

[PATCH] D27815: [clang-tidy] Add obvious module for obvious bugs

2016-12-19 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. The example of this kind of check is here: https://reviews.llvm.org/D27806 I am not sure if it make sense to put it as clang warning. After a little bit of thinking I guess name "typos" would be better, because I want to look for checks that are mostly typos (which are

[PATCH] D28746: Mention ThinLTO with PGO in ReleaseNotes 4.0

2017-01-15 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek marked 2 inline comments as done. Prazek added a comment. Cool, This still might require some small fixes - I haven't generate docs for it, so I will check it before release. https://reviews.llvm.org/D28746 ___ cfe-commits mailing list

[PATCH] D28746: Mention ThinLTO with PGO in ReleaseNotes 4.0

2017-01-15 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek updated this revision to Diff 84503. Prazek added a comment. update https://reviews.llvm.org/D28746 Files: docs/ReleaseNotes.rst Index: docs/ReleaseNotes.rst === --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@

[PATCH] D31830: Emit invariant.group.barrier when using union field

2017-04-10 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek created this revision. We need to emit barrier if the union field is CXXRecordDecl because it might have vptrs. The testcode was wrongly devirtualized. It also proves that having different groups for different dynamic types is not sufficient. https://reviews.llvm.org/D31830 Files:

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

2017-04-10 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. Yep, I am also a fan of "and","or" and "not". The other tokens doesn't make it more readable imho https://reviews.llvm.org/D31308 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D31830: Emit invariant.group.barrier when using union field

2017-04-12 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. ping https://reviews.llvm.org/D31830 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32110: Emit invariant.group loads with empty group md

2017-04-15 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek created this revision. As discussed here http://lists.llvm.org/pipermail/llvm-dev/2017-January/109332.html having different groups doesn't solve the problem entirly. https://reviews.llvm.org/D32110 Files: lib/CodeGen/CodeGenModule.cpp test/CodeGenCXX/invariant.group-for-vptrs.cpp

[PATCH] D31830: Emit invariant.group.barrier when using union field

2017-04-15 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. ping https://reviews.llvm.org/D31830 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32294: [clang-tidy] run-clang-tidy.py: check if clang-apply-replacements succeeds

2017-04-20 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. In https://reviews.llvm.org/D32294#732861, @kuhar wrote: > After thinking about Piotr's comment, I think that a better way to perform > the check would be te invoking clang-apply-replacements with `--version` and > seeing if it fails even before running clang-tidy. >

[PATCH] D32294: [clang-tidy] run-clang-tidy.py: check if clang-apply-replacements succeeds

2017-04-20 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek accepted this revision. Prazek added a comment. LGTM, but please leave the FIXME comment about the running of apply replacement https://reviews.llvm.org/D32294 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D31830: Emit invariant.group.barrier when using union field

2017-04-17 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added inline comments. Comment at: lib/CodeGen/CGExpr.cpp:3517 +CGM.getCodeGenOpts().StrictVTablePointers && +CGM.getCodeGenOpts().OptimizationLevel > 0) + addr = Address(Builder.CreateInvariantGroupBarrier(addr.getPointer()),

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

2017-04-23 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added inline comments. Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:93 + to(functionDecl(hasName("::std::make_pair" + + .bind("make_pair")); JonasToth wrote: > is the new line here necessary? i think it

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

2017-04-23 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added inline comments. Comment at: test/clang-tidy/modernize-use-emplace.cpp:284 + // CHECK-FIXES: v.emplace_back(42LL, 13); + + v.push_back(std::make_pair(0, 3)); I would add here test like: class X { X(std:;pair a) {} };

[PATCH] D32378: Insert invariant.group.barrier for pointers comparisons

2017-04-23 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. This is actually good catch, we also need to do it when inserting barrier in placement new. I think that for the ctors and dtors we can do it only with optimizations enabled, because if optimizations are not enabled then ctors and dtors won't have the invariant.group

[PATCH] D31830: Emit invariant.group.barrier when using union field

2017-04-23 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek updated this revision to Diff 96312. Prazek added a comment. - Inserting barrier with -O0 https://reviews.llvm.org/D31830 Files: lib/CodeGen/CGExpr.cpp test/CodeGenCXX/strict-vtable-pointers.cpp Index: test/CodeGenCXX/strict-vtable-pointers.cpp

[PATCH] D32378: Insert invariant.group.barrier for pointers comparisons

2017-04-23 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek marked an inline comment as done. Prazek added inline comments. Comment at: lib/CodeGen/CGExprScalar.cpp:3066-3067 +} else { // Unsigned integers and pointers. + if (CGF.CGM.getCodeGenOpts().StrictVTablePointers && +

[PATCH] D32378: Insert invariant.group.barrier for pointers comparisons

2017-04-23 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek updated this revision to Diff 96311. Prazek marked an inline comment as done. Prazek added a comment. Addressing Richard's comment https://reviews.llvm.org/D32378 Files: lib/CodeGen/CGExprScalar.cpp test/CodeGenCXX/strict-vtable-pointers.cpp Index:

[PATCH] D32401: [Devirtualization] insert placement new barrier with -O0

2017-04-23 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek created this revision. Herald added a subscriber: mehdi_amini. To not break LTO with different optimizations levels, we should insert the barrier regardles of optimization level. https://reviews.llvm.org/D32401 Files: lib/CodeGen/CGExprCXX.cpp Index: lib/CodeGen/CGExprCXX.cpp

  1   2   3   >