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
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
