[libcxx] Introducing _LIBCPP_INLINE_VARIABLE

[AntonBikineev]

Summary

Hey Eric,
This is so *cosmetic*, but wouldn't it be more consistent with other macros like _LIBCPP_CONSTEXPR, etc..?

[EricWF]

I didn't add _LIBCPP_INLINE_VARIABLE because we shouldn't make a habit of having variables that are sometimes inline and sometimes not. So it should be as ugly as possible when you have to do it. Hopefully the current usages in <utility> will be all we ever need.

[EricWF]

Resigning as review because I don't think this patch should proceed.

[AntonBikineev]

In D26859#600024, @EricWF wrote:

I didn't add _LIBCPP_INLINE_VARIABLE because we shouldn't make a habit of having variables that are sometimes inline and sometimes not. So it should be as ugly as possible when you have to do it. Hopefully the current usages in <utility> will be all we ever need.

Eric, I can see a potential for this macro. We have a lot of tag types instances in Standard Library -- seq, par, piecewise_construct, nullopt, ignore to name a few. All of them are considered to become inline to prevent odr-violations. There are 2 National Body comments addressing this issue (GB28, FI9). I absolutely agree about monstrosity of such macros, but from my perspective it's inevitable.

[EricWF]

In D26859#604261, @AntonBikineev wrote:

In D26859#600024, @EricWF wrote:

I didn't add _LIBCPP_INLINE_VARIABLE because we shouldn't make a habit of having variables that are sometimes inline and sometimes not. So it should be as ugly as possible when you have to do it. Hopefully the current usages in <utility> will be all we ever need.

Eric, I can see a potential for this macro. We have a lot of tag types instances in Standard Library -- seq, par, piecewise_construct, nullopt, ignore to name a few. All of them are considered to become inline to prevent odr-violations. There are 2 National Body comments addressing this issue (GB28, FI9). I absolutely agree about monstrosity of such macros, but from my perspective it's inevitable.

I'll be happy to reconsider when that actually comes to fruition.

include/__config

 # define _LIBCPP_HAS_NO_INLINE_VARIABLES
 #endif
 
+#if !defined(_LIBCPP_HAS_NO_INLINE_VARIABLES)
+# define _LIBCPP_INLINE_VARIABLE inline
+#else
+# define _LIBCPP_INLINE_VARIABLE
+#endif
+
 #ifdef _LIBCPP_HAS_NO_RVALUE_REFERENCES
 #  define _LIBCPP_EXPLICIT_MOVE(x) _VSTD::move(x)
 #else

include/utility

 struct _LIBCPP_TYPE_VIS in_place_t {
     explicit in_place_t() = default;
 };
-#ifndef _LIBCPP_HAS_NO_INLINE_VARIABLES
-inline
-#endif
+_LIBCPP_INLINE_VARIABLE
 constexpr in_place_t in_place{};
 
 template <class _Tp>

     explicit in_place_type_t() = default;
 };
 template <class _Tp>
-#ifndef _LIBCPP_HAS_NO_INLINE_VARIABLES
-inline
-#endif
+_LIBCPP_INLINE_VARIABLE
 constexpr in_place_type_t<_Tp> in_place_type{};
 
 template <size_t _Idx>

     explicit in_place_index_t() = default;
 };
 template <size_t _Idx>
-#ifndef _LIBCPP_HAS_NO_INLINE_VARIABLES
-inline
-#endif
+_LIBCPP_INLINE_VARIABLE
 constexpr in_place_index_t<_Idx> in_place_index{};
 
 template <class _Tp> struct __is_inplace_type_imp : false_type {};