Support etl::inplace_function #1046

jiangyilism · 2025-03-06T07:31:12Z

Provide similar api to std::function.

Current use case is mainly to hold a r-value lambda object, which etl::delegate is unsuitable since etl::delegate does not extend the lifetime of r-value lambda object.

Store a callable inside the inplace_function object's member field in a type-erasure way.
The member field, i.e. storage or buffer, has a compile-time fixed size.
The size is specified either by the macro ETL_INPLACE_FUNCTION_DEFAULT_CAPACITY or a non-type template parameter.

The implementation is inspired by:

SG14 inplace_function
folly::Function
function2

jiangyilism · 2025-03-06T07:54:16Z

include/etl/private/inplace_function_cpp14.h

+      using dtor_func_t = void (*)(storage_ptr_t);
+
+      const invoke_func_t       invoke_func{[](storage_ptr_t, Args&&...) -> R
+                                      { return R(); }};


std::function throws std::bad_function_call if operator() is called on an empty object while etl::inplace_function safely returns a default-constructed object in this case.
That means type typename R is required to be default-constructible.
void type is fine here ( https://timsong-cpp.github.io/cppwp/std14/expr.type.conv )
But this will not compile since std::is_default_constructible_v<Type_a> is false:

struct Type_a { Type_a(int32_t val) : val_(val) {} int32_t val_; }; etl::inplace_function<Type_a()> func0;

This deviation from std::function is by designed and also tested in test_inplace_function.cpp

In etl::delegate an ETL error is raised.
ETL_ASSERT(is_valid(), ETL_ERROR(delegate_uninitialised));

Also in etl::delegate there are call_if and call_or member functions that handle uninitialised objects.

Updated to make it raise etl::bad_inplace_function_call when an empty etl::inplace_function is invoked with operator() . (Similar to std::bad_function_call)
std::function does not support call_if() and call_or(). Do you suggest adding these 2 member func?

@jiangyilism I suggest adding them. It avoids having to check if the object is assigned anything, and helps moving from delegate.

clear() would also be nice to have for delegate compatibility.

jwellbelove · 2025-03-06T19:22:37Z

include/etl/private/inplace_function_cpp14.h

+https://github.com/ETLCPP/etl
+https://www.etlcpp.com
+
+Copyright(c) 2025 BMW AG


How close a copy is this of the implementations that 'inspired' you?
There may be copyright implications.

The closest open source implementation is SG14 inplace_function.
etl::inplace_function compiled object memory layout will be almost identical (except alignment).
The main differences:

etl is compatible with c++11 while others, including SG14, typically requires c++14

etl one has no member/non-member function swap(), operator==(nullptr_t), operator!=(nullptr_t) .

etl does not utilize aligned storage, which will be deprecated in c++23.

etl renames most of the class and variable names, except the name "inplace_function", which can be renamed too.

etl simplifies some template meta programming with etl type_traits header

move almost all stuffs in private namespace into private sections of classes.

jwellbelove · 2025-03-06T19:33:10Z

include/etl/private/inplace_function_cpp14.h

+      typename T,
+      typename C = etl::decay_t<T>,
+      typename = etl::enable_if_t<
+        !private_inplace_function::is_inplace_function<C>::value && etl::is_invocable_r<R, C&, Args...>::value>>


This class has limited usefulness as is restricted to STL and C++14 and above.
Many of the users of the ETL do not/cannot use the STL.

etl::is_invocable_r depends on ETL_USING_CPP11 but not STL.
I updated to make etl::inplace_function compatible to c++11.

denravonska · 2025-03-15T11:44:05Z

The missing R-value support blocks us from upgrading ETL. This will be great :)

BartolomeyKant · 2025-04-01T05:31:11Z

include/etl/private/inplace_function_cpp11.h

+      explicit ETL_CONSTEXPR vtable_t(etl::type_identity<T>) ETL_NOEXCEPT
+        : invoke_func{[](char* pobj, Args&&... args) -> R
+                      { return (*reinterpret_cast<T*>(pobj))(
+                          etl::forward<Args&&>(args)...); }},


I'm confused about this construction. Why etl::forward<Args&&> instead of etl::forward<Args> is used here?

Thanks. Fixed to etl::forward<Args> .

BartolomeyKant · 2025-04-01T05:40:16Z

include/etl/private/inplace_function_cpp11.h

+      ETL_ASSERT(vtable_ptr->invoke_func, ETL_ERROR(bad_inplace_function_call));
+      return vtable_ptr->invoke_func(
+        obj,
+        etl::forward<Args>(args)...);


Does conversion from Args... to Args&&... is working here?

For example I want to pass some struct Foo by value into inplace_function. Would it be copied three times?

First in call to this operator(), second in invoke_func call, and third in call to actual functor inside the invoke_func lambda?

I'm not an expert I'm just wondering how this should work.

Maybe std::move(args) should be used here, and then std::forward<Args>(args) in invoke_func lambda?

It is copied twice under gcc/clang -O0 , -O1 and -O2 . Using etl::move/std::move still copies twice.
A test case NoRedundentCopy is added to verify that during inplace_function::operator(), if the contained callable signature is:

call-by-value: then at most 2 copyings are made in inplace_function::operator().

call-by-reference: then no copying is made.

This example gives me one copy and one move https://godbolt.org/z/3YPMccbnz

Yes. And if the move constructor is not explicitly defined then copy constructor is called instead. NoRedundentCopy only checks the number of copy is less than 3 . It roughly checks that inplace_function is not copying too many times than expected. It does not guarantee the exact number of copying.

With gcc:

copy ctor not explicitly defined : copy twice

copy ctor =default : copy once

copy ctor explicitly defined : copy once

denravonska · 2025-04-03T06:29:37Z

Out of curiosity, what's using all the space? I had to bump the size to 48 bytes for an inplace_function<void(void *)> on Cortex-M33.

rolandreichweinbmw · 2025-04-03T07:14:35Z

Out of curiosity, what's using all the space? I had to bump the size to 48 bytes for an inplace_function<void(void *)> on Cortex-M33.

Depends on what you save inside. If it is just a "void f1(void*){}" it maybe fits?

But if it's a lambda capturing a number of arguments by value inside, it may be much more.

rolandreichweinbmw · 2025-04-03T07:16:18Z

John, can you please enable the CI workflows for this PR?

denravonska · 2025-04-03T07:20:41Z

Out of curiosity, what's using all the space? I had to bump the size to 48 bytes for an inplace_function<void(void *)> on Cortex-M33.

Depends on what you save inside. If it is just a "void f1(void*){}" it maybe fits?

But if it's a lambda capturing a number of arguments by value inside, it may be much more.

Actually, I accidentally compiled for host so it's 48 bytes on a 64 bit system which makes a bit more sense.

BartolomeyKant · 2025-04-03T07:44:58Z

64 bit system

What if ETL_INPLACE_FUNCTION_DEFAULT_CAPACITY would be calculated based on pointer size, like sizeof(void*)*4?

denravonska · 2025-04-03T07:59:51Z

include/etl/private/inplace_function_cpp11.h

+#include "../utility.h"
+
+#ifndef ETL_INPLACE_FUNCTION_DEFAULT_CAPACITY
+  #define ETL_INPLACE_FUNCTION_DEFAULT_CAPACITY 32


Can this be based on the target architecture? When building for 64 bit the bare minimum (void()) seems to be 40 bytes.

rolandreichweinbmw · 2025-04-03T08:03:08Z

64 bit system

What if ETL_INPLACE_FUNCTION_DEFAULT_CAPACITY would be calculated based on pointer size, like sizeof(void*)*4?

Similar to etl::delegate and etl::function, inplace_function has one single global size.

This one needs to store all possible closures in a whole project that uses ETL for a inplace_function template instance type. I.e. it depends.

E.g. for a simple etl::inplace_function<void(void *)> as mentioned by Denravonska, you might save different closure implementations, e.g.:

small: void f1(void*){}
big: [=]<void(void *)>(){a(b);c(d);e(f);g(h);}

I.e. needs to be adjusted if necessary for every project, to not waste memory.

I'm not sure if you can find a way to automatically calculate a global maximum size over all user locations. Finally, you can only find at link time (or after compiling all comile units) if you have captured all necessary source locations.

BartolomeyKant · 2025-04-03T08:18:25Z

This one needs to store all possible closures in a whole project that uses ETL

I agree with that, and it's the library user's responsibility to use only closures that fit or provide a large enough value to inplace_function Capacity.

But why did 32 was chosen for default?

For me the usual case closure captures some values by reference i.e. maximum 4 pointers for 32 byte storage on 64 bit system.
What if I had tested my application on system with 4 bytes pointers and capture 8 pointers in closure, and then moved to 8 byte sized pointer system?

@denravonska also asked about ETL_INPLACE_FUNCTION_DEFAULT_CAPACITY based on architecture.

I think solution based on sizeof(void*) is the simplest. The smaller pointers the smaller inplace_function and vice versa.

rolandreichweinbmw · 2025-04-03T08:40:34Z

Hi @BartolomeyKant and @denravonska , I think now I got your point. :-) And I agree that different defaults for e.g. 32 and 64 bit platforms can be provided.

Even though an individual ETL_INPLACE_FUNCTION_DEFAULT_CAPACITY will typically be part of a project's etl_profile.h anyway.

