[libcxx-commits] [PATCH] D96946: [libcxx][RFC] Unspecified behavior randomization in libcxx

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/include/algorithm:3113
+
+template <class _Dummy>
+class _LIBCPP_TYPE_VIS __random_unspecified_behavior_gen_
----------------
Quuxplusone wrote:
> danlark wrote:
> > ldionne wrote:
> > > Why are we making this a template?
> > Because there is no algorithm.cpp and I decided not to create one because of such feature. It is needed to instantiate `static const void* const __seed_static;` and inline variables are only a C++17 feature, before that it was a common trick to put the static variable definition in header
> > 
> > See
> > 
> > ```
> > template <class _Void>
> > const void* const __random_unspecified_behavior_gen_<_Void>::__seed_static =
> >     &__seed_static;
> > ```
> Seems like it would be way simpler to just do the //other// C++ singleton trick, which is "make the thing a function-local static variable":
> ```
>     _LIBCPP_ALWAYS_INLINE static result_type __seed() {
> #ifdef _LIBCPP_RANDOMIZE_UNSPECIFIED_STABILITY_SEED
>       return _LIBCPP_RANDOMIZE_UNSPECIFIED_STABILITY_SEED;
> #else
>       static char __x;
>       return reinterpret_cast<uintptr_t>(&__x);
> #endif
>     }
> ```
> I see no reason for any of the helper machinery here; it looks like we're just trying to generate a "random seed" from an address, so let's just do that. No global variables, no templates (not even any unnecessary casts!), nice and simple.
Yeah, agreed, IMO that seems cleaner.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96946