-
Notifications
You must be signed in to change notification settings - Fork 191
mrgrid, mrtransform -template: warn if stride mismatch #3186
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
base: dev
Are you sure you want to change the base?
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.
clang-tidy made some suggestions
|
|
||
| Header output_header(regrid_filter); | ||
| Stride::set_from_command_line(output_header); | ||
| if (get_options("template").size() && !get_options("strides").size()) { |
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.
warning: implicit conversion 'size_type' (aka 'unsigned long') -> bool [readability-implicit-bool-conversion]
| if (get_options("template").size() && !get_options("strides").size()) { | |
| if ((get_options("template").size() != 0u) && !get_options("strides").size()) { |
|
|
||
| Header output_header(regrid_filter); | ||
| Stride::set_from_command_line(output_header); | ||
| if (get_options("template").size() && !get_options("strides").size()) { |
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.
warning: implicit conversion 'size_type' (aka 'unsigned long') -> bool [readability-implicit-bool-conversion]
if (get_options("template").size() && !get_options("strides").size()) {
^this fix will not be applied because it overlaps with another fix
|
|
||
| Header output_header(regrid_filter); | ||
| Stride::set_from_command_line(output_header); | ||
| if (get_options("template").size() && !get_options("strides").size()) { |
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.
warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
| if (get_options("template").size() && !get_options("strides").size()) { | |
| if (!get_options("template").empty() && !get_options("strides").size()) { |
Additional context
/usr/include/c++/13/bits/stl_vector.h:1087: method 'vector'::empty() defined here
empty() const _GLIBCXX_NOEXCEPT
^|
|
||
| Header output_header(regrid_filter); | ||
| Stride::set_from_command_line(output_header); | ||
| if (get_options("template").size() && !get_options("strides").size()) { |
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.
warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
| if (get_options("template").size() && !get_options("strides").size()) { | |
| if (get_options("template").size() && get_options("strides").empty()) { |
Additional context
/usr/include/c++/13/bits/stl_vector.h:1087: method 'vector'::empty() defined here
empty() const _GLIBCXX_NOEXCEPT
^| output_header.transform() = template_header.transform(); | ||
| } | ||
|
|
||
| if (!get_options("strides").size()) { |
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.
warning: implicit conversion 'size_type' (aka 'unsigned long') -> bool [readability-implicit-bool-conversion]
if (!get_options("strides").size()) {
^this fix will not be applied because it overlaps with another fix
| output_header.transform() = template_header.transform(); | ||
| } | ||
|
|
||
| if (!get_options("strides").size()) { |
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.
warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
| if (!get_options("strides").size()) { | |
| if (get_options("strides").empty()) { |
Additional context
/usr/include/c++/13/bits/stl_vector.h:1087: method 'vector'::empty() defined here
empty() const _GLIBCXX_NOEXCEPT
^
Replacement of #2460. The merge conflicts claimed there were larger in magnitude than the change itself---how much of that was due to #3167 I'm not sure---so I instead re-applied these changes manually.
Would benefit from tests exemplifying the use cases involved to make sure that the warning appears when it should and doesn't when it shouldn't.