-Wdangling warning on `std::unique_ptr<int>(nullptr).get()`.

[hokein]

This is an interesting case:

const int* p2 = std::unique_ptr<int>(nullptr).get();  

Clang emits a object backing the pointer will be destroyed at the end of the full-expression warning. For this case, we know that the result p2 is nullptr, this is not a dangling case.

[llvmbot]

@llvm/issue-subscribers-clang-frontend

Author: Haojian Wu (hokein)

This is an interesting case:

const int* p2 = std::unique_ptr<int>(nullptr).get();  

Clang emits a object backing the pointer will be destroyed at the end of the full-expression warning. For this case, we know that the result p2 is nullptr, this is not a dangling case.

[philnik777]

While that's technically true, I don't think it makes a ton of sense to complicate things for this case. If you actually wanted to do this, why not change it to const int* p2 = nullptr directly?

[hokein]

Yeah, I agree that it doesn't make sense for this case.

It was simplified from a real usecase, see below:

#include <map>
#include <memory>

// A public API
// Returns a const reference to the value associated with the given key if it
// exists, otherwise returns a const reference to the provided default value.
template<typename Key, typename Value>
const Value& FindWithDefault(const std::map<Key, Value>& container, const Key& key, const Value& default_value [[clang::lifetimebound]]);

void test() {
  std::map<int, std::unique_ptr<int>> container;
  auto* p = FindWithDefault(container,  1, std::unique_ptr<int>(nullptr)).get(); // compiler warns p is dangling.
}  

[ilya-biryukov]

I also think that the analysis is doing ok here and we should not complicate things for this case.
Sure, it's a false-positive, there are ways to rewrite code to avoid the warning and people should just do that.

The code here is asking for people to keep track of an important implicit contract (the value inside unique_ptr is valid despite the unique_ptr going away). This is quite non-obvious and could even lead to errors if people accidentally replaces nullptr with a proper value that should take ownership. I think it's actually a good thing that the warning forces a rewrite to something that manages the lifetime more explicitly.

[hokein]

The explanation makes sense. Additionally, the FindWithDefault API is risky as it can easily lead to dangling references if used incorrectly. We discourage its use internally and recommend a safer alternative.