How to use static analysis to improve performance

Since you are asking for ideas for performance related checkers I thought I'd post a few small ideas.

1. Check for comparisons of .size() against 0 instead of calling .empty()/.isEmpty()

2. Check for use of post-increment where pre-increment could have been used just as well.

3. Check for loops that open-code std::fill() rather than calling the algorithm.

4. Check for loops that iterate column-major rather than row-major. Since the latter usually has better cache performance characteristics.

It would be great to have a check for passing shared_ptr parameters by value - and better still still to have a tool that converted such calls to passing by const reference!

• Heap variables that could be put on the stack (like a new at the beginning of a block and a delete at the end).
• Passing types by const& when it would be faster to pass them by value.
• Use of Qt containers in C++11 range-based for loops which implies a deep copy.
• Downcasts with static_cast instead of dynamic_ / qobject_
• Functions taking pointers in argument if they don't take ownership somewhere (they could take references instead).
• Functions taking non-const elements and which does not modify them (a bit tricky because for instance the function could be virtual and the variable could be modified in the same function in another child class)
• Elements initialized in the ctor instead of the member init-list

And more broadly-scoped ones : - Long types used in more than one place (e.g. QMap) which should be given a name instead - Code that would benefit from inlining (dunno if it's doable / computable ? maybe if it's very few LLVM instructions ?)

• Check for use of 'new' to initialize unique_ptr/shared_ptr rather than using make_unique/make_shared.

• Suggest using 'extern template' if the same template is instantiated in multiple compilation units (to improve build performance, not runtime performance).

• Suggest using emplace()/emplace_back() rather than insert()/push_back() if the object in question is an rvalue.

• Suggest removing virtual if a class has virtual functions but no other classes inherit from it and the virtual functions also do not override any from base classes.

• If a class has a move constructor that is not 'noexcept', suggest to make it so. So that it can be used with functions that use std::move_if_noexcept to determine whether to move or copy objects (for example std::vector::resize()).

It seems that your plug-in is made for checking the use of Qt.

It will be great to separate the different kinds of checkings you do.

You can make a plug-in with several kind of checkings

1) Check code before c++11 2) Check c++11 code 3) Check code for specific libraries (like Qt ...)

It will be great to separate the different kinds of checking into several plug-in.

Check if a class is following the "rule of zero" -- if it defines any of the 5 auto-generated functions (dtor, copy ctor and assignment, move ctor and assignment), it should declare all of them.

Even continuing to follow the "rule of three" under C++11 can leave some performance on the table because if one of the auto-generated functions appears, the compiler won't generate move operations implicitly.

Very Nice, I was considering using libclang to compute the "sizeof" of all the class/struct defined in a project. Your checks takes in account object usage which is even better.

