STL icon indicating copy to clipboard operation
STL copied to clipboard

`<memory>`: `!_Get_ptr()` in the destructors of `inout_ptr_t` and `out_ptr_t` _should_ be correct

Open frederick-vs-ja opened this issue 1 year ago • 5 comments

Describe the bug

Currently !_Get_ptr() is used in the destructors of inout_ptr_t and out_ptr_t.

https://github.com/microsoft/STL/blob/a26f4adb1ecf767784bb4e4318093c4579fdb364/stl/inc/memory#L4409-L4411 https://github.com/microsoft/STL/blob/a26f4adb1ecf767784bb4e4318093c4579fdb364/stl/inc/memory#L4328-L4330

However, it seems that nothing in Cpp17NullablePointer ([nullablepointer.requirements]) requires !p to be semantically equivalent to p != nullptr or even to be well-formed. E.g. a type with a deleted member operator! (the NoExclamation type shown below) can still meet the Cpp17NullablePointer requirements.

#include <memory>

struct NoExclamation {
    using pointer = NoExclamation;

    void* p_{};

    NoExclamation() = default;
    constexpr NoExclamation(decltype(nullptr)) noexcept {}

    friend bool operator==(NoExclamation, NoExclamation) = default;
    friend constexpr bool operator==(NoExclamation lhs, decltype(nullptr)) noexcept
    {
        return lhs.p_ == nullptr;
    }

    constexpr explicit operator bool() const noexcept
    {
        return p_ != nullptr;
    }

    void operator!() const = delete; // HERE!
};

int main()
{
    NoExclamation ne{};
    std::out_ptr(ne);
}

Currently no implementation accepts the example due to the deleted operator!.

Command-line test case

Godbolt link

Expected behavior

Perhaps the program should compile.

STL version

a26f4adb1ecf767784bb4e4318093c4579fdb364 (probably every version after #1998)

Additional context

As libc++ and libstdc++ also expect usable operator!, should there be an LWG issue to make current implementation strategies comforming?

frederick-vs-ja avatar Sep 18 '24 10:09 frederick-vs-ja

[nullablepointer.requirements]:

An object p of type P can be contextually converted to bool. The effect shall be as if p != nullptr had been evaluated in place of p.

Pedantically, this doesn't require !p to be meaningful (nor p && p or p || p, nor even implicit conversion to bool). But the intent should be quite clear to a non-pedant.

cpplearner avatar Sep 18 '24 12:09 cpplearner

[nullablepointer.requirements]:

An object p of type P can be contextually converted to bool. The effect shall be as if p != nullptr had been evaluated in place of p.

Pedantically, this doesn't require !p to be meaningful (nor p && p or p || p, nor even implicit conversion to bool). But the intent should be quite clear to a non-pedant.

Sounds like that contextual version of boolean-testability is wanted.

frederick-vs-ja avatar Sep 18 '24 13:09 frederick-vs-ja

We talked about this at the weekly maintainer meeting. The fact that the Majestic Three implementations all want !p to behave reasonably is a strong indication to us that Cpp17NullablePointer should be requiring boolean-testable ([concept.booleantestable]) and that we should file an LWG issue rather than attempting to contort our implementation to handle absurdly pathological types.

StephanTLavavej avatar Sep 18 '24 22:09 StephanTLavavej

Cpp17NullablePointer should be requiring boolean-testable ([concept.booleantestable])

I guess it's intentional that implicit convertibility to bool (which is required in boolean-testable) is not required - all of unique_ptr, shared_ptr, and exception_ptr (~in Majestic Three implementations~ required by [propagation]/5) have explicit conversion functions to bool.

frederick-vs-ja avatar Sep 21 '24 14:09 frederick-vs-ja

This is LWG-4155.

(Good point about not wanting implicit conversions to bool, forgot about that.)

StephanTLavavej avatar Sep 25 '24 21:09 StephanTLavavej