[ADT] Factor out in_place_t and expose in Optional ctor

[dblaikie]

This usage doesn't seem to quite match the standard - which provides an existing instance of in_place_t for callers to use:

std::optional<std::string> o4(std::in_place, {'a', 'b', 'c'});

(to quote https://en.cppreference.com/w/cpp/utility/optional/optional )

Probably good to match this sort of behavior.

llvm/unittests/ADT/OptionalTest.cpp

227

Could write this as: return std::tie(LHS.x, LHS.y) == std::tie(RHS.x, RHS.y); which I think is a bit tidier/easier to generalize to other comparisons, etc.

[scott.linder]

In D100671#2695923, @dblaikie wrote:

This usage doesn't seem to quite match the standard - which provides an existing instance of in_place_t for callers to use:

std::optional<std::string> o4(std::in_place, {'a', 'b', 'c'});

(to quote https://en.cppreference.com/w/cpp/utility/optional/optional )

Probably good to match this sort of behavior.

My understanding is that the C++17 inline constexpr is what makes that generally "safe" wrt. ODR, but I'm not actually sure that it come up in practice (i.e. I don't suspect we will ever actually cause this thing to have an address).

Maybe I can add the variable with a note about not doing anything with it that could cause it to violate ODR? I.e. don't take its address (and probably other thing I need to refresh my memory on)

llvm/unittests/ADT/OptionalTest.cpp

227

Will do, thank you!

[dexonsmith]

In D100671#2696123, @scott.linder wrote:

In D100671#2695923, @dblaikie wrote:

This usage doesn't seem to quite match the standard - which provides an existing instance of in_place_t for callers to use:

std::optional<std::string> o4(std::in_place, {'a', 'b', 'c'});

(to quote https://en.cppreference.com/w/cpp/utility/optional/optional )

Probably good to match this sort of behavior.

My understanding is that the C++17 inline constexpr is what makes that generally "safe" wrt. ODR, but I'm not actually sure that it come up in practice (i.e. I don't suspect we will ever actually cause this thing to have an address).

Maybe I can add the variable with a note about not doing anything with it that could cause it to violate ODR? I.e. don't take its address (and probably other thing I need to refresh my memory on)

I imagine whatever we do for llvm::None would be fine for llvm::inplace too.

[scott.linder]

Add constexpr convenience variables

[dblaikie]

Looks good, thanks! Probably skip the other two in_place_* until we have a use for them.

llvm/include/llvm/ADT/STLForwardCompat.h

53–73

I'd probably skip these until we have a use for them?

clang/include/clang/Basic/DirectoryEntry.h

 template <class RefTy> class MapEntryOptionalStorage {
   using optional_none_tag = typename RefTy::optional_none_tag;
   RefTy MaybeRef;
 
 public:
   MapEntryOptionalStorage() : MaybeRef(optional_none_tag()) {}
 
   template <class... ArgTypes>
-  explicit MapEntryOptionalStorage(llvm::optional_detail::in_place_t,
-                                   ArgTypes &&...Args)
+  explicit MapEntryOptionalStorage(llvm::in_place_t, ArgTypes &&...Args)
       : MaybeRef(std::forward<ArgTypes>(Args)...) {}
 
   void reset() { MaybeRef = optional_none_tag(); }
 
   bool hasValue() const { return MaybeRef.hasOptionalValue(); }
 
   RefTy &getValue() LLVM_LVALUE_FUNCTION {
     assert(hasValue());

llvm/include/llvm/ADT/Optional.h

 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_ADT_OPTIONAL_H
 #define LLVM_ADT_OPTIONAL_H
 
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/None.h"
+#include "llvm/ADT/STLForwardCompat.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/type_traits.h"
 #include <cassert>
 #include <memory>
 #include <new>
 #include <utility>
 
 namespace llvm {
 
 class raw_ostream;
 
 namespace optional_detail {
 
-struct in_place_t {};
-
 /// Storage for any type.
 //
 // The specialization condition intentionally uses
 // llvm::is_trivially_copy_constructible instead of
 // std::is_trivially_copy_constructible.  GCC versions prior to 7.4 may
 // instantiate the copy constructor of `T` when
 // std::is_trivially_copy_constructible is instantiated.  This causes
 // compilation to fail if we query the trivially copy constructible property of
template <typename T> class Optional {
   optional_detail::OptionalStorage<T> Storage;
 
 public:
   using value_type = T;
 
   constexpr Optional() {}
   constexpr Optional(NoneType) {}
 
-  constexpr Optional(const T &y) : Storage(optional_detail::in_place_t{}, y) {}
+  constexpr Optional(const T &y) : Storage(in_place, y) {}
   constexpr Optional(const Optional &O) = default;
 
-  constexpr Optional(T &&y)
-      : Storage(optional_detail::in_place_t{}, std::move(y)) {}
+  constexpr Optional(T &&y) : Storage(in_place, std::move(y)) {}
   constexpr Optional(Optional &&O) = default;
 
+  template <typename... ArgTypes>
+  constexpr Optional(in_place_t, ArgTypes &&...Args)
+      : Storage(in_place, std::forward<ArgTypes>(Args)...) {}
+
   Optional &operator=(T &&y) {
     Storage = std::move(y);
     return *this;
   }
   Optional &operator=(Optional &&O) = default;
 
   /// Create a new object by constructing it in place with the given arguments.
   template <typename... ArgTypes> void emplace(ArgTypes &&... Args) {

llvm/include/llvm/ADT/STLForwardCompat.h

 template <typename...>
 struct disjunction // NOLINT(readability-identifier-naming)
     : std::false_type {};
 template <typename B1> struct disjunction<B1> : B1 {};
 template <typename B1, typename... Bn>
 struct disjunction<B1, Bn...>
     : std::conditional<bool(B1::value), B1, disjunction<Bn...>>::type {};
 
+struct in_place_t // NOLINT(readability-identifier-naming)
+{
+  explicit in_place_t() = default;
+};
+/// \warning This must not be odr-used, as it cannot be made \c inline in C++14.
+constexpr in_place_t in_place; // NOLINT(readability-identifier-naming)
+
+template <typename T>
+struct in_place_type_t // NOLINT(readability-identifier-naming)
+{
+  explicit in_place_type_t() = default;
+};
+
+template <std::size_t I>
+struct in_place_index_t // NOLINT(readability-identifier-naming)
+{
+  explicit in_place_index_t() = default;
+};
+
 //===----------------------------------------------------------------------===//
 //     Features from C++20
 //===----------------------------------------------------------------------===//
 
 template <typename T>
 struct remove_cvref // NOLINT(readability-identifier-naming)
 {
   using type = std::remove_cv_t<std::remove_reference_t<T>>;
 };
 
 template <typename T>
 using remove_cvref_t // NOLINT(readability-identifier-naming)
     = typename llvm::remove_cvref<T>::type;
 
 } // namespace llvm
 
 #endif // LLVM_ADT_STLFORWARDCOMPAT_H

llvm/unittests/ADT/OptionalTest.cpp

NonDefaultConstructible::ResetCounts();
     EXPECT_EQ(0u, NonDefaultConstructible::Destructions);
     NonDefaultConstructible::ResetCounts();
   }
   EXPECT_EQ(0u, NonDefaultConstructible::CopyConstructions);
   EXPECT_EQ(0u, NonDefaultConstructible::CopyAssignments);
   EXPECT_EQ(0u, NonDefaultConstructible::Destructions);
 }
 
+TEST(OptionalTest, InPlaceConstructionNonDefaultConstructibleTest) {
+  NonDefaultConstructible::ResetCounts();
+  { Optional<NonDefaultConstructible> A{in_place, 1}; }
+  EXPECT_EQ(0u, NonDefaultConstructible::CopyConstructions);
+  EXPECT_EQ(0u, NonDefaultConstructible::CopyAssignments);
+  EXPECT_EQ(1u, NonDefaultConstructible::Destructions);
+}
+
 TEST(OptionalTest, GetValueOr) {
   Optional<int> A;
   EXPECT_EQ(42, A.getValueOr(42));
 
   A = 5;
   EXPECT_EQ(5, A.getValueOr(42));
 }
 
 struct MultiArgConstructor {
   int x, y;
   MultiArgConstructor(int x, int y) : x(x), y(y) {}
   explicit MultiArgConstructor(int x, bool positive)
     : x(x), y(positive ? x : -x) {}
 
   MultiArgConstructor(const MultiArgConstructor &) = delete;
   MultiArgConstructor(MultiArgConstructor &&) = delete;
   MultiArgConstructor &operator=(const MultiArgConstructor &) = delete;
   MultiArgConstructor &operator=(MultiArgConstructor &&) = delete;
 
+  friend bool operator==(const MultiArgConstructor &LHS,
+                         const MultiArgConstructor &RHS) {
+    return LHS.x == RHS.x && LHS.y == RHS.y;
+  }
+
   static unsigned Destructions;
   ~MultiArgConstructor() {
     ++Destructions;
   }
   static void ResetCounts() {
     Destructions = 0;
   }
 };
TEST(OptionalTest, Emplace) {
 
   A.emplace(5, false);
   EXPECT_TRUE(A.hasValue());
   EXPECT_EQ(5, A->x);
   EXPECT_EQ(-5, A->y);
   EXPECT_EQ(1u, MultiArgConstructor::Destructions);
 }
 
+TEST(OptionalTest, InPlaceConstructionMultiArgConstructorTest) {
+  MultiArgConstructor::ResetCounts();
+  {
+    Optional<MultiArgConstructor> A{in_place, 1, 2};
+    EXPECT_TRUE(A.hasValue());
+    EXPECT_EQ(1, A->x);
+    EXPECT_EQ(2, A->y);
+    Optional<MultiArgConstructor> B{in_place, 5, false};
+    EXPECT_TRUE(B.hasValue());
+    EXPECT_EQ(5, B->x);
+    EXPECT_EQ(-5, B->y);
+    EXPECT_EQ(0u, MultiArgConstructor::Destructions);
+  }
+  EXPECT_EQ(2u, MultiArgConstructor::Destructions);
+}
+
+TEST(OptionalTest, InPlaceConstructionAndEmplaceEquivalentTest) {
+  MultiArgConstructor::ResetCounts();
+  {
+    Optional<MultiArgConstructor> A{in_place, 1, 2};
+    Optional<MultiArgConstructor> B;
+    B.emplace(1, 2);
+    EXPECT_EQ(0u, MultiArgConstructor::Destructions);
+    ASSERT_EQ(A, B);
+  }
+  EXPECT_EQ(2u, MultiArgConstructor::Destructions);
+}
+
 struct MoveOnly {
   static unsigned MoveConstructions;
   static unsigned Destructions;
   static unsigned MoveAssignments;
   int val;
   explicit MoveOnly(int val) : val(val) {
   }
   MoveOnly(MoveOnly&& other) {
TEST(OptionalTest, ImmovableEmplace) {
   Immovable::ResetCounts();
   A.emplace(4);
   EXPECT_TRUE((bool)A);
   EXPECT_EQ(4, A->val);
   EXPECT_EQ(1u, Immovable::Constructions);
   EXPECT_EQ(0u, Immovable::Destructions);
 }
 
+TEST(OptionalTest, ImmovableInPlaceConstruction) {
+  Immovable::ResetCounts();
+  Optional<Immovable> A{in_place, 4};
+  EXPECT_TRUE((bool)A);
+  EXPECT_EQ(4, A->val);
+  EXPECT_EQ(1u, Immovable::Constructions);
+  EXPECT_EQ(0u, Immovable::Destructions);
+}
+
 // Craft a class which is_trivially_copyable, but not
 // is_trivially_copy_constructible.
 struct NonTCopy {
   NonTCopy() = default;
 
   // Delete the volatile copy constructor to engage the "rule of 3" and delete
   // any unspecified copy assignment or constructor.
   NonTCopy(volatile NonTCopy const &) = delete;