aaron.ballman added a comment. In https://reviews.llvm.org/D32207#730681, @erichkeane wrote:
> 1 more thing: I excepted post-increment/decrement from this warning because > I figured it would give a TON of warnings from people using post-inc/dec in > for loops. > > However, after further discussion with Craig (who brought this up on the > mailing list), I severely wonder if that is the 'right' thing to do. Someone > who is going to mark their struct in this way would likely ALSO mean it in > this case, right? > > Aaron: Should I simply remove the special casing all together, so that it > warns on post inc/dec? That would definitely solve all your concerns in the > review so far. I think that would make sense, yes. ================ Comment at: lib/AST/Decl.cpp:3010 + (OpCode == OO_PlusPlus || OpCode == OO_MinusMinus) && + (this->getNumParams() + (isa<CXXMethodDecl>(this) ? 1 : 0)) == 2; + if (Ret && !IsPostfix) { ---------------- erichkeane wrote: > 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. I was wondering more how a friend declaration of the operator might show up. e.g., ``` struct S { friend S operator++(S Operand, int); }; ``` ================ 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 {}; }; ---------------- erichkeane wrote: > 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? I think so, yes. The C++ standard doesn't provide normative requirements here, but its encouragement to implementers doesn't mention this kind of behavior. [dcl.attr.nodiscard]p2: ``` A nodiscard call is a function call expression that calls a function previously declared nodiscard, or whose return type is a possibly cv-qualified class or enumeration type marked nodiscard. Appearance of a nodiscard call as a potentially-evaluated discarded-value expression (Clause 8) is discouraged unless explicitly cast to void. Implementations are encouraged to issue a warning in such cases. This is typically because discarding the return value of a nodiscard call has surprising consequences. ``` Since this is returning a class type that is marked as nodiscard, I would expect it to be diagnosed with the standard attribute spelling. https://reviews.llvm.org/D32207 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits