[PATCH] D41056: [clang-tidy] New check misc-uniqueptr-release-unused-retval

[Kalle Huttunen via Phabricator via cfe-commits]

khuttun added a comment.

In https://reviews.llvm.org/D41056#951145, @aaron.ballman wrote:

> In https://reviews.llvm.org/D41056#951083, @alexfh wrote:
>
> > In https://reviews.llvm.org/D41056#950605, @khuttun wrote:
> >
> > > In https://reviews.llvm.org/D41056#950570, @Eugene.Zelenko wrote:
> > >
> > > > May be //bugprone// is better module then //misc//?
> > >
> > >
> > > Maybe. I can move it if all the reviewers think that it would be better suited there.
> >
> >
> > Yup, bugprone- should be a better category for this, IMO.
> >
> > I wonder whether libc++ folks are interested in marking unique_ptr::release() with `__attribute__ ((warn_unused_result))`. A compiler warning (with -Werror) would be a better protection against this kind of a bug.
>
>
> There's a push in WG21 to mark more of the library with `[[nodiscard]]`: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0600r1.pdf
>
> If we have a check for this, I do not think it should be specific to `unique_ptr::release()`, but instead be more broadly applicable to APIs that should be marked `[[nodiscard]]` but are not (currently). P0600R1 is a good place to start, but I'm guessing there are POSIX APIs (among others) that would also qualify.


P0600R1 seems to suggest quite conservative approach (and rightly so, IMO) in taking `[[nodiscard]]` in use in std lib. For example it proposes *not* to use it with `unique_ptr::release`. I think there could still be room for static unused ret val checking for parts of std lib not covered by P0600R1, but also for boost, POSIX APIs etc.

What do you think about making this check fully configurable through the check options? The options could list the functions to check. The documentation could suggest some candidate functions from std lib to configure checks for.


Repository:
  rL LLVM

https://reviews.llvm.org/D41056