[PATCH] D55289: [analyzer] MoveChecker Pt.5: Improve invalidation policies.

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

Cool!



================
Comment at: lib/StaticAnalyzer/Checkers/MoveChecker.cpp:528
     ArrayRef<const MemRegion *> ExplicitRegions,
     ArrayRef<const MemRegion *> Regions, const LocationContext *LCtx,
     const CallEvent *Call) const {
----------------
This isn't specific to this revision, but I find the parameter name `Regions` way too vague. Maybe `ImplicitRegions`?


================
Comment at: lib/StaticAnalyzer/Checkers/MoveChecker.cpp:540-542
+    // Explicit regions are the regions passed into the call directly, but
+    // not all of them end up being invalidated. The ones that do appear in
+    // the Regions array as well.
----------------
Really? Then I guess this needs to be updated in `CheckerDocumentation.cpp`:

```
  /// \param ExplicitRegions The regions explicitly requested for invalidation.
  ///        For a function call, this would be the arguments. For a bind, this
  ///        would be the region being bound to.
```

To me, this clearly indicates that the elements of `ExplicitRegions` will be invalidated. Does "requested for" really just mean "requested for potential"? Since this happens //before// any invalidation, it could easily be interpreted as "invalidated after the end of the callback".


Repository:
  rC Clang

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

https://reviews.llvm.org/D55289