libc++ should (behind a macro that can enable and disable) add [[nodiscard]] unique_ptr::release

[chandlerc]

Bugzilla Link	30898
Version	unspecified
OS	All
CC	@mclow

Extended Description

We have a C++ annotation ([[nodiscard]]) which allows warnings when returned values are ignored. This should be possible to enable on std::unique_ptr<...>::release()'s returned pointer so that we don't accidentally leak memory.

[llvmbot]

Patch up for review as D26596 (https://reviews.llvm.org/D26596).

[mclow]

The macro you want is available, it's called _LIBCPP_NODISCARD_AFTER_CXX17.

We put it on a few things (as specified in P0600), and there's a paper coming for JAX which will list many, many more places to add this. I'll make sure that shared_ptr::release and unique_ptr::release are on this list.

[llvmbot]

I'm not yet convinced we should limit these changes to > C++17.

[mclow]

I'm not yet convinced we should limit these changes to > C++17.

The last time I proposed doing this, I got a lot of push back.

[ldionne]

As explained in #110847, it turns out that adding [[nodiscard]] on unique_ptr::release() seems to trigger a lot of false positives. I'm not convinced this is a win.

Closing.

[philnik777]

Reopening, since I'd like us to investigate applying [[nodiscard]] only when using the default deleter. I don't know whether that's fruitful, but I think it's certainly worth a shot, since memory leaks are at the very least annoying to track down. If we could get at least the default deleter case to warn with a high signal to noise ratio that would already be a huge win I think.