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

Duncan P. N. Exon Smith via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 16 17:00:19 PDT 2021


dexonsmith added a comment.

In D100671#2696123 <https://reviews.llvm.org/D100671#2696123>, @scott.linder wrote:

> 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)

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100671



More information about the llvm-commits mailing list