[go: nahoru, domu]

Bug 115846 - std::optional<std::unique_ptr<int>> is constant expression even in C++20 mode
Summary: std::optional<std::unique_ptr<int>> is constant expression even in C++20 mode
Status: RESOLVED INVALID
Alias: None
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: 15.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: accepts-invalid
Depends on:
Blocks:
 
Reported: 2024-07-09 14:25 UTC by 康桓瑋
Modified: 2024-07-10 08:55 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description 康桓瑋 2024-07-09 14:25:38 UTC
#include <memory>
#include <optional>

// static_assert([] {
//   std::unique_ptr<int> p; // non-constant condition for static assertion
//   return true;
// }());

static_assert([] {
  std::optional<std::unique_ptr<int>> opt;
  return true;
}());

GCC rejects the first but accepts the last, which seems wrong since unique_ptr's destructor is not constexpr in C++20.

https://godbolt.org/z/38bToE8oE
Comment 1 Andrew Pinski 2024-07-09 17:33:54 UTC
I think it is due to:
include/std/optional:     _GLIBCXX20_CONSTEXPR ~_Storage() { }

Which was done in r12-4389-g476f305b6cf11d (for https://wg21.link/p2231 ).
Comment 2 Andrew Pinski 2024-07-09 17:41:15 UTC
My reading of the paper makes me think this is a libc++ bug because even though std::unique_ptr<int> is not constexpr std::optional<std::unique_ptr<int>> is still if there was optional (value) is not set/use.

Especially since std::optional<std::unique_ptr<int>>::reset is constexpr there.

Now I could be reading the paper incorrectly and even std::optional part in general incorrectly.
Comment 3 Andrew Pinski 2024-07-09 17:46:47 UTC
Note here is a more reduced and easier to understand testcase (use a custom class ratehr than unique_ptr):
```
#include <optional>

struct f
{
  ~f(){}
};

// static_assert([] {
//   f p; // non-constant condition for static assertion
//   return true;
// }());

static_assert([] {
  std::optional<f> opt;
  opt.reset();
  return true;
}());
```
Comment 4 Andrew Pinski 2024-07-09 17:47:47 UTC
(In reply to Andrew Pinski from comment #2)
> My reading of the paper makes me think this is a libc++ bug

And Microsoft's STD libc++ too.
Comment 5 Andrew Pinski 2024-07-09 17:54:30 UTC
Note I think the `intent` of std::optional is have it similar to something like myoptional (not exactly but you should get the idea) below:
```
struct f
{
  ~f(){}
};

struct myoptional
{
  f *t = nullptr;
  constexpr ~myoptional() {reset(); }
  constexpr void reset() {  delete t; t = nullptr; }
  constexpr myoptional &operator =(const f&&a) { reset(); t = new f(a); return *this; }
};

static_assert([] {
  myoptional opt;
  opt.reset();
  //opt = f{};
  return true;
}());
```

If so then libstdc++ is still correct.
Comment 6 Jonathan Wakely 2024-07-09 21:29:31 UTC
(In reply to Andrew Pinski from comment #1)
> I think it is due to:
> include/std/optional:     _GLIBCXX20_CONSTEXPR ~_Storage() { }
> 
> Which was done in r12-4389-g476f305b6cf11d (for https://wg21.link/p2231 ).

Yes, P2231 was approved as a DR against C++20, so std::optional has a constexpr destructor.

The example works because no std::unique_ptr object needs to be destroyed, but it's not related to unique_ptr at all. It works for any type:

Reduced:

#include <optional>
struct S { ~S() { } };
static_assert( not std::optional<S>{} );
Comment 7 康桓瑋 2024-07-10 00:30:54 UTC
(In reply to Jonathan Wakely from comment #6)
> (In reply to Andrew Pinski from comment #1)
> > I think it is due to:
> > include/std/optional:     _GLIBCXX20_CONSTEXPR ~_Storage() { }
> > 
> > Which was done in r12-4389-g476f305b6cf11d (for https://wg21.link/p2231 ).
> 
> Yes, P2231 was approved as a DR against C++20, so std::optional has a
> constexpr destructor.
> 
> The example works because no std::unique_ptr object needs to be destroyed,
> but it's not related to unique_ptr at all. It works for any type:
> 
> Reduced:
> 
> #include <optional>
> struct S { ~S() { } };
> static_assert( not std::optional<S>{} );

It seems like ~_Storage() doesn't call _M_value.~_Up(). 
I could be wrong.
Comment 8 康桓瑋 2024-07-10 00:32:56 UTC
(In reply to 康桓瑋 from comment #7)
> (In reply to Jonathan Wakely from comment #6)
> > (In reply to Andrew Pinski from comment #1)
> > > I think it is due to:
> > > include/std/optional:     _GLIBCXX20_CONSTEXPR ~_Storage() { }
> > > 
> > > Which was done in r12-4389-g476f305b6cf11d (for https://wg21.link/p2231 ).
> > 
> > Yes, P2231 was approved as a DR against C++20, so std::optional has a
> > constexpr destructor.
> > 
> > The example works because no std::unique_ptr object needs to be destroyed,
> > but it's not related to unique_ptr at all. It works for any type:
> > 
> > Reduced:
> > 
> > #include <optional>
> > struct S { ~S() { } };
> > static_assert( not std::optional<S>{} );
> 
> It seems like ~_Storage() doesn't call _M_value.~_Up(). 
> I could be wrong.

[optional.dtor] specifies to call val->T::~T().
Comment 9 Jonathan Wakely 2024-07-10 08:51:00 UTC
(In reply to 康桓瑋 from comment #7)
> It seems like ~_Storage() doesn't call _M_value.~_Up(). 
> I could be wrong.

Because the _Storage union doesn't know if there is an active member or not, so it doesn't know if anything needs to be destroyed.

It's done in _Optional_payload_base::_M_destroy().

(In reply to 康桓瑋 from comment #8)
> [optional.dtor] specifies to call val->T::~T().

But only if it has a contained value. You can't destroy something that doesn't exist.
Comment 10 Jonathan Wakely 2024-07-10 08:54:17 UTC
If you construct an optional<S> that contains a value, then of course it's destroyed, and then the destructor needs to be usable in constant expressions:

#include <optional>
struct S { ~S() { } };
S s;
static_assert( not std::optional<S>{s} );


/home/jwakely/gcc/15/include/c++/15.0.0/optional:284:42: error: call to non-'constexpr' function 'S::~S()'
Comment 11 Jonathan Wakely 2024-07-10 08:55:57 UTC
I don't see a problem here, and I don't see why we should change anything when the code works fine.

During constant evaluation, if nothing gets destroyed then the destructor is not needed. So it's irrelevant whether it's constexpr or not.