aaron.ballman added inline comments.
================
Comment at: clang-tidy/bugprone/UnusedReturnValueCheck.cpp:69
+ "the value returned by %0 should normally be used")
+ << dyn_cast_or_null<NamedDecl>(Matched->getCalleeDecl())
+ << Matched->getSourceRange();
----------------
khuttun wrote:
> aaron.ballman wrote:
> > In the event this returns null, the diagnostic is going to look rather odd.
> > Because the value of the call expression is unused, this will most often
> > trigger in a context where the method call can be inferred (especially
> > because you're now highlighting the source range). It might make sense to
> > simply replace the %0 with "this call expression" or somesuch in the
> > diagnostic.
> I can remove the function name from the diagnostic. Out of curiosity, in
> which situations could it be null?
Situations where the call doesn't refer back to a declaration at all. For
instance, a lambda or block.
================
Comment at: docs/clang-tidy/checks/bugprone-unused-return-value.rst:22
+ - ``std::unique_ptr::release()``. Not using the return value can lead to
+ resource leaks, if the same pointer isn't stored anywhere else. Often
+ ignoring the ``release()`` return value indicates that the programmer
----------------
resource leaks, if the -> resource leaks if the
Often -> Often,
================
Comment at: docs/clang-tidy/checks/bugprone-unused-return-value.rst:31
+ ``std::filesystem::path::empty()`` and ``std::empty()``. Not using the
+ return value doesn't cause any issues on itself, but often it indicates
+ that the programmer confused the function with ``clear()``.
----------------
I'd reword this sentence to: Not using the return value often indicates that
the programmer confused the function with clear().
================
Comment at: test/clang-tidy/bugprone-unused-return-value.cpp:163
+
+void noWarning() {
+ auto AsyncRetval1 = std::async(increment, 42);
----------------
Sorry, I just realized that we're missing a test case for a common situation --
where the result is used as part of another call expression. Can you add a test
to `noWarning()` to make sure this does not warn:
```
std::vector<int> v;
extern void f(bool);
f(v.empty()); // Should not warn
```
https://reviews.llvm.org/D41655
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
