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

Fix clang -Wdisabled-macro-expansion warning from FMT_STRING_IMPL. #1575

Merged
merged 1 commit into from
Mar 6, 2020

Conversation

refnum
Copy link
Contributor

@refnum refnum commented Mar 2, 2020

FMT_STRING_IMPL has an internal helper named FMT_STRING, however FMT_STRING is also the name of the macro that invokes FMT_STRING_IMPL.

Renaming this helper avoids the appearance of a recursive macro.

@@ -3504,15 +3504,15 @@ FMT_END_NAMESPACE
#define FMT_STRING_IMPL(s, ...) \
[] { \
/* Use a macro-like name to avoid shadowing warnings. */ \
struct FMT_STRING : fmt::compile_string { \
struct INVOKE_FMT_STRING : fmt::compile_string { \
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest renaming to FMT_COMPILE_STRING to follow the naming convention for {fmt} macros as per comment above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to use FMT_COMPILE_STRING as suggested.

FMT_STRING_IMPL has an internal helper named FMT_STRING, however FMT_STRING is also the name of the macro that invokes FMT_STRING_IMPL.

Renaming this helper avoids the appearance of a recursive macro.
@vitaut vitaut merged commit 29a1ea7 into fmtlib:master Mar 6, 2020
@vitaut
Copy link
Contributor

vitaut commented Mar 6, 2020

Thanks!

@refnum refnum deleted the recursive_macro branch March 6, 2020 15:40
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