https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112480

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2023-11-10
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I feel like the compiler should be able to do that anyway.

_M_destroy() is:

        _M_engaged = false;
        _M_payload._M_value.~_Stored_type();

For a trivially destructible type the second statement is a no-op, so we have:

  if (_M_engaged)
    _M_engaged = false;

The compiler should turn that into _M_engaged = false.

The suggested change is wrong though, because it runs the trivial destructor
unconditionally, even if there is no object within its lifetime. Clang will
diagnose that during constant evaluation (although gcc doesn't, which I think
is a known bug).

For example:

#include <optional>

constexpr bool f()
{
  std::optional<int> opt(1);
  opt = std::nullopt;
  opt = std::nullopt;
  return true;
}

static_assert(f());


opt.cc:10:15: error: static assertion expression is not an integral constant
expression
static_assert(f());
              ^~~
/home/jwakely/gcc/latest/lib/gcc/x86_64-pc-linux-gnu/14.0.0/../../../../include/c++/14.0.0/optional:282:2:
note: destruction of member '_M_value' of union with active member '_M_empty'
is not allowed in a constant expression
        _M_payload._M_value.~_Stored_type();
        ^
/home/jwakely/gcc/latest/lib/gcc/x86_64-pc-linux-gnu/14.0.0/../../../../include/c++/14.0.0/optional:313:4:
note: in call to '&opt._Optional_base::_M_payload->_M_destroy()'
          _M_destroy();
          ^
/home/jwakely/gcc/latest/lib/gcc/x86_64-pc-linux-gnu/14.0.0/../../../../include/c++/14.0.0/optional:465:45:
note: in call to '&opt._Optional_base::_M_payload->_M_reset()'
      { static_cast<_Dp*>(this)->_M_payload._M_reset(); }
                                            ^
/home/jwakely/gcc/latest/lib/gcc/x86_64-pc-linux-gnu/14.0.0/../../../../include/c++/14.0.0/optional:831:8:
note: in call to '&opt->_M_reset()'
        this->_M_reset();
              ^
opt.cc:6:7: note: in call to '&opt->operator=({})'
  opt = std::nullopt;
      ^
opt.cc:10:15: note: in call to 'f()'
static_assert(f());
              ^
1 error generated.


We could do:

      constexpr void
      _M_reset() noexcept
      {
        if constexpr (is_trivially_destructible_v<_Tp>)
          {
            if (!std::__is_constant_evaluated())
              {
                this->_M_engaged = false;
                return;
              }
          }

        if (this->_M_engaged)
          _M_destroy();
      }


Yuck

Reply via email to