boostorg / core

Boost Core Utilities
132 stars 87 forks source link

empty_init convenience #63

Closed breese closed 4 years ago

breese commented 4 years ago

Instantiating boost::empty_value with arguments is done using the empty_init_t tag. This means that we have to write something like base(empty_init_t{}, args....

Can we add an empty_init instance of that tag, so we can write base(empty_init, args...)?

We can avoid the usual problems of header-only instantiations of a global object by changing struct empty_init_t {}; into enum empty_init_t { empty_init };.

glenfe commented 4 years ago

A BOOST_STATIC_CONSTEXPR empty_init_t empty_init = empty_init_t(); should satisfy that request too, right?

breese commented 4 years ago

That will work.

The reason I mention the enum alternative, is that when I had to investigate the binary size of a large embedded project, I found a boost::none (from Boost.Optional) instance in each translation unit that used this constant. While this is not a major problem per se, the enum solution completely avoids it, which is why I prefer enum for tagging.

Lastique commented 4 years ago

I suppose, we should mark boost::none inline on C++17 compilers. That should mitigate binary size bloat.

glenfe commented 4 years ago

It's also why I never created an empty_init constant initially. The four extra characters "_t()" did not seem like.a big deal. :)

Users will always create constants if they want them, maybe even shorter names than the ones we might provide.

(We could even just create a type alias for empty_init_t to a shorter type name.)

Lastique commented 4 years ago

I think the mental pattern to assign a global value (think nullopt, nullptr, nothrow) is too strong. And asking users to create constants isn't solving anything - they will still have the binary bloat. It only shifts the burden on the users.

breese commented 4 years ago

There is an issue for boost::none.

C++17 inline constant is good general solution, but for integers or tags the enum solution works with older compilers as well.

Lastique commented 4 years ago

Enum, if we're talking about unscoped enums, has the potential to be converted to int, which can bite if a function overload accepts int and a type constructible from the enum. I believe, the compiler should pick the int overload because this is a builtin conversion as opposed to a user-defined constructor.

pdimov commented 4 years ago

constexpr is automatically inline, but none isn't BOOST_STATIC_CONSTEXPR.

pdimov commented 4 years ago

Wait, no, this was only true for static class members. Namespace-scope constexpr variables aren't automatically inline. Sad.

Maybe we need to add inline to BOOST_STATIC_CONSTEXPR.

glenfe commented 4 years ago

I'm not a fan of the enum approach; I prefer keeping empty_init_t a mono state type (like nullopt_t, nothrow_t, inplace_t). Maybe a new Boost.Config macro, with this intention in mind. e.g. BOOST_INLINE_CONSTEXPR which is inline constexpr if available, static constexpr if not.

Lastique commented 4 years ago

Honestly, I don't see the point of BOOST_STATIC_CONSTEXPR (or BOOST_INLINE_CONSTEXPR) given that we already have BOOST_CONSTEXPR_OR_CONST. It might be useful to add BOOST_INLINE_VARIABLE, which expands to inline or nothing depending on BOOST_NO_CXX17_INLINE_VARIABLES.

glenfe commented 4 years ago

Sounds fine to me. So in this case, usage would be of the form: BOOST_INLINE_VARIABLE BOOST_CONSTEXPR_OR_CONST type name = type(); ?

Lastique commented 4 years ago

Yes.

glenfe commented 4 years ago

And in the case where BOOST_NO_CXX17_INLINE_VARIABLES is defined, we're fine with the above too? i.e. We don't want that to have static or be in an unnamed namespace?

Lastique commented 4 years ago

Yes, we're fine. Namespace-scope constants have internal linkage by default.

glenfe commented 4 years ago

@pdimov Do you remember why bind couldn't have the placeholder constants in GCC versions before 4?

#if defined(__BORLANDC__) || defined(__GNUC__) && (__GNUC__ < 4)
pdimov commented 4 years ago

BOOST_INLINE_VARIABLE BOOST_CONSTEXPR_OR_CONST is missing the current static though. Although I'm not sure whether it's needed. Not sure why this is any improvement over BOOST_INLINE_CONSTEXPR that expands to whatever's correct.

pdimov commented 4 years ago

No, I don't.

pdimov commented 4 years ago

Looking at the history, it was probably a precompiled header problem.