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

Re-enable compile-time format-string checking #827

Merged
merged 1 commit into from
Aug 7, 2018
Merged

Re-enable compile-time format-string checking #827

merged 1 commit into from
Aug 7, 2018

Conversation

medithe
Copy link
Contributor

@medithe medithe commented Aug 7, 2018

Since the commit 691a7a9 compile-time string checking is (accidently?) not possible anymore.
With this change, it should be possible again.

Since the commit 691a7a9 compile-time string checking was accidently not possible anymore.
With this change, it should be possible again.
@vitaut vitaut merged commit 4b868b8 into fmtlib:master Aug 7, 2018
@vitaut
Copy link
Contributor

vitaut commented Aug 7, 2018

Merged, thanks!

@beojan
Copy link

beojan commented Aug 9, 2018

Does this mean compile time format checking isn't in 5.1.0? If so, perhaps this warrants a 5.1.1.

@vitaut
Copy link
Contributor

vitaut commented Aug 9, 2018

Compile-time format checking is in 5.1.0, only the UDLs are not which is not that important considering that they are non-portable anyway.

@medithe
Copy link
Contributor Author

medithe commented Aug 10, 2018

But as an example in the spdlog library, the constexpr checking doesn't work without this fix.
Therefore, it may be a good idea, to release a 5.1.1.

@vitaut
Copy link
Contributor

vitaut commented Aug 11, 2018

But as an example in the spdlog library, the constexpr checking doesn't work without this fix.

Could you elaborate why it doesn't work with spdlog? The main way of using constexpr checks is by wrapping the format string in the fmt macro which is unaffected by this issue.

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