lebedev.ri created this revision. lebedev.ri added reviewers: aaron.ballman, rsmith, rtrieu, rjmccall, dblaikie.
I have seen such a problem when reviewing https://reviews.llvm.org/D43341. https://godbolt.org/g/aJYcaa #include <utility> struct S {}; void test(S a) { std::move(a); } Since `std::move()` is not marked with `nodiscard` attribute in the standard (should it be? how complicated would it be to write such a proposal?), nothing diagnoses such code. But i really don't see why one would intentionally write that. You have either forgot to assign/pass the result, or you wanted to cast to `void`. Stage-2 self-hosting is green, no preparatory changes needed! Repository: rC Clang https://reviews.llvm.org/D45163 Files: docs/ReleaseNotes.rst include/clang/Basic/DiagnosticSemaKinds.td lib/AST/Expr.cpp lib/Sema/SemaStmt.cpp test/Analysis/MisusedMovedObject.cpp test/SemaCXX/warn-unused-result-nodiscard.cpp test/SemaCXX/warn-unused-xvalue.cpp
Index: test/SemaCXX/warn-unused-xvalue.cpp =================================================================== --- /dev/null +++ test/SemaCXX/warn-unused-xvalue.cpp @@ -0,0 +1,97 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s + +// definitions for std::move +namespace std { +inline namespace foo { +template <class T> +struct remove_reference { typedef T type; }; +template <class T> +struct remove_reference<T &> { typedef T type; }; +template <class T> +struct remove_reference<T &&> { typedef T type; }; + +template <class T> +typename remove_reference<T>::type &&move(T &&t); +} // namespace foo +} // namespace std + +int f0(); +int &&f1(); + +int global; + +void test_negative() { + // Ok. Should not warn + f0(); + f1(); + + static_cast<int>(f0()); // expected-warning {{expression result unused}} + static_cast<int>(f1()); // expected-warning {{expression result unused}} + + static_cast<int>(global); // expected-warning {{expression result unused}} + + (void)std::move(global); + (void)noexcept(std::move(global)); + (void)sizeof(std::move(global)); +} + +// Unevaluated contexts should not trigger unused result warnings. +template <typename T> +auto foo(T) -> decltype(std::move(global), bool()) { // Should not warn. + return true; +} + +void test_simple() { + int x = 1; + + static_cast<int &&>(x); // expected-warning {{ignoring return value of cast to rvalue reference of type 'int'}} + std::move(x); // expected-warning {{ignoring return value of std::move() function returning rvalue reference of type 'int'}} + + using std::move; + move(x); // expected-warning {{ignoring return value of std::move() function returning rvalue reference of type 'int'}} +} + +void test_global() { + static_cast<int &&>(global); // expected-warning {{ignoring return value of cast to rvalue reference of type 'int'}} + std::move(global); // expected-warning {{ignoring return value of std::move() function returning rvalue reference of type 'int'}} + + using std::move; + move(global); // expected-warning {{ignoring return value of std::move() function returning rvalue reference of type 'int'}} +} + +void test_call() { + static_cast<int &&>(f0()); // expected-warning {{ignoring return value of cast to rvalue reference of type 'int'}} + static_cast<int &&>(f1()); // expected-warning {{ignoring return value of cast to rvalue reference of type 'int'}} + std::move(f0()); // expected-warning {{ignoring return value of std::move() function returning rvalue reference of type 'int'}} + std::move(f1()); // expected-warning {{ignoring return value of std::move() function returning rvalue reference of type 'int'}} + + using std::move; + move(f0()); // expected-warning {{ignoring return value of std::move() function returning rvalue reference of type 'int'}} + move(f1()); // expected-warning {{ignoring return value of std::move() function returning rvalue reference of type 'int'}} +} + +void arg(int a) { + static_cast<int &&>(a); // expected-warning {{ignoring return value of cast to rvalue reference of type 'int'}} + + std::move(a); // expected-warning {{ignoring return value of std::move() function returning rvalue reference of type 'int'}} +} + +struct A {}; +struct B { + A a; +}; +struct C { + C(){}; + ~C() {} +}; +void struct_test() { + A a; + std::move(a); // expected-warning{{ignoring return value of std::move() function returning rvalue reference of type 'A'}} + + B b; + std::move(b); // expected-warning{{ignoring return value of std::move() function returning rvalue reference of type 'B'}} + std::move(b.a); // expected-warning{{ignoring return value of std::move() function returning rvalue reference of type 'A'}} + + C c; + std::move(c); // expected-warning{{ignoring return value of std::move() function returning rvalue reference of type 'C'}} +} Index: test/SemaCXX/warn-unused-result-nodiscard.cpp =================================================================== --- /dev/null +++ test/SemaCXX/warn-unused-result-nodiscard.cpp @@ -0,0 +1,44 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 %s + +// definitions for somthing similar to the std::move. +namespace notstd { +inline namespace foo { +template <class T> +struct remove_reference { typedef T type; }; +template <class T> +struct remove_reference<T &> { typedef T type; }; +template <class T> +struct remove_reference<T &&> { typedef T type; }; + +// This is just like the std::move(), but marked with warn_unused_result attribute +template <class T> +__attribute__((warn_unused_result)) typename remove_reference<T>::type &¬move1(T &&t); +template <class T> +[[clang::warn_unused_result]] typename remove_reference<T>::type &¬move2(T &&t); +template <class T> +[[nodiscard]] typename remove_reference<T>::type &¬move3(T &&t); +} // namespace foo +} // namespace notstd + +// definitions for std::move, but actually marked with nodiscard attribute +namespace std { +inline namespace foo { +template <class T> +struct remove_reference { typedef T type; }; +template <class T> +struct remove_reference<T &> { typedef T type; }; +template <class T> +struct remove_reference<T &&> { typedef T type; }; + +template <class T> +[[nodiscard]] typename remove_reference<T>::type &&move(T &&t); +} // namespace foo +} // namespace std + +void arg(int a) { + notstd::notmove1(a); // expected-warning {{ignoring return value of function declared with 'warn_unused_result' attribute}} + notstd::notmove2(a); // expected-warning {{ignoring return value of function declared with 'warn_unused_result' attribute}} + notstd::notmove3(a); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}} + + std::move(a); // expected-warning {{ignoring return value of std::move() function returning rvalue reference of type 'int'}} +} Index: test/Analysis/MisusedMovedObject.cpp =================================================================== --- test/Analysis/MisusedMovedObject.cpp +++ test/Analysis/MisusedMovedObject.cpp @@ -185,7 +185,7 @@ { A a; if (i == 1) { // expected-note {{Taking false branch}} expected-note {{Taking false branch}} - std::move(a); + std::move(a); // expected-warning {{ignoring return value of std::move() function returning rvalue reference of type 'A'}} } if (i == 2) { // expected-note {{Taking false branch}} expected-note {{Taking false branch}} a = A(); @@ -409,7 +409,7 @@ // Moves of global variables are not reported. A global_a; void globalVariablesTest() { - std::move(global_a); + std::move(global_a); // expected-warning {{ignoring return value of std::move() function returning rvalue reference of type 'A'}} global_a.foo(); // no-warning } Index: lib/Sema/SemaStmt.cpp =================================================================== --- lib/Sema/SemaStmt.cpp +++ lib/Sema/SemaStmt.cpp @@ -264,6 +264,10 @@ // is written in a macro body, only warn if it has the warn_unused_result // attribute. if (const Decl *FD = CE->getCalleeDecl()) { + if (CE->isCallToStdMove()) { + Diag(Loc, diag::warn_unused_stdmove) << E->getType().getCanonicalType(); + return; + } if (const Attr *A = isa<FunctionDecl>(FD) ? cast<FunctionDecl>(FD)->getUnusedResultAttr() : FD->getAttr<WarnUnusedResultAttr>()) { @@ -329,6 +333,11 @@ } } + if (E->isXValue() && isa<CastExpr>(E)) { + Diag(Loc, diag::warn_unused_xvalue_cast) << E->getType().getCanonicalType(); + return; + } + if (E->isGLValue() && E->getType().isVolatileQualified()) { Diag(Loc, diag::warn_unused_volatile) << R1 << R2; return; Index: lib/AST/Expr.cpp =================================================================== --- lib/AST/Expr.cpp +++ lib/AST/Expr.cpp @@ -2196,7 +2196,7 @@ // // Note: If new cases are added here, DiagnoseUnusedExprResult should be // updated to match for QoI. - if (HasWarnUnusedResultAttr || + if (CE->isCallToStdMove() || HasWarnUnusedResultAttr || FD->hasAttr<PureAttr>() || FD->hasAttr<ConstAttr>()) { WarnE = this; Loc = CE->getCallee()->getLocStart(); Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -7277,6 +7277,12 @@ def warn_unused_call : Warning< "ignoring return value of function declared with %0 attribute">, InGroup<UnusedValue>; +def warn_unused_stdmove : Warning< + "ignoring return value of std::move() function returning rvalue reference of type %0">, + InGroup<UnusedValue>; +def warn_unused_xvalue_cast : Warning< + "ignoring return value of cast to rvalue reference of type %0">, + InGroup<UnusedValue>; def warn_side_effects_unevaluated_context : Warning< "expression with side effects has no effect in an unevaluated context">, InGroup<UnevaluatedExpression>; Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -59,6 +59,9 @@ ``-Wno-c++98-compat-extra-semi``, so if you want that diagnostic, you need to explicitly re-enable it (e.g. by appending ``-Wextra-semi``). +- ``-Wunused-value`` diagnostic was taught to detect unused return value of + ``std::move()`` function. + Non-comprehensive list of changes in this release -------------------------------------------------
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits