erichkeane created this revision. The original idea was that if the attribute on an operator, that the return-value unused-ness wouldn't matter. However, all of the operators except postfix inc/dec return references! References don't result in this warning anyway, so those are already excluded.
However, the existing patch(in addition to missing a bunch of valid cases, such as a function returning a copy) would still hit non-member operators anyway! This patch removes the previous condition (if our current function is in the warn-unused-result'ed type) and replaces it with one that explicitly checks for post inc/dec (since these are likely going to be SO common that warning on them would be extreme). https://reviews.llvm.org/D32207 Files: include/clang/AST/Decl.h lib/AST/Decl.cpp test/SemaCXX/warn-unused-result.cpp Index: test/SemaCXX/warn-unused-result.cpp =================================================================== --- test/SemaCXX/warn-unused-result.cpp +++ test/SemaCXX/warn-unused-result.cpp @@ -160,3 +160,41 @@ (void)noexcept(f(), false); // Should not warn. } } +namespace { +// C++ Methods should warn even in their own class. Post increment/decrement +// are special-cased to not warn for return-type due to ubiquitousness. +struct[[clang::warn_unused_result]] S { + S DoThing() { return {}; }; + S operator++(int) { return {}; }; + S operator--(int) { return {}; }; + S operator++() { return {}; }; + S operator--() { return {}; }; +}; + +struct[[clang::warn_unused_result]] P { + P DoThing() { return {}; }; +}; + +P operator++(const P &, int) { return {}; }; +P operator++(const P &) { return {}; }; +P operator--(const P &, int) { return {}; }; +P operator--(const P &) { return {}; }; + +void f() { + S s; + P p; + s.DoThing(); // expected-warning {{ignoring return value}} + p.DoThing(); // expected-warning {{ignoring return value}} + // Special case the following to not warn. + s++; + s--; + p++; + p--; + // Improperly written prefix operators should still warn. + ++s; // expected-warning {{ignoring return value}} + --s; // expected-warning {{ignoring return value}} + ++p; // expected-warning {{ignoring return value}} + --p; // expected-warning {{ignoring return value}} + +} +} Index: lib/AST/Decl.cpp =================================================================== --- lib/AST/Decl.cpp +++ lib/AST/Decl.cpp @@ -3004,8 +3004,11 @@ QualType RetType = getReturnType(); if (RetType->isRecordType()) { const CXXRecordDecl *Ret = RetType->getAsCXXRecordDecl(); - const auto *MD = dyn_cast<CXXMethodDecl>(this); - if (Ret && !(MD && MD->getCorrespondingMethodInClass(Ret, true))) { + auto OpCode = getOverloadedOperator(); + bool IsPostfix = + (OpCode == OO_PlusPlus || OpCode == OO_MinusMinus) && + (this->getNumParams() + (isa<CXXMethodDecl>(this) ? 1 : 0)) == 2; + if (Ret && !IsPostfix) { if (const auto *R = Ret->getAttr<WarnUnusedResultAttr>()) return R; } Index: include/clang/AST/Decl.h =================================================================== --- include/clang/AST/Decl.h +++ include/clang/AST/Decl.h @@ -2082,10 +2082,9 @@ const Attr *getUnusedResultAttr() const; /// \brief Returns true if this function or its return type has the - /// warn_unused_result attribute. If the return type has the attribute and - /// this function is a method of the return type's class, then false will be - /// returned to avoid spurious warnings on member methods such as assignment - /// operators. + /// warn_unused_result attribute. If the return type has the attribute, + /// and this function is a post-increment operator, then false will be returned + /// to avoid valid, albeit ubiquitous warnings. bool hasUnusedResultAttr() const { return getUnusedResultAttr() != nullptr; } /// \brief Returns the storage class as written in the source. For the
Index: test/SemaCXX/warn-unused-result.cpp =================================================================== --- test/SemaCXX/warn-unused-result.cpp +++ test/SemaCXX/warn-unused-result.cpp @@ -160,3 +160,41 @@ (void)noexcept(f(), false); // Should not warn. } } +namespace { +// C++ Methods should warn even in their own class. Post increment/decrement +// are special-cased to not warn for return-type due to ubiquitousness. +struct[[clang::warn_unused_result]] S { + S DoThing() { return {}; }; + S operator++(int) { return {}; }; + S operator--(int) { return {}; }; + S operator++() { return {}; }; + S operator--() { return {}; }; +}; + +struct[[clang::warn_unused_result]] P { + P DoThing() { return {}; }; +}; + +P operator++(const P &, int) { return {}; }; +P operator++(const P &) { return {}; }; +P operator--(const P &, int) { return {}; }; +P operator--(const P &) { return {}; }; + +void f() { + S s; + P p; + s.DoThing(); // expected-warning {{ignoring return value}} + p.DoThing(); // expected-warning {{ignoring return value}} + // Special case the following to not warn. + s++; + s--; + p++; + p--; + // Improperly written prefix operators should still warn. + ++s; // expected-warning {{ignoring return value}} + --s; // expected-warning {{ignoring return value}} + ++p; // expected-warning {{ignoring return value}} + --p; // expected-warning {{ignoring return value}} + +} +} Index: lib/AST/Decl.cpp =================================================================== --- lib/AST/Decl.cpp +++ lib/AST/Decl.cpp @@ -3004,8 +3004,11 @@ QualType RetType = getReturnType(); if (RetType->isRecordType()) { const CXXRecordDecl *Ret = RetType->getAsCXXRecordDecl(); - const auto *MD = dyn_cast<CXXMethodDecl>(this); - if (Ret && !(MD && MD->getCorrespondingMethodInClass(Ret, true))) { + auto OpCode = getOverloadedOperator(); + bool IsPostfix = + (OpCode == OO_PlusPlus || OpCode == OO_MinusMinus) && + (this->getNumParams() + (isa<CXXMethodDecl>(this) ? 1 : 0)) == 2; + if (Ret && !IsPostfix) { if (const auto *R = Ret->getAttr<WarnUnusedResultAttr>()) return R; } Index: include/clang/AST/Decl.h =================================================================== --- include/clang/AST/Decl.h +++ include/clang/AST/Decl.h @@ -2082,10 +2082,9 @@ const Attr *getUnusedResultAttr() const; /// \brief Returns true if this function or its return type has the - /// warn_unused_result attribute. If the return type has the attribute and - /// this function is a method of the return type's class, then false will be - /// returned to avoid spurious warnings on member methods such as assignment - /// operators. + /// warn_unused_result attribute. If the return type has the attribute, + /// and this function is a post-increment operator, then false will be returned + /// to avoid valid, albeit ubiquitous warnings. bool hasUnusedResultAttr() const { return getUnusedResultAttr() != nullptr; } /// \brief Returns the storage class as written in the source. For the
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits