Skip to content

JANITORIAL: Add missing references to const function parameters #6542

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

Merged
merged 1 commit into from
Apr 12, 2025

Conversation

mikrosk
Copy link
Contributor

@mikrosk mikrosk commented Apr 6, 2025

Applied to all const Common:: and const Graphics:: instances.

The biggest performance win is surely Common::String and Common::Path.

It also uncovered a few inconsistencies where a .h had a const parameter while its .cpp didn't.

Applied to all 'const Common::' and 'const Graphics::' instances.
@bluegr
Copy link
Member

bluegr commented Apr 12, 2025

Nice work, thanks!

@bluegr bluegr merged commit 24dcacf into scummvm:master Apr 12, 2025
8 checks passed
@mikrosk mikrosk deleted the const_ref branch April 13, 2025 13:43
@lephilousophe
Copy link
Member

It seems you switched to references enum types and pointer types.
For such types there will be a useless in direction.

@mikrosk
Copy link
Contributor Author

mikrosk commented Apr 20, 2025

@lephilousophe good point. I have reverted changes for

  • Common::CodePage
  • Common::EventType
  • Common::Keycode
  • Common::Language
  • Common::Platform

in my local branch. However I'm not sure about Common::ArchiveMemberPtr: SO suggests passing by const reference in case of shared_ptr and older C++ standard: https://stackoverflow.com/a/8741626. AFAIR we officially support C++11, i.e. shared_ptr's move semantics still isn't a thing?

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