[PATCH] D32207: Corrrect warn_unused_result attribute

[Erich Keane via Phabricator via cfe-commits]

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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D32207.95664.patch
Type: text/x-patch
Size: 3086 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170419/5c02da98/attachment.bin>