aaron.ballman added inline comments.
================
Comment at: test/clang-tidy/bugprone-unused-return-value.cpp:210
+ // test that the check is disabled inside GNU statement expressions
+ ({ std::async(increment, 42); });
+ auto StmtExprRetval = ({ std::async(increment, 42); });
----------------
khuttun wrote:
> aaron.ballman wrote:
> > Not diagnosing this case is questionable -- the return value *is* unused
> > and that's a bad thing. However, this is likely to be an uncommon code
> > pattern, so I'd be fine if you added a FIXME to the AST matcher code that
> > excludes GNU expression statements to handle this case someday, and add a
> > FIXME comment here as well so we know what we would like the behavior to be
> > (you could fix the FIXMEs in a follow-up patch if you'd like).
> I'm not so sure whether this code should cause a warning. I see it as
> equivalent to this
>
>
> ```
> []{ return std::async(increment, 42); }();
> ```
>
> , where the return value of `std::async` is used in the return statement.
>
> One situation related to the GNU statement expressions that the checker
> currently misses is if the return value is unused inside the statement
> expression at some other than the last statement. I can see if this could be
> somehow covered.
It all depends on how far down the rabbit hole you want the check to go, I
guess.
```
auto fn = []{ return std::async(increment, 42); };
fn(); // Could also be reasonable to diagnose
```
I'm fine leaving it alone, but I'd like to see test coverage and comments
showing the cases were explicitly considered.
https://reviews.llvm.org/D41655
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
