<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, Feb 17, 2019 at 6:58 AM Serge Guelton via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: serge_sans_paille<br>
Date: Sun Feb 17 06:59:21 2019<br>
New Revision: 354217<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=354217&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=354217&view=rev</a><br>
Log:<br>
Revert  [NFC] Better encapsulation of llvm::Optional Storage<br>
<br>
I'm getting the feealing that current Optional implementation is full of UB :-/<br></blockquote><div><br>Any particular things worth writing down (in comments and/or commit messages, etc) for posterity/other people who might stumble across them?<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Modified:<br>
    llvm/trunk/include/llvm/ADT/Optional.h<br>
<br>
Modified: llvm/trunk/include/llvm/ADT/Optional.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/Optional.h?rev=354217&r1=354216&r2=354217&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/Optional.h?rev=354217&r1=354216&r2=354217&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/ADT/Optional.h (original)<br>
+++ llvm/trunk/include/llvm/ADT/Optional.h Sun Feb 17 06:59:21 2019<br>
@@ -30,12 +30,10 @@ class raw_ostream;<br>
<br>
 namespace optional_detail {<br>
 /// Storage for any type.<br>
-template <typename T, bool = is_trivially_copyable<T>::value><br>
-class OptionalStorage {<br>
+template <typename T, bool = is_trivially_copyable<T>::value> struct OptionalStorage {<br>
   AlignedCharArrayUnion<T> storage;<br>
   bool hasVal = false;<br>
<br>
-public:<br>
   OptionalStorage() = default;<br>
<br>
   OptionalStorage(const T &y) : hasVal(true) { new (storage.buffer) T(y); }<br>
@@ -109,14 +107,6 @@ public:<br>
     assert(hasVal);<br>
     return reinterpret_cast<const T *>(storage.buffer);<br>
   }<br>
-<br>
-  template <typename... ArgTypes> void emplace(ArgTypes &&... Args) {<br>
-    reset();<br>
-    hasVal = true;<br>
-    new (storage.buffer) T(std::forward<ArgTypes>(Args)...);<br>
-  }<br>
-<br>
-  bool hasValue() const { return hasVal; }<br>
 };<br>
<br>
 } // namespace optional_detail<br>
@@ -144,7 +134,9 @@ public:<br>
<br>
   /// Create a new object by constructing it in place with the given arguments.<br>
   template <typename... ArgTypes> void emplace(ArgTypes &&... Args) {<br>
-    Storage.emplace(std::forward<ArgTypes>(Args)...);<br>
+    reset();<br>
+    Storage.hasVal = true;<br>
+    new (getPointer()) T(std::forward<ArgTypes>(Args)...);<br>
   }<br>
<br>
   static inline Optional create(const T *y) {<br>
@@ -159,13 +151,19 @@ public:<br>
<br>
   void reset() { Storage.reset(); }<br>
<br>
-  const T *getPointer() const { return Storage.getPointer(); }<br>
-  T *getPointer() { return Storage.getPointer(); }<br>
+  const T *getPointer() const {<br>
+    assert(Storage.hasVal);<br>
+    return reinterpret_cast<const T *>(Storage.storage.buffer);<br>
+  }<br>
+  T *getPointer() {<br>
+    assert(Storage.hasVal);<br>
+    return reinterpret_cast<T *>(Storage.storage.buffer);<br>
+  }<br>
   const T &getValue() const LLVM_LVALUE_FUNCTION { return *getPointer(); }<br>
   T &getValue() LLVM_LVALUE_FUNCTION { return *getPointer(); }<br>
<br>
-  explicit operator bool() const { return hasValue(); }<br>
-  bool hasValue() const { return Storage.hasValue(); }<br>
+  explicit operator bool() const { return Storage.hasVal; }<br>
+  bool hasValue() const { return Storage.hasVal; }<br>
   const T *operator->() const { return getPointer(); }<br>
   T *operator->() { return getPointer(); }<br>
   const T &operator*() const LLVM_LVALUE_FUNCTION { return *getPointer(); }<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div>