Re: [webkit-dev] Moving from WTF::Optional to std::optional
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
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
[webkit-dev] Moving from WTF::Optional to std::optional
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
