[PATCH] D119670: [clang] Warn on unqualified calls to std::move and std::forward

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

Thanks for working on this! I'm overall very happy, but I don't see a test case for a unary `move` originating from another namespace.



================
Comment at: clang/lib/Sema/SemaExpr.cpp:6422
+static void DiagnosedUnqualifiedCallsToStdFunctions(Sema &S, CallExpr *Call) {
+  // We are only checking move and forward so exit early here
+  if (Call->getNumArgs() != 1)
----------------



================
Comment at: clang/lib/Sema/SemaExpr.cpp:6442
+  static constexpr llvm::StringRef SpecialFunctions[] = {"move", "forward"};
+  auto it = std::find(std::begin(SpecialFunctions), std::end(SpecialFunctions),
+                      D->getName());
----------------
There's a range-based `llvm::find` in `llvm/STLExtras.h`.


================
Comment at: clang/test/SemaCXX/warn-self-move.cpp:19
 
   using std::move;
+  x = move(x); // expected-warning{{explicitly moving}} \
----------------
Perhaps this should warn if the algorithm `std::move` isn't seen by the compiler (that isn't a request for this patch).


================
Comment at: clang/test/SemaCXX/warn-self-move.cpp:20-21
   using std::move;
-  x = move(x);  // expected-warning{{explicitly moving}}
+  x = move(x); // expected-warning{{explicitly moving}} \
+                   expected-warning {{unqualified call to std::move}}
 }
----------------
Rationale: there's less mental gymnastics when backslashes aren't involved.


================
Comment at: clang/test/SemaCXX/warn-self-move.cpp:30-31
   using std::move;
-  global = move(global);  // expected-warning{{explicitly moving}}
+  global = move(global); // expected-warning{{explicitly moving}} \
+                             expected-warning {{unqualified call to std::move}}
 }
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119670