Skip to content

Conversation

Nerixyz
Copy link
Contributor

@Nerixyz Nerixyz commented Aug 14, 2025

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 (like std::function_ref). Since we aren't on C++ 26, we need to add this ourselves. I used llvm::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.

Copy link
Contributor

@github-actions github-actions bot left a 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

Copy link
Member

@pajlada pajlada left a 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?

@Nerixyz
Copy link
Contributor Author

Nerixyz commented Aug 16, 2025

Want this to wait until the Link PR is merged in?

I'd be fine with both. Maybe @Mm2PL has a preferred order.

@Mm2PL
Copy link
Collaborator

Mm2PL commented Aug 16, 2025

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.

@pajlada pajlada enabled auto-merge (squash) August 16, 2025 18:46
@pajlada pajlada merged commit 0f11a90 into Chatterino:master Aug 16, 2025
18 checks passed
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.

3 participants