[PATCH] D23353: [clang-tidy] Add check 'misc-use-after-move'

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

A few initial comments.


================
Comment at: clang-tidy/misc/UseAfterMoveCheck.cpp:492
@@ +491,3 @@
+  DeclRefs->clear();
+  for (CFGBlock::const_iterator I = Block->begin(), E = Block->end(); I != E;
+       ++I) {
----------------
Any reason to avoid range-based for loops here?

================
Comment at: clang-tidy/misc/UseAfterMoveCheck.cpp:505
@@ +504,3 @@
+    for (const auto &Match : Matches) {
+      const DeclRefExpr *DeclRef = Match.getNodeAs<DeclRefExpr>("declref");
+      if (DeclRef && BlockMap->blockContainingStmt(DeclRef) == Block) {
----------------
The type is specified verbatim in the initializer, so it's better to use `const auto *` here.

================
Comment at: clang-tidy/misc/UseAfterMoveCheck.cpp:506
@@ +505,3 @@
+      const DeclRefExpr *DeclRef = Match.getNodeAs<DeclRefExpr>("declref");
+      if (DeclRef && BlockMap->blockContainingStmt(DeclRef) == Block) {
+        DeclRefs->insert(DeclRef);
----------------
It's uncommon to use braces for single-line `if`/`for`/... bodies in LLVM/Clang code.

================
Comment at: docs/clang-tidy/checks/misc-use-after-move.rst:51
@@ +50,3 @@
+In some cases, the check may not be able to detect that two branches are
+mutually exclusive. For example (assuming that ``i`` is an int):
+
----------------
Eugene.Zelenko wrote:
> i is not language construct, please use `.
It's an inline code snippet, isn't it? I'd format it as code (double backquotes).

================
Comment at: docs/clang-tidy/checks/misc-use-after-move.rst:190
@@ +189,3 @@
+
+The check will not consider ``s`` to be reinitialized after the last line;
+instead, the line that assigns to ``s.str`` will be flagged as a use-after-move.
----------------
Eugene.Zelenko wrote:
> s, s.str and S are not language construct. Please use `.
Seems fine to format these as code as well.


Repository:
  rL LLVM

https://reviews.llvm.org/D23353