[PATCH] D54556: [analyzer] MoveChecker Pt.1: Give MisusedMovedObject checker a more consistent name.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet, rnkovacs, Szelethus.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho, baloghadamsoftware, mgorny.
NoQ edited the summary of this revision.
NoQ added a dependency: D54372: [analyzer] MisusedMovedObject: NFC: Remove dead code after D18860.

I'd like to try to enable the use-after-move checker developed heroically by Peter "@szepet" Szecsi by default. While currently being a good opt-in lint check for enforcing coding style, currently it has too many false positives for an on-by-default check. The false positives seem to concentrate around objects that are fine to be used after they were moved from, even if no "state reset" method has been called on them, simply because their respective move-constructor/move-assignment leaves the object that is being moved in a well-specified state (eg., a null smart pointer or an empty set). This is not the case for STL objects, but it is the case for many custom objects.

I'll proceed to post a few patches and i'd love to hear your thoughts! The plan (of which this patch implements the first step) is as follows:

1. Rename the checker. In our tradition, "XChecker" usually means a checker that checks if "X" is used correctly, not a checker that finds "X". Eg., MallocChecker warns on misuse of malloc()/free(), but we don't call it "MisusedMallocChecker". The proposed name is "MoveChecker", i.e. checker that checks moves.
2. Address false positives under an on-by-default checker option. The proposed idea is to only warn by default in the following three cases:
  - If the object is a known STL object - because we know that it's left in unspecified state after move and we can memorize all state reset methods.
  - If the object is a local (or parameter) variable and hence there's no temptation to re-use the storage instead of making another local variable.
  - If the object is taken from a local (or parameter) rvalue reference, for the same reason.
3. Make warning messages more specific based information provided by facilities introduced on step 2.
4. Add a few missing state-reset methods.
5. Enable the checker, as restricted on step 2, by default. Keep the aggressive variant around in case someone needs it, but i don't have any specific plans for it.


Repository:
  rC Clang

https://reviews.llvm.org/D54556

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
  lib/StaticAnalyzer/Checkers/MoveChecker.cpp
  test/Analysis/MisusedMovedObject.cpp
  test/Analysis/use-after-move.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D54556.174123.patch
Type: text/x-patch
Size: 8510 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20181115/56afcaf3/attachment.bin>