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 LOG_EVERY_N with clang -Wunused-local-typedef #225

Merged
merged 2 commits into from
Aug 9, 2017

Conversation

jray272
Copy link
Contributor

@jray272 jray272 commented Aug 6, 2017

Please note, this should wait until PR #224 has been submitted as that PR includes adding me to the contributors.

Glog uses a pre-C++11 compile time assert to verify the validity of
the severity parameter for LOG_EVERY_N. Unfortunately, some compilers
will complain about the usage of LOG_EVERY_N with
"-Wunused-local-typedef" due to the way the compile time assert is
constructed. This makes it impossible to use LOG_EVERY_N with this
warning treated as an error.

The fix simply removes the assert entirely. This is safe to do since
you can't put anything invalid into the severity parameters without
generating a compile error elsewhere. This has been safe to do ever
since the GLOG_ prefixes were added as part of 6febec3.

Fixes #223

Glog uses a pre-C++11 compile time assert to verify the validity of
the severity parameter for LOG_EVERY_N. Unfortunately, some compilers
will complain about the usage of LOG_EVERY_N with
"-Wunused-local-typedef" due to the way the compile time assert is
constructed. This makes it impossible to use LOG_EVERY_N with this
warning treated as an error.

The fix simply removes the assert entirely. This is safe to do since
you can't put anything invalid into the severity parameters without
generating a compile error elsewhere. This has been safe to do ever
since the GLOG_ prefixes were added as part of 6febec3.

Fixes google#223
@shinh
Copy link
Collaborator

shinh commented Aug 9, 2017

Sounds reasonable. I believe this is the only user of GOOGLE_GLOG_COMPILE_ASSERT. Could you remove this macro altogether?

@jray272
Copy link
Contributor Author

jray272 commented Aug 9, 2017

Will do.

This compile time assert is no longer used anywhere in glog. Remove
it.
@jray272
Copy link
Contributor Author

jray272 commented Aug 9, 2017

Removed in dd19fb2

@shinh shinh merged commit 246a589 into google:master Aug 9, 2017
@shinh
Copy link
Collaborator

shinh commented Aug 9, 2017

Thanks!

@jray272 jray272 deleted the remove-log-every-n-assert branch August 9, 2017 19:40
durswd pushed a commit to durswd/glog that referenced this pull request Sep 2, 2019
Fix LOG_EVERY_N with clang -Wunused-local-typedef
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LOG_EVERY_N warns with clang -Wunused-local-typedef
3 participants