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

misc: avoid unnecesary shadowed variable #287

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

carenas
Copy link
Contributor

@carenas carenas commented Jan 7, 2025

Found when building the latest PCRE2 RC with gcc in macOS

@zherczeg
Copy link
Owner

zherczeg commented Jan 7, 2025

Is this related to #286 or something different?

@carenas
Copy link
Contributor Author

carenas commented Jan 8, 2025

Is this related to #286 or something different?

it is fundamentally the same issue (sorry, only found out there was one, after this one was already out), but the solution is different and I think could be complimentary.

When building with the default allocator that uses a global named
`total_size`, uses of a similar variable mame elsewhere will
trigger warnings.

Rename the local variable to something more accurate and make sure
that by default the warning that will trigger otherwise (supported by
gcc and clang) is added to cover regressions.
@zherczeg
Copy link
Owner

zherczeg commented Jan 8, 2025

I think total_size global now got a new name. Are these longer names still necessary?

@carenas
Copy link
Contributor Author

carenas commented Jan 8, 2025

a longer (more descriptive) name is always better IMHO, and changing the one used locally in all those RISC CPU files which seem to be somehow related is good on itself, but you are correct that with the renaming of the global (which hopefully is not being used outside our codebase and therefore doesn't "break ABI") it is no longer needed to avoid -Wshadow related warnings.

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.

2 participants