-
Notifications
You must be signed in to change notification settings - Fork 69
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 warnings (shadow and conversion) #340
Conversation
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.
Thanks for the fixes! : - )
@@ -651,15 +651,15 @@ constexpr void validate_strides(with_rank<N>, Layout, const Extents& ext, const | |||
|
|||
constexpr auto is_left = std::is_same<Layout, layout_left>::value; | |||
|
|||
typename Extents::index_type stride = 1; | |||
typename Extents::index_type stride_ = 1; |
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.
Should we consider avoiding use of trailing underscore for variables that aren't nonpublic data members of the class?
typename Extents::index_type stride_ = 1; | |
typename Extents::index_type the_stride = 1; |
and "invalid strides for layout_{left,right}"); | ||
|
||
stride *= ext.extent(s); | ||
stride_ *= ext.extent(s); |
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.
stride_ *= ext.extent(s); | |
the_stride *= ext.extent(s); |
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.
renamed to expected_stride
b59a26d
to
6a7bcc6
Compare
I don't think our CI is actually testing Wshadow |
Looks like the GitHub Workflows don't test with any warning flags and https://github.com/kokkos/mdspan/blob/stable/README.md#running-tests mentions that a lot of the configurations would not build with warnings as errors (depending on the C++ standard). |
I know. But we can touch CI separately. |
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 good to me!
The extent and stride thing shadow something new in utility.hpp