Skip to content
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

pass std::function by const ref #231

Merged
merged 3 commits into from
Jan 28, 2024
Merged

pass std::function by const ref #231

merged 3 commits into from
Jan 28, 2024

Conversation

neheb
Copy link
Contributor

@neheb neheb commented Jan 28, 2024

Too expensive to pass by value.

@robUx4

std::function is not trivially copyable, while means it's more efficient
to pass by const ref.

This change allows clang-tidy to make the necessary modifications
everywhere else.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Last one remaining. std::function should not be passed by value.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
@neheb
Copy link
Contributor Author

neheb commented Jan 28, 2024

Oh interesting. This was introduced with #173

Copy link
Contributor

@robUx4 robUx4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, it will require the same changes on the libmatroska side. Some of that code is generated, but I can fix that.

@robUx4 robUx4 added api-break breaks the API (e.g. programs using it will have to adjust their source code) abi-break breaks the ABI (e.g. programs linked against the library have to be recompiled) labels Jan 28, 2024
@mbunkus
Copy link
Contributor

mbunkus commented Jan 28, 2024

Nice, thanks.

@mbunkus mbunkus merged commit d369800 into Matroska-Org:master Jan 28, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
abi-break breaks the ABI (e.g. programs linked against the library have to be recompiled) api-break breaks the API (e.g. programs using it will have to adjust their source code)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants