search

Naios / function2

Improved and configurable drop-in replacement to std::function that supports move only types, multiple overloads and more
http://naios.github.io/function2
Boost Software License 1.0
545 stars 46 forks source link

Fix issue #21 #58

Closed wak-google closed 1 year ago

wak-google commented 1 year ago

This makes it possible to use const functions in function_views by fixing constness when setting the data pointer to the function.

wak-google commented 1 year ago

This definitely doesn't fix accessing it in a const context, but it should already be downcasting the void * to a proper const function pointer anyway.

Naios commented 1 year ago

@wak-google Thank you for your PR, and sorry for not responding earlier to your question in the related ticket. I'm currently quite busy.

Sadly this issue needs to be fixed without introducing an additional pointer into the storage, because the size (footprint) of the functional wrapper is a key criterium and needs to stay as low as possible.

Eventually a well-placed and checked const_cast might be the solution?

I can take an additional look at it in the upcoming days.

wak-google commented 1 year ago

@Naios yeah, I could just const cast instead. However, this doesn't change the size because it's a member of a union, not a struct.

Naios commented 1 year ago

Yes, you are right. It looked in my diff as it was a struct.

wak-google commented 1 year ago

There is a smaller change

Naios commented 1 year ago

I adapted your previous commit in a follow-up commit on top already (if this is also ok for you).

Could you explain this a little bit further please? What point is missing here?

This definitely doesn't fix accessing it in a const context

wak-google commented 1 year ago

My concern was just in case something was accessing it in a non-const context. I believe all of the uses get cast back with const applied prior to calling the function, so it should be fine.

Naios commented 1 year ago

The issue is in general that the view is non-const correct anymore like all view types are, since you are allowed to create a mutable view from a const view just by copying.

Naios commented 1 year ago

Thank you for your contribution, I highly appreciate it! Well done.