Use builtin recognition to detect std::move / std::forward. (#4668)

Replaces some prior ad-hoc detection strategies and generally cleans up
a little. No functional change intended.

(cherry picked from commit f6a5ab6)

Co-authored-by: Richard Smith <[email protected]>

Author: ahatanaka

clang/include/clang/AST/Expr.h

@@ -3128,11 +3128,7 @@ class CallExpr : public Expr {
     setDependence(getDependence() | ExprDependence::TypeValueInstantiation);
   }
 
-  bool isCallToStdMove() const {
-    const FunctionDecl *FD = getDirectCallee();
-    return getNumArgs() == 1 && FD && FD->isInStdNamespace() &&
-           FD->getIdentifier() && FD->getIdentifier()->isStr("move");
-  }
+  bool isCallToStdMove() const;
 
   static bool classof(const Stmt *T) {
     return T->getStmtClass() >= firstCallExprConstant &&

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -4924,7 +4924,7 @@ def ext_adl_only_template_id : ExtWarn<
   "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">>;
+  "unqualified call to '%0'">, InGroup<DiagGroup<"unqualified-std-cast-call">>;
 
 // C++ Template Argument Lists
 def err_template_missing_args : Error<

clang/lib/AST/Expr.cpp

@@ -1478,8 +1478,7 @@ Decl *Expr::getReferencedDeclOfCallee() {
 
 /// If this is a call to a builtin, return the builtin ID. If not, return 0.
 unsigned CallExpr::getBuiltinCallee() const {
-  auto *FDecl =
-      dyn_cast_or_null<FunctionDecl>(getCallee()->getReferencedDeclOfCallee());
+  auto *FDecl = getDirectCallee();
   return FDecl ? FDecl->getBuiltinID() : 0;
 }
 
@@ -3337,9 +3336,9 @@ bool Expr::isConstantInitializer(ASTContext &Ctx, bool IsForRef,
 }
 
 bool CallExpr::isBuiltinAssumeFalse(const ASTContext &Ctx) const {
-  const FunctionDecl* FD = getDirectCallee();
-  if (!FD || (FD->getBuiltinID() != Builtin::BI__assume &&
-              FD->getBuiltinID() != Builtin::BI__builtin_assume))
+  unsigned BuiltinID = getBuiltinCallee();
+  if (BuiltinID != Builtin::BI__assume &&
+      BuiltinID != Builtin::BI__builtin_assume)
     return false;
 
   const Expr* Arg = getArg(0);
@@ -3348,6 +3347,10 @@ bool CallExpr::isBuiltinAssumeFalse(const ASTContext &Ctx) const {
          Arg->EvaluateAsBooleanCondition(ArgVal, Ctx) && !ArgVal;
 }
 
+bool CallExpr::isCallToStdMove() const {
+  return getBuiltinCallee() == Builtin::BImove;
+}
+
 namespace {
   /// Look for any side effects within a Stmt.
   class SideEffectFinder : public ConstEvaluatedExprVisitor<SideEffectFinder> {

clang/lib/Sema/SemaExpr.cpp

@@ -6570,18 +6570,17 @@ static void DiagnosedUnqualifiedCallsToStdFunctions(Sema &S, CallExpr *Call) {
   if (DRE->getQualifier())
     return;
 
-  NamedDecl *D = dyn_cast_or_null<NamedDecl>(Call->getCalleeDecl());
-  if (!D || !D->isInStdNamespace())
+  const FunctionDecl *FD = Call->getDirectCallee();
+  if (!FD)
     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))
+  unsigned BuiltinID = FD->getBuiltinID();
+  if (BuiltinID != Builtin::BImove && BuiltinID != Builtin::BIforward)
     return;
 
   S.Diag(DRE->getLocation(), diag::warn_unqualified_call_to_std_cast_function)
-      << D->getQualifiedNameAsString()
+      << FD->getQualifiedNameAsString()
       << FixItHint::CreateInsertion(DRE->getLocation(), "std::");
 }
 

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

@@ -16,8 +16,8 @@ using namespace std;
 
 void f() {
   int i = 0;
-  (void)move(i); // expected-warning {{unqualified call to std::move}}
+  (void)move(i); // expected-warning {{unqualified call to 'std::move}}
   // CHECK: {{^}}  (void)std::move
-  (void)forward(i); // expected-warning {{unqualified call to std::forward}}
+  (void)forward(i); // expected-warning {{unqualified call to 'std::forward}}
   // CHECK: {{^}}  (void)std::forward
 }

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

@@ -25,23 +25,23 @@ 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}}
+  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}}
+  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}}
+  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}}
+  forward<decltype(foo)>(foo); // expected-warning{{unqualified call to 'std::forward}}
   move(1, 2);
   dummy(foo);
 }
@@ -59,16 +59,16 @@ using std::move;
 
 void f() {
   int i = 0;
-  move(i); // expected-warning{{unqualified call to std::move}}
+  move(i); // expected-warning{{unqualified call to 'std::move}}
   move(1, 2);
-  forward<int>(i); // expected-warning{{unqualified call to std::forward}}
+  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(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);
 }
 
@@ -90,7 +90,7 @@ void f() {
 
 namespace adl {
 void f() {
-  move(std::foo{}); // expected-warning{{unqualified call to std::move}}
+  move(std::foo{}); // expected-warning{{unqualified call to 'std::move}}
 }
 
 } // namespace adl
@@ -99,8 +99,8 @@ 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}}
+  move(i);         // expected-warning{{unqualified call to 'std::move}}
+  forward<int>(i); // expected-warning{{unqualified call to 'std::forward}}
 }
 
 } // namespace std
@@ -110,9 +110,9 @@ namespace alias = std;
 using namespace alias;
 void f() {
   int i = 0;
-  move(i); // expected-warning{{unqualified call to std::move}}
+  move(i); // expected-warning{{unqualified call to 'std::move}}
   move(1, 2);
-  forward<int>(i); // expected-warning{{unqualified call to std::forward}}
+  forward<int>(i); // expected-warning{{unqualified call to 'std::forward}}
 }
 
 } // namespace test_alias

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

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