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

This adds a diagnostic when an unqualified call is resolved
to std::move or std::forward.

This follows some C++ committee discussions where some
people where concerns that this might be an usual anti pattern
particularly britle worth warning about - both because move
is a common name and because these functions accept any values.

This warns inconditionnally of whether the current context is in
std:: or not, as implementations probably want to always qualify
these calls too, to avoid triggering adl accidentally.

Differential Revision: https://reviews.llvm.org/D119670

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -4804,6 +4804,9 @@ def ext_adl_only_template_id : ExtWarn<
   "use of function template name with no prior declaration in function call "
   "with explicit template arguments is a C++20 extension">, InGroup<CXX20>;
 
+def warn_unqualified_call_to_std_cast_function : Warning<
+  "unqualified call to %0">, InGroup<DiagGroup<"unqualified-std-cast-call">>;
+
 // C++ Template Argument Lists
 def err_template_missing_args : Error<
   "use of "

clang/lib/Sema/SemaExpr.cpp

@@ -6450,6 +6450,38 @@ tryImplicitlyCaptureThisIfImplicitMemberFunctionAccessWithDependentArgs(
   }
 }
 
+// Once a call is fully resolved, warn for unqualified calls to specific
+// C++ standard functions, like move and forward.
+static void DiagnosedUnqualifiedCallsToStdFunctions(Sema &S, CallExpr *Call) {
+  // We are only checking unary move and forward so exit early here.
+  if (Call->getNumArgs() != 1)
+    return;
+
+  Expr *E = Call->getCallee()->IgnoreParenImpCasts();
+  if (!E || isa<UnresolvedLookupExpr>(E))
+    return;
+  DeclRefExpr *DRE = dyn_cast_or_null<DeclRefExpr>(E);
+  if (!DRE || !DRE->getLocation().isValid())
+    return;
+
+  if (DRE->getQualifier())
+    return;
+
+  NamedDecl *D = dyn_cast_or_null<NamedDecl>(Call->getCalleeDecl());
+  if (!D || !D->isInStdNamespace())
+    return;
+
+  // Only warn for some functions deemed more frequent or problematic.
+  static constexpr llvm::StringRef SpecialFunctions[] = {"move", "forward"};
+  auto it = llvm::find(SpecialFunctions, D->getName());
+  if (it == std::end(SpecialFunctions))
+    return;
+
+  S.Diag(DRE->getLocation(), diag::warn_unqualified_call_to_std_cast_function)
+      << D->getQualifiedNameAsString()
+      << FixItHint::CreateInsertion(DRE->getLocation(), "std::");
+}
+
 ExprResult Sema::ActOnCallExpr(Scope *Scope, Expr *Fn, SourceLocation LParenLoc,
                                MultiExprArg ArgExprs, SourceLocation RParenLoc,
                                Expr *ExecConfig) {
@@ -6474,7 +6506,11 @@ ExprResult Sema::ActOnCallExpr(Scope *Scope, Expr *Fn, SourceLocation LParenLoc,
   if (LangOpts.OpenMP)
     Call = ActOnOpenMPCall(Call, Scope, LParenLoc, ArgExprs, RParenLoc,
                            ExecConfig);
-
+  if (LangOpts.CPlusPlus) {
+    CallExpr *CE = dyn_cast<CallExpr>(Call.get());
+    if (CE)
+      DiagnosedUnqualifiedCallsToStdFunctions(*this, CE);
+  }
   return Call;
 }
 

clang/test/SemaCXX/unqualified-std-call-fixits.cpp

@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -verify -std=c++20 -Wall %s
+// RUN: cp %s %t
+// RUN: %clang_cc1 -x c++ -std=c++20 -fixit %t
+// RUN: %clang_cc1 -Wall -Werror -x c++ -std=c++20 %t
+// RUN: cat %t | FileCheck %s
+
+namespace std {
+
+void move(auto &&a) {}
+
+void forward(auto &a) {}
+
+} // namespace std
+
+using namespace std;
+
+void f() {
+  int i = 0;
+  move(i); // expected-warning {{unqualified call to std::move}}
+  // CHECK: {{^}}  std::
+  forward(i); // expected-warning {{unqualified call to std::forward}}
+              // CHECK: {{^}}  std::
+}

clang/test/SemaCXX/unqualified-std-call.cpp

@@ -0,0 +1,118 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wall -std=c++11 %s
+
+namespace std {
+
+template <typename T>
+void dummy(T &&) {}
+template <typename T>
+void move(T &&) {}
+template <typename T, typename U>
+void move(T &&, U &&) {}
+
+inline namespace __1 {
+template <typename T>
+void forward(T &) {}
+} // namespace __1
+
+struct foo {};
+
+} // namespace std
+
+namespace global {
+
+using namespace std;
+
+void f() {
+  int i = 0;
+  std::move(i);
+  move(i);   // expected-warning{{unqualified call to std::move}}
+  (move)(i); // expected-warning{{unqualified call to std::move}}
+  std::dummy(1);
+  dummy(1);
+  std::move(1, 2);
+  move(1, 2);
+  forward<int>(i); // expected-warning{{unqualified call to std::forward}}
+  std::forward<int>(i);
+}
+
+template <typename T>
+void g(T &&foo) {
+  std::move(foo);
+  move(foo); // expected-warning{{unqualified call to std::move}}
+
+  std::forward<decltype(foo)>(foo);
+  forward<decltype(foo)>(foo); // expected-warning{{unqualified call to std::forward}}
+  move(1, 2);
+  dummy(foo);
+}
+
+void call() {
+  g(0); //expected-note {{here}}
+}
+
+} // namespace global
+
+namespace named {
+
+using std::forward;
+using std::move;
+
+void f() {
+  int i = 0;
+  move(i); // expected-warning{{unqualified call to std::move}}
+  move(1, 2);
+  forward<int>(i); // expected-warning{{unqualified call to std::forward}}
+}
+
+template <typename T>
+void g(T &&foo) {
+  move(foo);                     // expected-warning{{unqualified call to std::move}}
+  forward<decltype(foo)>(foo);   // expected-warning{{unqualified call to std::forward}}
+  (forward<decltype(foo)>)(foo); // expected-warning{{unqualified call to std::forward}}
+  move(1, 2);
+}
+
+void call() {
+  g(0); //expected-note {{here}}
+}
+
+} // namespace named
+
+namespace overload {
+using namespace std;
+template <typename T>
+int move(T &&);
+void f() {
+  int i = 0;
+  move(i);
+}
+} // namespace overload
+
+namespace adl {
+void f() {
+  move(std::foo{}); // expected-warning{{unqualified call to std::move}}
+}
+
+} // namespace adl
+
+namespace std {
+
+void f() {
+  int i = 0;
+  move(i);         // expected-warning{{unqualified call to std::move}}
+  forward<int>(i); // expected-warning{{unqualified call to std::forward}}
+}
+
+} // namespace std
+
+namespace test_alias {
+namespace alias = std;
+using namespace alias;
+void f() {
+  int i = 0;
+  move(i); // expected-warning{{unqualified call to std::move}}
+  move(1, 2);
+  forward<int>(i); // expected-warning{{unqualified call to std::forward}}
+}
+
+} // namespace test_alias

clang/test/SemaCXX/warn-self-move.cpp

@@ -17,7 +17,8 @@ void int_test() {
   (x) = std::move(x);  // expected-warning{{explicitly moving}}
 
   using std::move;
-  x = move(x);  // expected-warning{{explicitly moving}}
+  x = move(x); // expected-warning{{explicitly moving}} \
+                   expected-warning {{unqualified call to std::move}}
 }
 
 int global;
@@ -26,7 +27,8 @@ void global_int_test() {
   (global) = std::move(global);  // expected-warning{{explicitly moving}}
 
   using std::move;
-  global = move(global);  // expected-warning{{explicitly moving}}
+  global = move(global); // expected-warning{{explicitly moving}} \
+                             expected-warning {{unqualified call to std::move}}
 }
 
 class field_test {