[PATCH] D100671: [ADT] Factor out in_place_t and expose in Optional ctor

[Scott Linder via Phabricator via llvm-commits]

scott.linder added a comment.

In D100671#2695923 <https://reviews.llvm.org/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)



================
Comment at: llvm/unittests/ADT/OptionalTest.cpp:227
+                         const MultiArgConstructor &RHS) {
+    return LHS.x == RHS.x && LHS.y == RHS.y;
+  }
----------------
dblaikie wrote:
> 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.
Will do, thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100671