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

ENGINES: Use override #3531

Merged
merged 7 commits into from Nov 14, 2021
Merged

ENGINES: Use override #3531

merged 7 commits into from Nov 14, 2021

Conversation

@orgads
Copy link
Contributor

@orgads orgads commented Nov 13, 2021

Using clang-tidy modernize-use-override

@digitall
Copy link
Member

@digitall digitall commented Nov 14, 2021

@orgads : I would suggest splitting this change into a commit per engine as per your PR #3533 ... This makes bisecting for regressions and possible commit reversion in future easier.

Loading

@orgads
Copy link
Contributor Author

@orgads orgads commented Nov 14, 2021

There are 13 files overall, and the change is mechanical, so I don't think it's worth splitting.

If you insist, I can do it.

Loading

@orgads
Copy link
Contributor Author

@orgads orgads commented Nov 14, 2021

Note that buried had many changes, so I pushed it as a separate PR #3530.

Loading

@digitall
Copy link
Member

@digitall digitall commented Nov 14, 2021

@orgads I know what you mean, but it is neater and as I said makes future bisection / revert much easier and less manual.

Loading

@digitall
Copy link
Member

@digitall digitall commented Nov 14, 2021

@orgads : Thanks for looking at these improvements to override / nullptrs... but I just lost a week at work dealing with a difference between the older implementation of __FUNCTION__ and the newer one based on C++-11 __func__ which should be equivalent... but had a very bloody subtle breakage... hence :)

Loading

orgads added 7 commits Nov 14, 2021
Using clang-tidy modernize-use-override
Using clang-tidy modernize-use-override
Using clang-tidy modernize-use-override
Using clang-tidy modernize-use-override
Using clang-tidy modernize-use-override
Using clang-tidy modernize-use-override
Using clang-tidy modernize-use-override
@orgads orgads force-pushed the override-engines branch from 58683c5 to e6bdcb6 Nov 14, 2021
@orgads
Copy link
Contributor Author

@orgads orgads commented Nov 14, 2021

Split done.

What are the differences between __FUNCTION__ and __func__?

Loading

@digitall
Copy link
Member

@digitall digitall commented Nov 14, 2021

Thanks. It is a little involved, but basically the older __FUNCTION__ behaved like a string literal whereas the newer __func__ is an implicit definition of a static string in the function by the compiler... __FUNCTION__ which is C99 is now an alias to the C++ version. Functionally, they are identical... except when using embedded target GCC, where the older C99 construct allowed you to reallocate to flash memory without using RAM, the newer construct does not, so any code using this ported from C99 to a newer embedded compiler compiles and runs fine... but can run out of RAM at runtime when debug is enabled. HEADACHE. There is no way of forcing the strings into program memory with the newer standard.

Loading

@bluegr
Copy link
Member

@bluegr bluegr commented Nov 14, 2021

Nice! Thanks for splitting this into different commits. Merging

Loading

@bluegr bluegr merged commit 2e2a613 into scummvm:master Nov 14, 2021
8 checks passed
Loading
@orgads orgads deleted the override-engines branch Nov 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants