[PATCH] D11784: [PATCH] clang-tidy check for incorrect move constructor initializers

Thu Aug 20 05:13:53 PDT 2015

alexfh added a comment.

In http://reviews.llvm.org/D11784#227939, @aaron.ballman wrote:

> Addressed review comments. I re-ran the updated patch against LLVM and Clang, and there were some more false positives that I would like to address if possible. It seems my previous run against the source base was before expanding the scope of the patch to include more than just base class initialization (sorry for the earlier misinformation).
>
> 1. SourceMgr.h:58 is an example where the checker issues a diagnostic (for IncludeLoc), but given the triviality of the type, I don't see a reason to diagnose. However, this requires support from Sema, so I think a FIXME may be the best I can do. Note, adding a call to std::move() in these instances is not wrong, it's just not particularly useful.

Determining whether a type is trivially-copyable can be done on the AST level without Sema (see QualType::isTriviallyCopyableType).

> 2. we should not be warning on anything an implicit constructor does. For instance LiveQueryResult is triggering this because of SlotIndex. This should be fixed with this patch.

> 

>   Running over Clang and LLVM, there are 7 distinct false positives (repeated due to being in header files) and they all relate to triviality. The total false positive count was 832 of which two warnings (SourceMgr.h and Preprocessor.h) accounted for probably close to 90% of the diagnostics. This time around there were no true positives.

Can you list the list of unique locations?

================
Comment at: test/clang-tidy/misc-move-constructor-init.cpp:65
@@ +64,3 @@
+struct K {}; // Has implicit copy and move constructors
+struct L : K {
+  // CHECK: :[[@LINE+1]]:16: warning: move constructor initializes base class by calling a copy constructor [misc-move-constructor-init]
----------------
Please try to use the python script from http://reviews.llvm.org/D12180.

http://reviews.llvm.org/D11784