> On Dec 16, 2018, at 3:47 PM, Daniel Bates <dba...@webkit.org> wrote: > > > >> On Dec 14, 2018, at 3:52 PM, Chris Dumez <cdu...@apple.com >> <mailto:cdu...@apple.com>> wrote: >> >> So to be clear, it is often not truly about using the value after it is >> moved. It is about expecting that the variable / member has been nulled out >> after moving it. >> If I WTFMove() out a data member m_dataMember, I expect later on `if >> (m_dataMember)` to be false. >> > > I do not mean to derail the discussion about whether or not we should have > our own std::optional that disengages on move. I am all for convenience. I > think expecting a valid “empty” state (*) when moving out a data member is an > anti-pattern and I propose that we should be using std::exchange() instead > (or any code analogous to doing std::exchange()) for these cases, including > the case in your original email and especially with data members as in > <https://trac.webkit.org/changeset/239228 > <https://trac.webkit.org/changeset/239228>>.
Expectations can’t be an anti-pattern, only code can be an anti-pattern. At the point of checking whether a value is “empty” by some definition, you may not know that someone has std::move()d it, and shouldn’t have to. So to deal with the bug-prone nature of check after std::move(), we can do one of these: (1) Ban all use of std::move() unless it’s possible for some sort of static check to know that no one will examine the moved value (2) Make sure we only use types that leave themselves in a state that meets expectations post-move(). (3) Try real hard not to make mistakes and hope for the best Maybe there is an option I am missing? Out of these, I think (2) is likely the best, on the whole. > > Why do I consider it an anti-pattern? See (*) and because the concept of > “moving" is not spec'ed to behave like this. Here’s one reference to the > spec’ed behavior and an example comparable to the one in your first email > (highlighting is my own for emphasis): > > [[ > Unless otherwise specified, all standard library objects that have been moved > from are placed in a valid but unspecified state. That is, only the functions > without preconditions, such as the assignment operator, can be safely used on > the object after it was moved from: Note that this is specified for “standard library objects”. There’s nothing wrong with choosing to give a stronger API contract for our own objects, and then to rely on it. > std::vector <http://en.cppreference.com/w/cpp/container/vector><std::string > <http://en.cppreference.com/w/cpp/string/basic_string>> v; > std::string <http://en.cppreference.com/w/cpp/string/basic_string> str = > "example"; > v.push_back(std::move(str)); // str is now valid but unspecified > str.back(); // undefined behavior if size() == 0: back() has a precondition > !empty() > str.clear(); // OK, clear() has no preconditions > > ]] > <https://en.cppreference.com/w/cpp/utility/move > <https://en.cppreference.com/w/cpp/utility/move>> > > (*) What is the valid “empty” state for a type without a default constructor? > > Dan > >> -- >> Chris Dumez >> >> >> >> >>> On Dec 14, 2018, at 3:45 PM, Chris Dumez <cdu...@apple.com >>> <mailto:cdu...@apple.com>> wrote: >>> >>> >>>> On Dec 14, 2018, at 3:39 PM, Fujii Hironori <fujii.hiron...@gmail.com >>>> <mailto:fujii.hiron...@gmail.com>> wrote: >>>> >>>> >>>> On Sat, Dec 15, 2018 at 6:38 AM Chris Dumez <cdu...@apple.com >>>> <mailto:cdu...@apple.com>> wrote: >>>> >>>> I have now been caught twice by std::optional’s move constructor. >>>> >>>> I don't understand how this could be useful? Do you want to use the value >>>> after it is moved? I'd like to see these your code. Could you show me >>>> these two patches? >>> >>> This is the latest one: https://trac.webkit.org/changeset/239228/webkit >>> <https://trac.webkit.org/changeset/239228/webkit> >>> >>> >>> _______________________________________________ >>> webkit-dev mailing list >>> webkit-dev@lists.webkit.org <mailto:webkit-dev@lists.webkit.org> >>> https://lists.webkit.org/mailman/listinfo/webkit-dev >>> <https://lists.webkit.org/mailman/listinfo/webkit-dev> >> >> _______________________________________________ >> webkit-dev mailing list >> webkit-dev@lists.webkit.org <mailto:webkit-dev@lists.webkit.org> >> https://lists.webkit.org/mailman/listinfo/webkit-dev >> <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 mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev