-
Notifications
You must be signed in to change notification settings - Fork 304
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
Silence spurious compiler warnings #3913
Conversation
using minor_tmp_buffer_type = std::conditional_t<GraphViewType::is_storage_transposed, | ||
edge_src_property_t<GraphViewType, T>, | ||
edge_dst_property_t<GraphViewType, T>>; | ||
std::unique_ptr<minor_tmp_buffer_type> minor_tmp_buffer{}; |
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.
Would an explicit std::optional<minor_tmp_buffer_type>
defined here and initialized to std::nullopt
also work? That's a code pattern we use quite a bit. Would that perhaps be a closer idiom to [[maybe_unused]]
until the warning issue is corrected in the compiler?
Approving either way, just wondering.
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.
It may work as well but std::optional is for run-time resolution. Here whether minor_tmp_buffer
is used or not can be determined in compile-time.
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.
Yeah... but your comment reminded me that I didn't add [[maybe_unused]] in this update. Adding this to minor_tmp_buffer
declaration.
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.
This pattern is also nice that it doesn't require if/else block.
/merge |
We see compiler warnings basically saying some internal data member in
std::optional
may be used uninitialized. This lengthy warnings are very annoying and can hide important warnings/errors.Seeing https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635, it seems like this warning is not valid and due to the fact that the compiler cannot perfectly track when an
std::optional
object is valid or not in every code path.Anyways, this PR tweaks the code to silence the warnings. There is very little practical difference between the old and new code, but I don't see the annoying warnings anymore with this update.
See the below for the actual warnings.