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

[Aaron Ballman via cfe-commits]

aaron.ballman updated this revision to Diff 32697.
aaron.ballman added a comment.

In http://reviews.llvm.org/D11784#228764, @alexfh wrote:

> 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).


That should get us closer, but doesn't quite cut it. I think I may be able to combine this with CXXRecordDecl::isTriviallyCopyable() to get us close enough to cut down on the number of false positives. Thank you for pointing this out, it is implemented in this patch.

> > 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?


With the triviality implementation, I now get zero false positives (and zero true positives) in the Clang and LLVM source base.


http://reviews.llvm.org/D11784

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/MoveConstructorInitCheck.cpp
  clang-tidy/misc/MoveConstructorInitCheck.h
  test/clang-tidy/misc-move-constructor-init.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D11784.32697.patch
Type: text/x-patch
Size: 7950 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150820/63c5f01c/attachment.bin>