[clang] [clang][Sema] Warn on self move for inlined static cast (PR #76646)
Max Winkler via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 5 17:23:07 PST 2024
https://github.com/MaxEW707 updated https://github.com/llvm/llvm-project/pull/76646
>From a081f8266f24405523e6d283318bd898fd2d376a Mon Sep 17 00:00:00 2001
From: MaxEW707 <82551778+MaxEW707 at users.noreply.github.com>
Date: Sat, 30 Dec 2023 22:00:38 -0500
Subject: [PATCH 1/4] Warn on self move for inlined static cast
---
clang/lib/Sema/SemaChecking.cpp | 19 ++++++++++++++-----
clang/test/SemaCXX/warn-self-move.cpp | 13 +++++++++++++
2 files changed, 27 insertions(+), 5 deletions(-)
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 2a69325f029514..a21410434d8099 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -18843,17 +18843,26 @@ void Sema::DiagnoseSelfMove(const Expr *LHSExpr, const Expr *RHSExpr,
LHSExpr = LHSExpr->IgnoreParenImpCasts();
RHSExpr = RHSExpr->IgnoreParenImpCasts();
- // Check for a call expression
+ // Check for a call expression or static_cast expression
const CallExpr *CE = dyn_cast<CallExpr>(RHSExpr);
- if (!CE || CE->getNumArgs() != 1)
+ const CXXStaticCastExpr *CXXSCE = dyn_cast<CXXStaticCastExpr>(RHSExpr);
+ if (!CE && !CXXSCE)
return;
// Check for a call to std::move
- if (!CE->isCallToStdMove())
+ if (CE && (CE->getNumArgs() != 1 || !CE->isCallToStdMove()))
return;
- // Get argument from std::move
- RHSExpr = CE->getArg(0);
+ // Check for a static_cast<T&&>(..) to an xvalue which we can treat as an
+ // inlined std::move
+ if (CXXSCE && !CXXSCE->isXValue())
+ return;
+
+ // Get argument from std::move or static_cast
+ if (CE)
+ RHSExpr = CE->getArg(0);
+ else
+ RHSExpr = CXXSCE->getSubExpr();
const DeclRefExpr *LHSDeclRef = dyn_cast<DeclRefExpr>(LHSExpr);
const DeclRefExpr *RHSDeclRef = dyn_cast<DeclRefExpr>(RHSExpr);
diff --git a/clang/test/SemaCXX/warn-self-move.cpp b/clang/test/SemaCXX/warn-self-move.cpp
index 0987e9b6bf6017..d0158626424142 100644
--- a/clang/test/SemaCXX/warn-self-move.cpp
+++ b/clang/test/SemaCXX/warn-self-move.cpp
@@ -16,6 +16,9 @@ void int_test() {
x = std::move(x); // expected-warning{{explicitly moving}}
(x) = std::move(x); // expected-warning{{explicitly moving}}
+ x = static_cast<int&&>(x); // expected-warning{{explicitly moving}}
+ (x) = static_cast<int&&>(x); // expected-warning{{explicitly moving}}
+
using std::move;
x = move(x); // expected-warning{{explicitly moving}} \
expected-warning {{unqualified call to 'std::move}}
@@ -26,6 +29,9 @@ void global_int_test() {
global = std::move(global); // expected-warning{{explicitly moving}}
(global) = std::move(global); // expected-warning{{explicitly moving}}
+ global = static_cast<int&&>(global); // expected-warning{{explicitly moving}}
+ (global) = static_cast<int&&>(global); // expected-warning{{explicitly moving}}
+
using std::move;
global = move(global); // expected-warning{{explicitly moving}} \
expected-warning {{unqualified call to 'std::move}}
@@ -35,11 +41,14 @@ class field_test {
int x;
field_test(field_test&& other) {
x = std::move(x); // expected-warning{{explicitly moving}}
+ x = static_cast<int&&>(x); // expected-warning{{explicitly moving}}
x = std::move(other.x);
other.x = std::move(x);
other.x = std::move(other.x); // expected-warning{{explicitly moving}}
+ other.x = static_cast<int&&>(other.x); // expected-warning{{explicitly moving}}
}
void withSuggest(int x) {
+ x = static_cast<int&&>(x); // expected-warning{{explicitly moving variable of type 'int' to itself; did you mean to move to member 'x'?}}
x = std::move(x); // expected-warning{{explicitly moving variable of type 'int' to itself; did you mean to move to member 'x'?}}
}
};
@@ -50,11 +59,15 @@ struct C { C() {}; ~C() {} };
void struct_test() {
A a;
a = std::move(a); // expected-warning{{explicitly moving}}
+ a = static_cast<A&&>(a); // expected-warning{{explicitly moving}}
B b;
b = std::move(b); // expected-warning{{explicitly moving}}
+ b = static_cast<B&&>(b); // expected-warning{{explicitly moving}}
b.a = std::move(b.a); // expected-warning{{explicitly moving}}
+ b.a = static_cast<A&&>(b.a); // expected-warning{{explicitly moving}}
C c;
c = std::move(c); // expected-warning{{explicitly moving}}
+ c = static_cast<C&&>(c); // expected-warning{{explicitly moving}}
}
>From ecd9d5bf0d3fa0bc3e64266c6e7586da25438652 Mon Sep 17 00:00:00 2001
From: MaxEW707 <82551778+MaxEW707 at users.noreply.github.com>
Date: Sun, 31 Dec 2023 17:47:03 -0500
Subject: [PATCH 2/4] use auto
---
clang/lib/Sema/SemaChecking.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index a21410434d8099..ecb3269d0d30c7 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -18845,7 +18845,7 @@ void Sema::DiagnoseSelfMove(const Expr *LHSExpr, const Expr *RHSExpr,
// Check for a call expression or static_cast expression
const CallExpr *CE = dyn_cast<CallExpr>(RHSExpr);
- const CXXStaticCastExpr *CXXSCE = dyn_cast<CXXStaticCastExpr>(RHSExpr);
+ const auto *CXXSCE = dyn_cast<CXXStaticCastExpr>(RHSExpr);
if (!CE && !CXXSCE)
return;
>From a02d21f4e013efd823a0b078950948b534e31e8e Mon Sep 17 00:00:00 2001
From: MaxEW707 <82551778+MaxEW707 at users.noreply.github.com>
Date: Tue, 5 Mar 2024 19:51:00 -0500
Subject: [PATCH 3/4] PR feedback on reducing logic
---
clang/lib/Sema/SemaChecking.cpp | 26 ++++++++------------------
1 file changed, 8 insertions(+), 18 deletions(-)
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index ecb3269d0d30c7..c3f2650113f6c9 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -18843,26 +18843,16 @@ void Sema::DiagnoseSelfMove(const Expr *LHSExpr, const Expr *RHSExpr,
LHSExpr = LHSExpr->IgnoreParenImpCasts();
RHSExpr = RHSExpr->IgnoreParenImpCasts();
- // Check for a call expression or static_cast expression
- const CallExpr *CE = dyn_cast<CallExpr>(RHSExpr);
- const auto *CXXSCE = dyn_cast<CXXStaticCastExpr>(RHSExpr);
- if (!CE && !CXXSCE)
- return;
-
- // Check for a call to std::move
- if (CE && (CE->getNumArgs() != 1 || !CE->isCallToStdMove()))
- return;
-
- // Check for a static_cast<T&&>(..) to an xvalue which we can treat as an
- // inlined std::move
- if (CXXSCE && !CXXSCE->isXValue())
- return;
-
- // Get argument from std::move or static_cast
- if (CE)
+ // Check for a call to std::move or for a static_cast<T&&>(..) to an xvalue
+ // which we can treat as an inlined std::move
+ if (const auto *CE = dyn_cast<CallExpr>(RHSExpr);
+ CE && CE->getNumArgs() == 1 && CE->isCallToStdMove())
RHSExpr = CE->getArg(0);
- else
+ else if (const auto *CXXSCE = dyn_cast<CXXStaticCastExpr>(RHSExpr);
+ CXXSCE && CXXSCE->isXValue())
RHSExpr = CXXSCE->getSubExpr();
+ else
+ return;
const DeclRefExpr *LHSDeclRef = dyn_cast<DeclRefExpr>(LHSExpr);
const DeclRefExpr *RHSDeclRef = dyn_cast<DeclRefExpr>(RHSExpr);
>From bfaded8c9ba48e436a9061afcc5c48920cb6c21c Mon Sep 17 00:00:00 2001
From: MaxEW707 <82551778+MaxEW707 at users.noreply.github.com>
Date: Tue, 5 Mar 2024 20:22:52 -0500
Subject: [PATCH 4/4] Add unit tests that shouldn't warn
---
clang/test/SemaCXX/warn-self-move.cpp | 2 ++
1 file changed, 2 insertions(+)
diff --git a/clang/test/SemaCXX/warn-self-move.cpp b/clang/test/SemaCXX/warn-self-move.cpp
index d0158626424142..5937bb705ed227 100644
--- a/clang/test/SemaCXX/warn-self-move.cpp
+++ b/clang/test/SemaCXX/warn-self-move.cpp
@@ -43,7 +43,9 @@ class field_test {
x = std::move(x); // expected-warning{{explicitly moving}}
x = static_cast<int&&>(x); // expected-warning{{explicitly moving}}
x = std::move(other.x);
+ x = static_cast<int&&>(other.x);
other.x = std::move(x);
+ other.x = static_cast<int&&>(x);
other.x = std::move(other.x); // expected-warning{{explicitly moving}}
other.x = static_cast<int&&>(other.x); // expected-warning{{explicitly moving}}
}
More information about the cfe-commits
mailing list