Skip to content

Conversation

@wak-google
Copy link
Contributor

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

@wak-google wak-google requested a review from Naios as a code owner July 13, 2023 02:17
@wak-google
Copy link
Contributor Author

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
Copy link
Owner

Naios commented Jul 13, 2023

@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
Copy link
Contributor Author

@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
Copy link
Owner

Naios commented Jul 13, 2023

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

@wak-google
Copy link
Contributor Author

There is a smaller change

@Naios
Copy link
Owner

Naios commented Jul 13, 2023

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
Copy link
Contributor Author

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
Copy link
Owner

Naios commented Jul 13, 2023

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 Naios closed this in 4be9277 Jul 13, 2023
@Naios
Copy link
Owner

Naios commented Jul 13, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants