Re: [webkit-dev] Moving from WTF::Optional to std::optional

2021-06-02 Thread Darin Adler via webkit-dev
Thanks for pointing that out, Chris.

This advice goes beyond std::optional, by the way. For anything that we move 
from, there are two operations at are intended to be safe, from a C++ language 
and library design point of view: destroying the object and overwriting it by 
assigning a new value. If our code relies on doing anything else after the 
object is moved from, like examining the value after the old value is moved 
out, please use std::exchange to set the new value while moving the old value 
out. This even applies to large-seeming objects like HashMap, which I will note 
is not large: in release builds a HashMap is implemented as a single pointer to 
a structure on the heap and a new empty HashMap is a null pointer.

— Darin

Sent from my iPad

> On Jun 1, 2021, at 10:01 PM, Chris Dumez  wrote:
> 
> Hi,
> 
> Another thing Darin didn’t mention but I think people should be careful about:
> The move constructor for std::optional does not clear the is-set flag (while 
> the one for WTF::Optional did).
> 
> As a result, you will be having a very bad time if you do a use-after-move of 
> a std::optional. Please make sure to use std::exchange() instead of WTFMove() 
> if you want to leave to std::optional in a clean state for reuse later.
> 
> Chris Dumez
> 
>>> On Jun 1, 2021, at 8:54 PM, Darin Adler via webkit-dev 
>>>  wrote:
>>> 
>> Hi folks.
>> 
>> We’re getting rid of the WTF::Optional class template, because, hooray, 
>> std::optional is supported quite well by all the C++17 compilers we use, and 
>> we don’t have to keep using our own special version. Generally we don’t want 
>> to reimplement the C++ standard library when there is not a significant 
>> benefit, and this is one of those times.
>> 
>> Here are a few considerations:
>> 
>> 1) Since https://commits.webkit.org/238290@main, if you use Optional<> by 
>> mistake instead of std::optional<>, your code won’t compile. (Unless you are 
>> writing code for ANGLE, which has its own separate Optional<>.)
>> 
>> 2) If you want to use std::optional, include the C++ standard header, 
>> , or something that includes it. In a lot of cases, adding an 
>> include will not be required since it’s included by widely-used headers like 
>> WTFString.h and Vector.h, so if you include one of those are covered. 
>> Another way to think about this is that if your base class already uses 
>> std::optional, then you don’t need to include it.
>> 
>> 3) Once the patch in https://bugs.webkit.org/show_bug.cgi?id=226437 lands, 
>> includes of  won’t forward declare optional, and includes of 
>>  won’t do anything at all.
>> 
>> — Darin
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org
>> https://lists.webkit.org/mailman/listinfo/webkit-dev
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Moving from WTF::Optional to std::optional

2021-06-01 Thread Chris Dumez via webkit-dev
Hi,

Another thing Darin didn’t mention but I think people should be careful about:
The move constructor for std::optional does not clear the is-set flag (while 
the one for WTF::Optional did).

As a result, you will be having a very bad time if you do a use-after-move of a 
std::optional. Please make sure to use std::exchange() instead of WTFMove() if 
you want to leave to std::optional in a clean state for reuse later.

Chris Dumez

> On Jun 1, 2021, at 8:54 PM, Darin Adler via webkit-dev 
>  wrote:
> 
> Hi folks.
> 
> We’re getting rid of the WTF::Optional class template, because, hooray, 
> std::optional is supported quite well by all the C++17 compilers we use, and 
> we don’t have to keep using our own special version. Generally we don’t want 
> to reimplement the C++ standard library when there is not a significant 
> benefit, and this is one of those times.
> 
> Here are a few considerations:
> 
> 1) Since https://commits.webkit.org/238290@main, if you use Optional<> by 
> mistake instead of std::optional<>, your code won’t compile. (Unless you are 
> writing code for ANGLE, which has its own separate Optional<>.)
> 
> 2) If you want to use std::optional, include the C++ standard header, 
> , or something that includes it. In a lot of cases, adding an 
> include will not be required since it’s included by widely-used headers like 
> WTFString.h and Vector.h, so if you include one of those are covered. Another 
> way to think about this is that if your base class already uses 
> std::optional, then you don’t need to include it.
> 
> 3) Once the patch in https://bugs.webkit.org/show_bug.cgi?id=226437 lands, 
> includes of  won’t forward declare optional, and includes of 
>  won’t do anything at all.
> 
> — Darin
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev