-
-
Notifications
You must be signed in to change notification settings - Fork 519
fix: take handler for sol_lua_check
as function reference
#6393
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Want this to wait until the Link PR is merged in?
I'd be fine with both. Maybe @Mm2PL has a preferred order. |
I'm fine with this being merged before link PR is merged, I'll deal with the conflicts. That one still needs a little more time in the oven. |
I mentioned this in #6386 (comment) - we shouldn't take the handler as a
std::function
which could potentially allocate. The handler is called when the check fails, so it's not stored anywhere. Sol passes the handler as a template argument. We could do the same, but this would mean that we'd need to define the functions in the header. Instead, we can use a function reference (likestd::function_ref
). Since we aren't on C++ 26, we need to add this ourselves. I usedllvm::function_ref
as the base for this, since it's concise (the actual implementation in libc++ and libstdc++ is larger because they handle more cases like noexcept/const/volatile, but the idea of keeping two pointers is the same). We're using C++ 20, so we can use concepts instead of SFINAE.