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

Suppress formattable hides the global definition warning #4378

Closed
wants to merge 2 commits into from

Conversation

evanwporter
Copy link

@evanwporter evanwporter commented Mar 8, 2025

This fixes the warning that is received on MSVC v17.3.6 with the compiler flag /W4. The warning is as follows: declaration of 'formattable' hides global declaration. It appears on every file where fmt/core.h is included.

I renamed it to is_formattable but this can be changed since it seems like is_formattable is used in other places as well.

EDIT: After looking through other PRs, namely #4377, I thought it may be preferred (and less breaking) to suppress the warnings via macros rather than change existing code.

This fixes the warning that is received on MSVC v17.3.6 with the compiler flag `/W4`. The warning is as follows: `declaration of 'formattable' hides global declaration`. It appears on every file where `fmt/core.h` is included.
@evanwporter evanwporter changed the title Rename formattable so it doesn't hide the global definition Suppress formattable hides the global definition warning Mar 8, 2025
@vitaut
Copy link
Contributor

vitaut commented Mar 9, 2025

Thanks for the PR but we try to avoid cluttering the code with compiler-specific pragmas.

@vitaut vitaut closed this Mar 9, 2025
@evanwporter
Copy link
Author

@vitaut

Idk if its possible for me to reopen but I renamed formattable to is_it_formattable. This will get rid of MSVC warning 4459. At the same time it avoids conflicts with the already named variable is_formattable

@dinomight
Copy link
Contributor

@evanwporter See #4354 . The issue should go away in newer compilers, even if the fix for older compilers isn't significant.

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