[RFC] libstdc++: optional: Set _M_engaged after the call to destructor [PR127005]

[Andrea Pinski]

On Sun, Aug 23, 2026 at 4:41 PM Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
>
>
>
> On Sun, 23 Aug 2026, 20:26 Andrea Pinski, <andrew.pinski@oss.qualcomm.com> wrote:
>>
>> Sometimes the destructor will cause the optimizers not to see that
>> _M_engaged has been to set to false (e.g. an external call). This
>> means that we might get an uninitialized warning after the destructor
>> in some cases.
>> Setting it after the destructor will cause one extra store in some cases
>> but that store allows the optimizers to get rid of other code.
>>
>> This is a RFC because I don't know if we want to remove the first store
>> of _M_engaged or keep it.
>> The patch has been bootstrapped and tested on x86_64-linux-gnu with no regressions.
>> And tested provided in the bug report works and also the generated code looks much
>> better too.
>>
>>         PR libstdc++/127005
>>
>> libstdc++-v3/ChangeLog:
>>
>>         * include/std/optional (optional::_M_destroy): Set _M_engaged
>>         to false after the destructor call.
>>
>> Signed-off-by: Andrea Pinski <andrew.pinski@oss.qualcomm.com>
>> ---
>>  libstdc++-v3/include/std/optional | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/libstdc++-v3/include/std/optional b/libstdc++-v3/include/std/optional
>> index d6fd33e8c2e..8884c54f3e7 100644
>> --- a/libstdc++-v3/include/std/optional
>> +++ b/libstdc++-v3/include/std/optional
>> @@ -303,6 +303,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>        {
>>         _M_engaged = false;
>>         _M_payload._M_value.~_Stored_type();
>> +       _M_engaged = false;
>
>
> Do you see the same benefits from
>
> if (_M_engaged)
>   __builtin_unreachable();
>
> ?

Yes. that works here too. Yes I agree this version is better so I will
go and test this version tomorrow and submit with a full patch
including a testcase.


>
>>  #if defined(__clang__) && __cpp_lib_optional >= 202106L // full constexpr support
>>         if (std::is_constant_evaluated())
>>           // Work around PR124910 for Clang.
>> --
>> 2.43.0
>>