The original default of 32 covered our initial use cases in a 32 bit project.

Luckily, the compiler will tell you at build time if it doesn't fit, as shown in @denravonska 's example.

denravonska · 2025-04-03T10:39:03Z

Hi @BartolomeyKant and @denravonska , I think now I got your point. :-) And I agree that different defaults for e.g. 32 and 64 bit platforms can be provided.

Even though an individual ETL_INPLACE_FUNCTION_DEFAULT_CAPACITY will typically be part of a project's etl_profile.h anyway.

The original default of 32 covered our initial use cases in a 32 bit project.

Luckily, the compiler will tell you at build time if it doesn't fit, as shown in @denravonska 's example.

I thought about this a bit more. My original concern was that the default size was valid for 32 bit but failed to compile on 64 bit, so maybe the default should be the smallest size required to fit the simplest inplace function given the target architecture.

However this only applies to the default function. As soon as you do anything fancier you need to handle the size differences in the application anyway.

jiangyilism · 2025-04-03T14:21:44Z

Thanks for all the feedback.
@jwellbelove As you said in the slack channel discussion, your have a plan for a new fixed-storage function/delegate . Do you think I should drop this PR?

denravonska · 2025-04-03T15:02:54Z

Thanks for all the feedback. @jwellbelove As you said in the slack channel discussion, your have a plan for a new fixed-storage function/delegate . Do you think I should drop this PR?

He also said

I may decided to go with the etl::inplace_function instead.

I have no major quarrel with the implementation. The only request I have so far is a default size that works for both 32- and 64 bit systems.

jwellbelove · 2025-04-04T12:50:59Z

@jiangyilism No, keep it active for the time being. I'm in Greece at the moment, so I'll look at this functionality again when I get back next week.

jiangyilism · 2025-04-07T06:12:41Z

@denravonska , @BartolomeyKant
ETL_INPLACE_FUNCTION_DEFAULT_CAPACITY is changed to 8 * sizeof(void*) .

rolandreichweinbmw · 2025-04-07T11:39:40Z

test/test_inplace_function.cpp

