[clang] 70b1f6d - [clang] Warn on unqualified calls to std::move and std::forward
Erich Keane via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 24 07:23:43 PST 2022
Author: Corentin Jabot
Date: 2022-02-24T07:23:39-08:00
New Revision: 70b1f6de539867353940d3dcb8b25786d5082d63
URL: https://github.com/llvm/llvm-project/commit/70b1f6de539867353940d3dcb8b25786d5082d63
DIFF: https://github.com/llvm/llvm-project/commit/70b1f6de539867353940d3dcb8b25786d5082d63.diff
LOG: [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
Added:
clang/test/SemaCXX/unqualified-std-call-fixits.cpp
clang/test/SemaCXX/unqualified-std-call.cpp
Modified:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaExpr.cpp
clang/test/SemaCXX/warn-self-move.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 11fcd5ff5a323..15487260eb732 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/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 "
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 88fc89bec629a..454e21ecfa2d9 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/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;
}
diff --git a/clang/test/SemaCXX/unqualified-std-call-fixits.cpp b/clang/test/SemaCXX/unqualified-std-call-fixits.cpp
new file mode 100644
index 0000000000000..0b2d70c0360a3
--- /dev/null
+++ b/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::
+}
diff --git a/clang/test/SemaCXX/unqualified-std-call.cpp b/clang/test/SemaCXX/unqualified-std-call.cpp
new file mode 100644
index 0000000000000..fa66ae9f8e321
--- /dev/null
+++ b/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
diff --git a/clang/test/SemaCXX/warn-self-move.cpp b/clang/test/SemaCXX/warn-self-move.cpp
index 23778c1871132..6f7ff08664356 100644
--- a/clang/test/SemaCXX/warn-self-move.cpp
+++ b/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 {
More information about the cfe-commits
mailing list