3734. Inconsistency in inout_ptr and out_ptr for empty case

Section: 20.3.4.1 [out.ptr.t] Status: C++23 Submitter: Doug Cook Opened: 2022-07-11 Last modified: 2023-11-22

Priority: 2

View all issues with C++23 status.

Discussion:

out_ptr and inout_ptr are inconsistent when a pointer-style function returns nullptr.

• out_ptr leaves the stale value in smart (not the value returned by the pointer-style function).

• inout_ptr (as resolved by LWG 3594) leaves nullptr in smart (the value returned by the pointer-style function).

Assume we have the following pointer-style functions that return nullptr in case of failure:

void ReplaceSomething(/*INOUT*/ int** pp) {
  delete *pp;
  *pp = nullptr;
  return; // Failure!
} 

void GetSomething(/*OUT*/ int** pp) {
  *pp = nullptr;
  return; // Failure!
}

In the scenario that led to the creation of issue LWG 3594:

// Before the call, inout contains a stale value.
auto inout = std::make_unique<int>(1);
ReplaceSomething(std::inout_ptr(inout));
// (1) If ReplaceSomething failed (returned nullptr), what does inout contain?

Assuming LWG 3594 is resolved as suggested, inout will be empty. (The original N4901 text allows inout to be either empty or to hold a pointer to already-deleted memory.) Using the resolution suggested by LWG 3594, it expands to something like the following (simplified to ignore exceptions and opting to perform the release() before the ReplaceSomething() operation):

// Before the call, inout contains a stale value.
auto inout = std::make_unique<int>(1);
int* p = inout.release();
ReplaceSomething(&p);
if (p) {
  inout.reset(p);
}
// (1) If ReplaceSomething failed (returned nullptr), inout contains nullptr.

This behavior seems reasonable.

Now consider the corresponding scenario with out_ptr:

// Before the call, out contains a stale value.
auto out = std::make_unique<int>(2);
GetSomething(std::out_ptr(out));
// (2) If GetSomething failed (returned nullptr), what does out contain? 

Based on N4901, out contains the stale value (from make_unique), not the nullptr value returned by GetSomething(). The N4901 model (simplified to ignore exceptions) expands to the following:

// Before the call, out contains a stale value.
auto out = std::make_unique<int>(2);
int* p{};
GetSomething(&p);
if (p) {
  out.reset(p);
}
// (2) If GetSomething failed (returned nullptr), out contains a pointer to "2".

This behavior seems incorrect to me. It is inconsistent with the behavior of inout_ptr and it is inconsistent with my expectation that out should contain the value returned by GetSomething(), even if that value is nullptr. Intuitively, I expect it to behave as if the out.reset(p) were unconditional.

The reset(p) is conditional as an optimization for cases where reset is non-trivial. For example, shared_ptr's reset(p) requires the allocation of a control block even if p is nullptr. As such, simply making the reset unconditional may be sub-optimal.

I see two primary options for making out_ptr's behavior consistent with inout_ptr:

• Perform an unconditional out.reset() or out = Smart() in the out_ptr_t constructor.

• Add an else clause to the if statement, containing out.reset() or out = Smart().

I note that these solutions do not make use of the additional args..., leaving the out pointer in an empty state. This is analogous to the corresponding state in the similar inout scenario where the inout pointer is left empty as a result of the call to smart.release().

I favor the first resolution, freeing any existing value in the out_ptr_t constructor.

[2022-08-23; Reflector poll]

Set priority to 2 after reflector poll. "A bit like design."

[Issaquah 2023-02-07; LWG]

Move to Immediate for C++23

[2023-02-13 Approved at February 2023 meeting in Issaquah. Status changed: Immediate → WP.]

Proposed resolution:

This wording is relative to N4910.

1. Modify 20.3.4.1 [out.ptr.t] as indicated:

   explicit out_ptr_t(Smart& smart, Args... args);
   

   -6- Effects: Initializes s with smart, a with std::forward<Args>(args)..., and value-initializes p. Then, equivalent to:

   • (6.1) —

     s.reset();

     if the expression s.reset() is well-formed;

   • (6.2) — otherwise,

     s = Smart();
     

     if is_constructible_v<Smart> is true;

   • (6.3) — otherwise, the program is ill-formed.

   -7- [Note 2: The constructor is not noexcept to allow for a variety of non-terminating and safe implementation strategies. For example, an implementation can allocate a shared_ptr's internal node in the constructor and let implementation-defined exceptions escape safely. The destructor can then move the allocated control block in directly and avoid any other exceptions. — end note]