+{
+  SUITE(test_inplace_function)
+  {
+    TEST(NoRedundentCopy)


NoRedundantCopy

denravonska

It would be a quality of life change with .call_if(). Consider

for(auto cb : callbacks) {
   if(cb) {
      cb();
   }
}

vs

for(auto cb : callbacks) {
   cb.call_if();
}

That, .call_or and a .clear would make it a lot more compatible with delegate.

Provide similar api to std::function. Store a callable inside the inplace_function object's member field in a type-erasure way. The member field, i.e. storage or buffer, has a compile-time fixed size. The size is specify either by the macro ETL_INPLACE_FUNCTION_DEFAULT_CAPACITY or a non-type template parameter. The implementation is inspired by: 1. SG14 inplace_function 2. folly::Function 3. function2

jiangyilism · 2025-04-07T13:00:05Z

call_if

std::function does not support call_if, call_or, clear .
std::function and etl::inplace_function clear its content with operator=(nullptr_t) noexcept, which is now tested in HasValue test case.

If we want all etl::delegate api then it might be better to make inplace_function and delegate into a same class.
We need @jwellbelove 's feedback to continue with one of the directions:

extend inplace_function with api compatible with std::function. We can rename inplace_function though.
merge inplace_function into delegate
merge inplace_function (and delegate?) into etl::function
something else?

denravonska · 2025-04-07T13:14:51Z

call_if

std::function does not support call_if, call_or, clear . std::function and etl::inplace_function clear its content with operator=(nullptr_t) noexcept, which is now tested in HasValue test case.

If we want all etl::delegate api then it might be better to make inplace_function and delegate into a same class. We need @jwellbelove 's feedback to continue with one of the directions:

extend inplace_function with api compatible with std::function. We can rename inplace_function though.

merge inplace_function into delegate

merge inplace_function (and delegate?) into etl::function

something else?

I'm thinking more from the convenience perspective. I think a very common use case is going to be moving from delegate to inplace_function once you realize the limitations of the former and are willing to pay the cost of the latter. delegate has the luxury of not having a real counter part in STL so it can be a bit more free in its API.

I guess it all boils down to how true to the standard ETL wants to be in terms of adding functionality that's outside of the specification.

rolandreichweinbmw · 2025-04-07T13:55:38Z

I'd propose @jiangyilism 's option 1. in this way: Be close to std::function to minimize confusion, and if possible rename etl::inplace_function to etl::function. The latter of course only if @jwellbelove decides to abandon the old (already deprecated) etl::function.

jwellbelove · 2025-04-07T14:56:01Z

@rolandreichweinbmw The current etl::function (based on virtual functions) is indeed deprecated.
I have a complete std::function clone in a private branch, although it currently has the same limitation as etl::delegate when it comes to lambdas, which is why I 'parked' it for the time being. Its only real advantage over etl::delegate was that it had the same API as std::function (apart from lambdas) and could accept runtime function pointers.
It's possible I may be able to add runtime function pointers to etl::delegate without affecting constexpr compatibility.

I don't have any problem with adding functions that enhance the usability of an ETL class; making an exact clone of the STL was never my intention.

jwellbelove · 2025-04-07T15:02:46Z

std::function does not support call_if, call_or, clear .\nstd::function and etl::inplace_function clear its content with operator=(nullptr_t) noexcept

I've always treated the STL as inspiration for classes in the ETL, not as a rigid specification for what can and can't be included.

jwellbelove · 2025-04-07T15:08:06Z

I have limited options to review this PR at the moment, as I'm on holiday in Kos. I'm not quite sure when I will get back, as the Greek air traffic controllers are threatening a strike on the day we are supposed to fly home, and the postponed flights may be backed up for a day or two.

jiangyilism commented Mar 6, 2025

View reviewed changes

rolandreichweinbmw approved these changes Mar 6, 2025

View reviewed changes

jwellbelove assigned jwellbelove and jiangyilism Mar 6, 2025

jwellbelove added this to Embedded Template Library Mar 6, 2025

github-project-automation bot moved this to In progress in Embedded Template Library Mar 6, 2025

jwellbelove reviewed Mar 6, 2025

View reviewed changes

jiangyilism force-pushed the yichiang/support_inplace_function branch 2 times, most recently from af095d7 to 064f21d Compare March 13, 2025 16:50

jiangyilism requested a review from jwellbelove March 13, 2025 16:51

jiangyilism force-pushed the yichiang/support_inplace_function branch from 064f21d to 2a9fb5d Compare March 17, 2025 09:21

BartolomeyKant reviewed Apr 1, 2025

View reviewed changes

denravonska reviewed Apr 3, 2025

View reviewed changes

Support c++17 is_invocable_r

2db6667

jiangyilism force-pushed the yichiang/support_inplace_function branch from 2a9fb5d to a8c9a54 Compare April 7, 2025 06:09

jiangyilism force-pushed the yichiang/support_inplace_function branch from a8c9a54 to 2362d9d Compare April 7, 2025 06:15

BartolomeyKant approved these changes Apr 7, 2025

View reviewed changes

rolandreichweinbmw reviewed Apr 7, 2025

View reviewed changes

denravonska suggested changes Apr 7, 2025

View reviewed changes

jiangyilism force-pushed the yichiang/support_inplace_function branch from 2362d9d to d5726a9 Compare April 7, 2025 12:59

Support etl::inplace_function #1046

Are you sure you want to change the base?

Support etl::inplace_function #1046

Conversation

jiangyilism commented Mar 6, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jwellbelove Mar 6, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

denravonska commented Mar 15, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BartolomeyKant Apr 1, 2025 • edited Loading

Choose a reason for hiding this comment

BartolomeyKant Apr 1, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

denravonska commented Apr 3, 2025

rolandreichweinbmw commented Apr 3, 2025

rolandreichweinbmw commented Apr 3, 2025

denravonska commented Apr 3, 2025

BartolomeyKant commented Apr 3, 2025

Choose a reason for hiding this comment

rolandreichweinbmw commented Apr 3, 2025 • edited Loading

BartolomeyKant commented Apr 3, 2025 • edited Loading

rolandreichweinbmw commented Apr 3, 2025

denravonska commented Apr 3, 2025

jiangyilism commented Apr 3, 2025

denravonska commented Apr 3, 2025

jwellbelove commented Apr 4, 2025

jiangyilism commented Apr 7, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

denravonska left a comment

Choose a reason for hiding this comment

jiangyilism commented Apr 7, 2025

denravonska commented Apr 7, 2025 • edited Loading

rolandreichweinbmw commented Apr 7, 2025

jwellbelove commented Apr 7, 2025 • edited Loading

jwellbelove commented Apr 7, 2025

jwellbelove commented Apr 7, 2025

jwellbelove Mar 6, 2025 •

edited

Loading

BartolomeyKant Apr 1, 2025 •

edited

Loading

BartolomeyKant Apr 1, 2025 •

edited

Loading

rolandreichweinbmw commented Apr 3, 2025 •

edited

Loading

BartolomeyKant commented Apr 3, 2025 •

edited

Loading

denravonska commented Apr 7, 2025 •

edited

Loading

jwellbelove commented Apr 7, 2025 •

edited

Loading