[clang] f6a5ab6 - Use builtin recognition to detect std::move / std::forward.

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 21 14:21:19 PDT 2022


Author: Richard Smith
Date: 2022-04-21T14:21:07-07:00
New Revision: f6a5ab6c8c316fa4f60e40030586e230920a63ea

URL: https://github.com/llvm/llvm-project/commit/f6a5ab6c8c316fa4f60e40030586e230920a63ea
DIFF: https://github.com/llvm/llvm-project/commit/f6a5ab6c8c316fa4f60e40030586e230920a63ea.diff

LOG: Use builtin recognition to detect std::move / std::forward.

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

Added: 
    

Modified: 
    clang/include/clang/AST/Expr.h
    clang/include/clang/Basic/DiagnosticSemaKinds.td
    clang/lib/AST/Expr.cpp
    clang/lib/Sema/SemaExpr.cpp
    clang/test/SemaCXX/unqualified-std-call-fixits.cpp
    clang/test/SemaCXX/unqualified-std-call.cpp
    clang/test/SemaCXX/warn-self-move.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index b4f76fd10e6cd..574d2fad216ea 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/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 &&

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 26e6db82a47f4..69093a6e51dc3 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4852,7 +4852,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<

diff  --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 2a086131c0a03..f761d79340797 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/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> {

diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 3bd83b8136869..90f96a3660dea 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -6555,18 +6555,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::");
 }
 

diff  --git a/clang/test/SemaCXX/unqualified-std-call-fixits.cpp b/clang/test/SemaCXX/unqualified-std-call-fixits.cpp
index d6f8e5e2b95ed..1a1ffc7ba2e82 100644
--- a/clang/test/SemaCXX/unqualified-std-call-fixits.cpp
+++ b/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
 }

diff  --git a/clang/test/SemaCXX/unqualified-std-call.cpp b/clang/test/SemaCXX/unqualified-std-call.cpp
index 0c78c26d063a9..7cb50e378daee 100644
--- a/clang/test/SemaCXX/unqualified-std-call.cpp
+++ b/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

diff  --git a/clang/test/SemaCXX/warn-self-move.cpp b/clang/test/SemaCXX/warn-self-move.cpp
index 6f7ff08664356..f1bcf6e051976 100644
--- a/clang/test/SemaCXX/warn-self-move.cpp
+++ b/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 {


        


More information about the cfe-commits mailing list