<div dir="ltr">Excuse me, I have reverted it in r317077.<div>It confused clang with g++-4.8. See;</div><div><br></div><div><div> <a href="http://bb9.pgr.jp/#/builders/2/builds/335">http://bb9.pgr.jp/#/builders/2/builds/335</a></div><div> <a href="http://lab.llvm.org:8011/builders/clang-with-thin-lto-ubuntu/builds/6326">http://lab.llvm.org:8011/builders/clang-with-thin-lto-ubuntu/builds/6326</a></div><div> <a href="http://lab.llvm.org:8011/builders/clang-atom-d525-fedora-rel/builds/10853">http://lab.llvm.org:8011/builders/clang-atom-d525-fedora-rel/builds/10853</a></div></div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr">On Wed, Nov 1, 2017 at 3:36 AM Benjamin Kramer 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: d0k<br>
Date: Tue Oct 31 11:35:54 2017<br>
New Revision: 317019<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=317019&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=317019&view=rev</a><br>
Log:<br>
[ADT] Split optional to only include copy mechanics and dtor for non-trivial types.<br>
<br>
This makes uses of Optional more transparent to the compiler (and<br>
clang-tidy) and generates slightly smaller code.<br>
<br>
Modified:<br>
llvm/trunk/include/llvm/ADT/Optional.h<br>
llvm/trunk/unittests/ADT/OptionalTest.cpp<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=317019&r1=317018&r2=317019&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/Optional.h?rev=317019&r1=317018&r2=317019&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/ADT/Optional.h (original)<br>
+++ llvm/trunk/include/llvm/ADT/Optional.h Tue Oct 31 11:35:54 2017<br>
@@ -27,114 +27,164 @@<br>
<br>
namespace llvm {<br>
<br>
-template<typename T><br>
-class Optional {<br>
+namespace optional_detail {<br>
+/// Storage for any type.<br>
+template <typename T, bool IsPodLike> struct OptionalStorage {<br>
AlignedCharArrayUnion<T> storage;<br>
bool hasVal = false;<br>
<br>
-public:<br>
- using value_type = T;<br>
-<br>
- Optional(NoneType) {}<br>
- explicit Optional() {}<br>
-<br>
- Optional(const T &y) : hasVal(true) {<br>
- new (storage.buffer) T(y);<br>
- }<br>
+ OptionalStorage() = default;<br>
<br>
- Optional(const Optional &O) : hasVal(O.hasVal) {<br>
+ OptionalStorage(const T &y) : hasVal(true) { new (storage.buffer) T(y); }<br>
+ OptionalStorage(const OptionalStorage &O) : hasVal(O.hasVal) {<br>
if (hasVal)<br>
- new (storage.buffer) T(*O);<br>
+ new (storage.buffer) T(*O.getPointer());<br>
}<br>
-<br>
- Optional(T &&y) : hasVal(true) {<br>
+ OptionalStorage(T &&y) : hasVal(true) {<br>
new (storage.buffer) T(std::forward<T>(y));<br>
}<br>
-<br>
- Optional(Optional<T> &&O) : hasVal(O) {<br>
- if (O) {<br>
- new (storage.buffer) T(std::move(*O));<br>
+ OptionalStorage(OptionalStorage &&O) : hasVal(O.hasVal) {<br>
+ if (O.hasVal) {<br>
+ new (storage.buffer) T(std::move(*O.getPointer()));<br>
O.reset();<br>
}<br>
}<br>
<br>
- ~Optional() {<br>
- reset();<br>
- }<br>
-<br>
- Optional &operator=(T &&y) {<br>
+ OptionalStorage &operator=(T &&y) {<br>
if (hasVal)<br>
- **this = std::move(y);<br>
+ *getPointer() = std::move(y);<br>
else {<br>
new (storage.buffer) T(std::move(y));<br>
hasVal = true;<br>
}<br>
return *this;<br>
}<br>
-<br>
- Optional &operator=(Optional &&O) {<br>
- if (!O)<br>
+ OptionalStorage &operator=(OptionalStorage &&O) {<br>
+ if (!O.hasVal)<br>
reset();<br>
else {<br>
- *this = std::move(*O);<br>
+ *this = std::move(*O.getPointer());<br>
O.reset();<br>
}<br>
return *this;<br>
}<br>
<br>
- /// Create a new object by constructing it in place with the given arguments.<br>
- template<typename ...ArgTypes><br>
- void emplace(ArgTypes &&...Args) {<br>
- reset();<br>
- hasVal = true;<br>
- new (storage.buffer) T(std::forward<ArgTypes>(Args)...);<br>
- }<br>
-<br>
- static inline Optional create(const T* y) {<br>
- return y ? Optional(*y) : Optional();<br>
- }<br>
-<br>
// FIXME: these assignments (& the equivalent const T&/const Optional& ctors)<br>
// could be made more efficient by passing by value, possibly unifying them<br>
// with the rvalue versions above - but this could place a different set of<br>
// requirements (notably: the existence of a default ctor) when implemented<br>
// in that way. Careful SFINAE to avoid such pitfalls would be required.<br>
- Optional &operator=(const T &y) {<br>
+ OptionalStorage &operator=(const T &y) {<br>
if (hasVal)<br>
- **this = y;<br>
+ *getPointer() = y;<br>
else {<br>
new (storage.buffer) T(y);<br>
hasVal = true;<br>
}<br>
return *this;<br>
}<br>
-<br>
- Optional &operator=(const Optional &O) {<br>
- if (!O)<br>
+ OptionalStorage &operator=(const OptionalStorage &O) {<br>
+ if (!O.hasVal)<br>
reset();<br>
else<br>
- *this = *O;<br>
+ *this = *O.getPointer();<br>
return *this;<br>
}<br>
<br>
+ ~OptionalStorage() { reset(); }<br>
+<br>
void reset() {<br>
if (hasVal) {<br>
- (**this).~T();<br>
+ (*getPointer()).~T();<br>
hasVal = false;<br>
}<br>
}<br>
<br>
- const T* getPointer() const { assert(hasVal); return reinterpret_cast<const T*>(storage.buffer); }<br>
- T* getPointer() { assert(hasVal); return reinterpret_cast<T*>(storage.buffer); }<br>
- const T& getValue() const LLVM_LVALUE_FUNCTION { assert(hasVal); return *getPointer(); }<br>
- T& getValue() LLVM_LVALUE_FUNCTION { assert(hasVal); return *getPointer(); }<br>
+ T *getPointer() {<br>
+ assert(hasVal);<br>
+ return reinterpret_cast<T *>(storage.buffer);<br>
+ }<br>
+ const T *getPointer() const {<br>
+ assert(hasVal);<br>
+ return reinterpret_cast<const T *>(storage.buffer);<br>
+ }<br>
+};<br>
+<br>
+/// Storage for trivially copyable types only.<br>
+template <typename T> struct OptionalStorage<T, true> {<br>
+ AlignedCharArrayUnion<T> storage;<br>
+ bool hasVal = false;<br>
+<br>
+ OptionalStorage() = default;<br>
+<br>
+ OptionalStorage(const T &y) : hasVal(true) { new (storage.buffer) T(y); }<br>
+ OptionalStorage &operator=(const T &y) {<br>
+ new (storage.buffer) T(y);<br>
+ hasVal = true;<br>
+ return *this;<br>
+ }<br>
+<br>
+ void reset() { hasVal = false; }<br>
+};<br>
+} // namespace optional_detail<br>
+<br>
+template <typename T> class Optional {<br>
+ optional_detail::OptionalStorage<T, isPodLike<T>::value> Storage;<br>
+<br>
+public:<br>
+ using value_type = T;<br>
+<br>
+ constexpr Optional() {}<br>
+ constexpr Optional(NoneType) {}<br>
+<br>
+ Optional(const T &y) : Storage(y) {}<br>
+ Optional(const Optional &O) = default;<br>
+<br>
+ Optional(T &&y) : Storage(std::forward<T>(y)) {}<br>
+ Optional(Optional &&O) = default;<br>
+<br>
+ Optional &operator=(T &&y) {<br>
+ Storage = std::move(y);<br>
+ return *this;<br>
+ }<br>
+ Optional &operator=(Optional &&O) = default;<br>
+<br>
+ /// Create a new object by constructing it in place with the given arguments.<br>
+ template <typename... ArgTypes> void emplace(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>
+ return y ? Optional(*y) : Optional();<br>
+ }<br>
+<br>
+ Optional &operator=(const T &y) {<br>
+ Storage = y;<br>
+ return *this;<br>
+ }<br>
+ Optional &operator=(const Optional &O) = default;<br>
+<br>
+ void reset() { Storage.reset(); }<br>
+<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 hasVal; }<br>
- bool hasValue() const { return hasVal; }<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 { assert(hasVal); return *getPointer(); }<br>
- T& operator*() LLVM_LVALUE_FUNCTION { assert(hasVal); return *getPointer(); }<br>
+ const T &operator*() const LLVM_LVALUE_FUNCTION { return *getPointer(); }<br>
+ T &operator*() LLVM_LVALUE_FUNCTION { return *getPointer(); }<br>
<br>
template <typename U><br>
constexpr T getValueOr(U &&value) const LLVM_LVALUE_FUNCTION {<br>
@@ -142,8 +192,8 @@ public:<br>
}<br>
<br>
#if LLVM_HAS_RVALUE_REFERENCE_THIS<br>
- T&& getValue() && { assert(hasVal); return std::move(*getPointer()); }<br>
- T&& operator*() && { assert(hasVal); return std::move(*getPointer()); }<br>
+ T &&getValue() && { return std::move(*getPointer()); }<br>
+ T &&operator*() && { return std::move(*getPointer()); }<br>
<br>
template <typename U><br>
T getValueOr(U &&value) && {<br>
<br>
Modified: llvm/trunk/unittests/ADT/OptionalTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/OptionalTest.cpp?rev=317019&r1=317018&r2=317019&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/OptionalTest.cpp?rev=317019&r1=317018&r2=317019&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/unittests/ADT/OptionalTest.cpp (original)<br>
+++ llvm/trunk/unittests/ADT/OptionalTest.cpp Tue Oct 31 11:35:54 2017<br>
@@ -518,5 +518,14 @@ TEST_F(OptionalTest, OperatorGreaterEqua<br>
CheckRelation<GreaterEqual>(InequalityLhs, InequalityRhs, !IsLess);<br>
}<br>
<br>
+#if (__has_feature(is_trivially_copyable) && defined(_LIBCPP_VERSION)) || \<br>
+ (defined(__GNUC__) && __GNUC__ >= 5)<br>
+static_assert(std::is_trivially_copyable<Optional<int>>::value,<br>
+ "Should be trivially copyable");<br>
+static_assert(<br>
+ !std::is_trivially_copyable<Optional<NonDefaultConstructible>>::value,<br>
+ "Shouldn't be trivially copyable");<br>
+#endif<br>
+<br>
} // end anonymous namespace<br>
<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="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>