Maybe integrate your checks with clang-tidy(http://clang.llvm.org/extra/clang-tidy/index.html). This tool is handy if you just want the analysis without the compilation/code-emission.

A few more ideas:

• check for uses of static's in header files. They will get a copy in multiple compilation units, and if the linker is not smart (enough) about removing them they can bloat your libraries and executables (which can have a performance impact).

• Check for classes/structs where shuffling the order of member variables can significantly reduce the amount of padding the compiler has to add. Smaller objects are always nice, but when reordering members can bring a class size down from occupying more than 1 cache line to less than one then there can really be significant performance gains.

• Look for code that uses "out parameters" that should just use a normal return value. The RVO and NRVO optimizations can kick in for normal returns - not so much for out parameters.

• Check for empty user-defined implementations dof destructors and default constructors and suggest that they either be deleted or defaulted (=default) instead. When user-defined they are by dwfinition never trivial, but when they are trivial they can often enable compiler optimizations that are otherwise not available (especially if making those functions trivial makes the entire type POD).

By the way. Would you appreciate outside contributions on the code or would you rather just get ideas and input and implement them yourself? I think a tool like this is a great idea and I'd be happy to contribute (with what little time I have (which may be zero)). But I won't bother if you'd rather just have suggestions and evolve it on your own...?

Hi,

need a bit of help please. I've downloaded clazy from https://quickgit.kde.org/?p=scratch%2Fsmartins%2Fclazy.git and clang from http://llvm.org/releases/download.html. Compilation failing here.

Downloads/scratch-smartins-clazy/Utils.cpp:859:48: error: no matching function for call to ‘std::vector::erase(__gnu_cxx::__normal_iterator<clang::DeclContext* const*, std::vector >&, __gnu_cxx::__normal_iterator<clang::DeclContext* const*, std::vector >)’
                 methodContexts.erase(it, it + 1);
                                                ^
Downloads/scratch-smartins-clazy/Utils.cpp:859:48: note: candidates are:
In file included from /usr/include/c++/4.8/vector:69:0,
                 from /usr/include/c++/4.8/bits/random.h:34,
                 from /usr/include/c++/4.8/random:50,
                 from /usr/include/c++/4.8/bits/stl_algo.h:65,
                 from /usr/include/c++/4.8/algorithm:62,
                 from /usr/include/llvm/ADT/SmallVector.h:22,
                 from /usr/include/llvm/Support/Allocator.h:24,
                 from /usr/include/clang/AST/ASTVector.h:23,
                 from /usr/include/clang/AST/ASTUnresolvedSet.h:18,
                 from /usr/include/clang/AST/DeclCXX.h:19,
                 from Downloads/scratch-smartins-clazy/Utils.h:31,
                 from Downloads/scratch-smartins-clazy/Utils.cpp:28:
/usr/include/c++/4.8/bits/vector.tcc:134:5: note: std::vector::iterator std::vector::erase(std::vector::iterator) [with _Tp = clang::DeclContext*; _Alloc = std::allocator; std::vector::iterator = __gnu_cxx::__normal_iterator<clang::DeclContext**, std::vector >; typename std::_Vector_base::pointer = clang::DeclContext**]
     vector::
     ^
/usr/include/c++/4.8/bits/vector.tcc:134:5: note:   candidate expects 1 argument, 2 provided
/usr/include/c++/4.8/bits/vector.tcc:146:5: note: std::vector::iterator std::vector::erase(std::vector::iterator, std::vector::iterator) [with _Tp = clang::DeclContext*; _Alloc = std::allocator; std::vector::iterator = __gnu_cxx::__normal_iterator<clang::DeclContext**, std::vector >; typename std::_Vector_base::pointer = clang::DeclContext**]
     vector::
     ^
/usr/include/c++/4.8/bits/vector.tcc:146:5: note:   no known conversion for argument 1 from ‘__gnu_cxx::__normal_iterator<clang::DeclContext* const*, std::vector >’ to ‘std::vector::iterator {aka __gnu_cxx::__normal_iterator<clang::DeclContext**, std::vector >}’
make[2]: *** [CMakeFiles/ClangLazy.dir/Utils.cpp.o] Error 1
make[1]: *** [CMakeFiles/ClangLazy.dir/all] Error 2
make: *** [all] Error 2

Thanks in advance.

Fantastic stuff - I'm loving it!

One small issue: with the function-args-by-ref check, I see a warnings generated whenever the Q_DECLARE_OPERATORS_FOR_FLAGS macro is used. Could this macro be ignored for this test?

[  1%] Building CXX object src/thirdparty/qtlockedfile/CMakeFiles/qtlockedfile.dir/qtlockedfile.cpp.o
terminate called after throwing an instance of 'std::bad_alloc'
  what():  std::bad_alloc
0  clang-3.7       0x00000000013e06b8 llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 40
1  clang-3.7       0x00000000013e1a1b
2  libpthread.so.0 0x00007ff6acdf1d10
3  libc.so.6       0x00007ff6ac1ab267 gsignal + 55
4  libc.so.6       0x00007ff6ac1aceca abort + 362
5  libstdc++.so.6  0x00007ff6ac7e6b7d __gnu_cxx::__verbose_terminate_handler() + 365
6  libstdc++.so.6  0x00007ff6ac7e49c6
7  libstdc++.so.6  0x00007ff6ac7e4a11
8  libstdc++.so.6  0x00007ff6ac7e4c29
9  libstdc++.so.6  0x00007ff6ac7e51cc
10 ClangLazy.so    0x00007ff6ab3a3553
11 ClangLazy.so    0x00007ff6ab39fc4b OldStyleConnect::OldStyleConnect(std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&) + 107
12 ClangLazy.so    0x00007ff6ab3a3233
13 ClangLazy.so    0x00007ff6ab39154f CheckManager::createCheck(std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&) + 319
14 ClangLazy.so    0x00007ff6ab3924d4 CheckManager::createChecks(std::vector<std::__cxx11::basic_string<char, std::char_traits, std::allocator >, std::allocator<std::__cxx11::basic_string<char, std::char_traits, std::allocator > > >) + 516
15 ClangLazy.so    0x00007ff6ab3c9ce1
16 clang-3.7       0x000000000173b4ea clang::FrontendAction::CreateWrappedASTConsumer(clang::CompilerInstance&, llvm::StringRef) + 330
17 clang-3.7       0x000000000173c092 clang::FrontendAction::BeginSourceFile(clang::CompilerInstance&, clang::FrontendInputFile const&) + 2578
18 clang-3.7       0x000000000170d4c7 clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) + 695
19 clang-3.7       0x00000000017a9de3 clang::ExecuteCompilerInvocation(clang::CompilerInstance*) + 3123
20 clang-3.7       0x00000000006f6634 cc1_main(llvm::ArrayRef, char const*, void*) + 1156
21 clang-3.7       0x00000000006f586f main + 11983
22 libc.so.6       0x00007ff6ac196a40 __libc_start_main + 240
23 clang-3.7       0x00000000006f2893

using prebuild clang from llvm website, ubuntu 15.10. with cmake command:

cmake .. -DCMAKE_INSTALL_PREFIX=/opt/dev_qt5 -DCMAKE_BUILD_TYPE=Debug -DCMAKE_CXX_FLAGS_DEBUG="-Xclang -load -Xclang /opt/clang37/lib/ClangLazy.so -Xclang -add-plugin -Xclang clang-lazy" -DCMAKE_PREFIX_PATH=/opt/clang37

Thanks for this great tool ! I am getting a lot of "c++11 range-loop might detach Qt container", especially when iterating on member variables in non const member function. What would be the best way to make sure it does not detach ? The only I can come up is "for(const auto &e: static_cast(this)->member)", but it feels ugly

A lot of suggested checks are already implemented in Clang-tidy(http://clang.llvm.org/extra/clang-tidy/). C++11 migration checks were moved in there recently.

By the word, it may make sense to port Clazy checks to Clang-tidy.