[libcxx-commits] [PATCH] D156783: [libc++] Fix `std::out_of_range` thrown from `basic_stringbuf::str() &&`

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/include/sstream:405
+        if (!__view.empty())
+            __result.assign(std::move(__str_), __view.data() - __str_.data(), __view.size());
         __str_.clear();
----------------
Mordante wrote:
> pfusik wrote:
> > Mordante wrote:
> > > Depending on the order of evaluation `__view.data() - __str_.data()` may be used after `__str_` has been moved. Lets determine the size before `assign`. This is pre-existing.
> > > 
> > > Whether this is save depends on the order in which the function arguments are evaluated. Clang and GCC use a different order.
> > I was also having this concern for a moment. :) But `std::move` is just a cast that does not modify its argument. The actual move is inside `assign`.
> Good point, you're right. It only does a cast and nothing else. 
While that is true, I think moving the calculation out of the function call doesn't hurt. This would be a real bug if `assign` took the string by value, which makes this really subtile and definitely worthy of a comment if we were to keep it this way.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156783/new/

https://reviews.llvm.org/D156783