[PATCH] D50710: [ADT] Fix Optional ABI mismatch between clang & gcc

Richard Smith - zygoloid via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 25 11:50:20 PDT 2018


rsmith added inline comments.
Herald added a subscriber: kristina.


================
Comment at: include/llvm/ADT/Optional.h:113
 template <typename T> struct OptionalStorage<T, true> {
   AlignedCharArrayUnion<T> storage;
   bool hasVal = false;
----------------
When the type is *actually* trivially-copyable (and not just `isPodLike`, which lies), we could just store it directly: `union { T storage; };`


================
Comment at: include/llvm/ADT/Optional.h:122
+      // Use std::addressof to silence strict-aliasing warnings from gcc.
+      *reinterpret_cast<T *>(std::addressof(storage.buffer)) = y;
+    } else {
----------------
This would be more correct as:

```
#ifdef __cpp_lib_launder
*std::launder(reinterpret_cast<T *>(&storage.buffer)) = y;
#else
*reinterpret_cast<T *>(std::addressof(storage.buffer)) = y;
#endif
```

(I'd expect the `std::launder` to also shut up the TBAA warnings.)


Repository:
  rL LLVM

https://reviews.llvm.org/D50710





More information about the llvm-commits mailing list