[PATCH] D32207: Corrrect warn_unused_result attribute

[Erich Keane via Phabricator via cfe-commits]

erichkeane marked an inline comment as done.
erichkeane added inline comments.


================
Comment at: lib/AST/Decl.cpp:3010
+        (OpCode == OO_PlusPlus || OpCode == OO_MinusMinus) &&
+        (this->getNumParams() + (isa<CXXMethodDecl>(this) ? 1 : 0)) == 2;
+    if (Ret && !IsPostfix) {
----------------
aaron.ballman wrote:
> CXXMethodDecl represents a static or instance method, so I'm not certain this logic is quite correct.
I actually stole this logic from lib/Sema/SemaDeclCXX.cpp lines 12708 and 12754.  I believe the logic here is that it operators aren't allowed to be static?  Since it is illegal to do so,  I would expect that I'd simply need to do an assert to prevent it?

I'm open for ideas if you have one here, I'm not sure what the 'right' thing to do is.


================
Comment at: test/SemaCXX/warn-unused-result.cpp:166
+// are special-cased to not warn for return-type due to ubiquitousness.
+struct[[clang::warn_unused_result]] S {
+  S DoThing() { return {}; };
----------------
aaron.ballman wrote:
> Can you also add a test with [[nodiscard]] and verify that the uses of increment/decrement *are* diagnosed?
I can add that, it will obviously cause a slight change in logic since I didn't handle that separately.  Are we surewe WANT [[nodiscard]] to warn in this case?


https://reviews.llvm.org/D32207