[libcxx-commits] [PATCH] D125019: [libc++] Avoid creating temporaries in unary expressions involving valarray

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a subscriber: libc++ vendors.
ldionne marked 4 inline comments as done.
ldionne added a comment.

In D125019#3494305 <https://reviews.llvm.org/D125019#3494305>, @philnik wrote:

> Isn't this ABI breaking? Or is it in this case OK for some reason?

It is technically an ABI break. However, the only way I can think of being broken by this is to depend on the exact type of an expression involving `valarray` (such as `decltype(!a && b)`). Furthermore, if that were the case, it would almost surely show up as a link-time error, not a runtime error (it would be a runtime error if a function's return type suddenly changed as a result of this change). Given that the current behavior is extremely broken and that `valarray` doesn't see all that much use in the wild, I believe that while the risk of breakage is non-zero, the risk-to-reward ratio makes it a no brainer to make the change. I'll add a release note, though.

Note that in particular, I am erring towards not introducing an ABI macro for this because it fixes a bug and anyone having used these methods non-trivially is probably broken anyways. Please let me know if you think that's reasonable. Also, pinging libc++ vendors in case they have something to add.



================
Comment at: libcxx/include/valarray:926-929
+    __val_expr<_UnaryOp<__unary_plus<_Tp>, const valarray&> >   operator+() const;
+    __val_expr<_UnaryOp<negate<_Tp>, const valarray&> >         operator-() const;
+    __val_expr<_UnaryOp<__bit_not<_Tp>, const valarray&> >      operator~() const;
+    __val_expr<_UnaryOp<logical_not<_Tp>, const valarray&> >    operator!() const;
----------------
philnik wrote:
> Add `_LIBCPP_HIDE_FROM_ABI`?
I'll use `_LIBCPP_INLINE_VISIBILITY` for consistency with the rest of the file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125019