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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
