khuttun added a comment.

In https://reviews.llvm.org/D41655#974725, @JonasToth wrote:

> High Integrity C++ has the rule `17.5.1 Do not ignore the result of 
> std::remove, std::remove_if or std::unique`. Do we want to add those to the 
> preconfigured list?


To me these sound like reasonable additions. I can add them and run the checker 
against LLVM source to see if we get any false positives with them.

I also noticed that the checker currently misses unused values inside other 
kinds of statements than compound statement (if statements, case statements 
etc.). I will also update the checker to handle these.



================
Comment at: clang-tidy/bugprone/UnusedReturnValueCheck.cpp:47
+                            "^::std::launder$|"
+                            "^::std::unique_ptr<.*>::release$|"
+                            "^::std::.*::allocate$|"
----------------
JonasToth wrote:
> Is the following type a problem for you check?
> 
> `std::unique_ptr<std::vector<int>>` should not be matchable with regex but I 
> don't know if that would have an impact on the functionality.
`std::unique_ptr<std::vector<int>>` is also matched. I added a test case for it.


================
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); });
----------------
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.


https://reviews.llvm.org/D41655



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to