[PATCH] D55387: [analyzer] MoveChecker Pt.7: NFC: Misc refactoring.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.


================
Comment at: lib/StaticAnalyzer/Checkers/MoveChecker.cpp:265
 
+void MoveChecker::checkUse(ProgramStateRef State, const MemRegion *Region,
+                           const CXXRecordDecl *RD, MisuseKind MK,
----------------
a_sidorin wrote:
> I think that if the function is named "checkUse()", committing State changes is not what is really expected from it. Should we rename it or change the logic somehow? For example, return true if a report was emitted and add transition in the caller?
Good point. Renamed to `modelUse()` because the `addTransition()` logic becomes more complicated in the next patch, so i didn't want to duplicate it on all those call sites.


================
Comment at: lib/StaticAnalyzer/Checkers/MoveChecker.cpp:272
+  if (!RS || isAnyBaseRegionReported(State, Region)
+          || isInMoveSafeContext(C.getLocationContext())) {
+    // Finalize changes made by the caller.
----------------
a_sidorin wrote:
> This formatting is different from what clang-format does.
This gets overwritten in the next patch anyway. But imho this is more fancy than what clang-format does.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55387/new/

https://reviews.llvm.org/D55387