boostorg / core

Boost Core Utilities
132 stars 86 forks source link

Add `functor` #161

Closed Lastique closed 5 months ago

Lastique commented 5 months ago

This PR:

functor is a C++17 utility class template that wraps a raw function into a function object class. The raw function is specified in a non-type template parameter, and hence its pointer is not stored internally as a data member (i.e. the functor class is empty). functor::operator() forwards any arguments to the wrapped raw function and returns its result.

The primary use case is converting raw deleter functions into function object classes for std::unique_ptr, unique_resource and similar. However, it may also be useful with std::shared_ptr and std::bind and similar utilities to improve efficiency (compared to using a pointer to function) and/or code size (compared to using a lambda).

Lastique commented 5 months ago

Re. posix-cmake-test on Mac OS, those seem to time out randomly and not caused by this change. I think db20a49e480aa43a7c65cfa79d9b9d55053924f6 was a red herring, since when these jobs succeed, they do in less than 5 minutes, so something about these jobs seems to be wrong. They timeout after the compilation is complete, so I'm not sure what's going on there.

pdimov commented 5 months ago

I'm sorry, why have you removed C++03 from CI?

Lastique commented 5 months ago

Because it was failing to build because Boost.Bind dropped it. See here, for example. And Boost.SmartPtr warns that it will remove it soon. As well as Boost.Optional and Boost.Function. Those libraries are pulled indirectly through Boost.Serialization.

pdimov commented 5 months ago

Should be fixed.

Lastique commented 5 months ago

You mean I should remove the "drop C++03" commit from this PR?

pdimov commented 5 months ago

Yeah, CMake on macOS started randomly hanging at some point. I think it was when they updated CMake to the latest version, but it might be something else. No idea what causes it or how it can be fixed.

pdimov commented 5 months ago

You mean I should remove the "drop C++03" commit from this PR?

Yes.

Lastique commented 5 months ago

You mean I should remove the "drop C++03" commit from this PR?

Yes.

Done.

Lastique commented 5 months ago

Are there any comments or objections to merging this?

pdimov commented 5 months ago

It's a good component, although there's always the question of putting more things into Core.

I think that you should document and test that functor is callable with some arguments if and only if Function is callable with them, and change the implementation to do that. At the moment I think that neither decltype(auto) nor noexcept have the effect of SFINAE-ing properly; it's better to use decltype(expr) as the return type.

The Returns and Throws clauses should probably be replaced with Effects: return ...;

Lastique commented 5 months ago

I can SFINAE-enable the operator() but is there a case where that would be useful, other than type traits? Function objects don't participate in overload resolution.

The Returns and Throws clauses should probably be replaced with Effects: return ...;

Yes, although I would keep "Throws:" since the synopsis shows noexcept(...) for brevity, implying that the noexcept-ness will be documented in the operator() documentation.

pdimov commented 5 months ago

I can't give you an example right now, but a lot of places tend to use this type trait.

Lastique commented 5 months ago

I think that you should document and test that functor is callable with some arguments if and only if Function is callable with them, and change the implementation to do that. At the moment I think that neither decltype(auto) nor noexcept have the effect of SFINAE-ing properly; it's better to use decltype(expr) as the return type.

The Returns and Throws clauses should probably be replaced with Effects: return ...;

Done.