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+maxew...@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+maxew...@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+maxew...@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+maxew...@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}} } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits