-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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 conversion and other pedantic warnings #983
Conversation
With 8ea3c13, if we enable sign-conversion warnings as part of the FMT_PEDANTIC flag set then we don't get any more warnings in the headers. There are however several conversion warnings in the test code - but it is heavily templated so I'm not sure how to fix them. |
8ea3c13
to
e4edc84
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, just some minor comments inline. Thanks for working on this!
@@ -223,7 +223,7 @@ class basic_buffer { | |||
|
|||
protected: | |||
// Don't initialize ptr_ since it is not accessed to save a few cycles. | |||
basic_buffer(std::size_t sz) FMT_NOEXCEPT: size_(sz), capacity_(sz) {} | |||
basic_buffer(std::size_t sz) FMT_NOEXCEPT: ptr_(FMT_NULL), size_(sz), capacity_(sz) {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ptr_
is not initialized intentionally here. This has measurable performance impact on some benchmarks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK - as long as the header does not produce warnings when included in user-level code.
Superseded by #989 |
Supersedes #981 and #982 . Rebased onto current master.