Skip to content

MAINT: Correct code producing warnings #18432

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Feb 18, 2021
Merged

MAINT: Correct code producing warnings #18432

merged 2 commits into from
Feb 18, 2021

Conversation

bashtage
Copy link
Contributor

Cast to avoid warnings
Correct function

@bashtage
Copy link
Contributor Author

These produce warnings when building using clang-cl on Windows. Two are just casts. The other appears to be the wrong function for the input (labs -> llabs).

@bashtage
Copy link
Contributor Author

Failure is due to coverage, not being wrong.

mattip
mattip reviewed Feb 17, 2021
View reviewed changes
numpy/core/src/multiarray/ctors.c Outdated
@@ -2907,7 +2907,7 @@ _arange_safe_ceil_to_intp(double value)
"arange: cannot compute length");
return -1;
}
if (!(NPY_MIN_INTP <= ivalue && ivalue <= NPY_MAX_INTP)) {
if (!(NPY_MIN_INTP <= ivalue && ivalue <= (double)NPY_MAX_INTP)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is strange: shouldn't you need to cast both NPY_MIN_INTP and NPY_MAX_INTP?

@bashtage
Copy link
Contributor Author

It doesn't complain about NPY_MIN_INTP, just NPY_MAX_INTP. Could the double representation of NPY_MIN_INTP is exact?

2021-02-15T14:35:31.0278557Z numpy\core\src\multiarray\ctors.c(2910,47): warning: implicit conversion from 'long long' to 'double' changes value from 9223372036854775807 to 9223372036854775808 [-Wimplicit-const-int-float-conversion]
2021-02-15T14:35:31.0280755Z     if (!(NPY_MIN_INTP <= ivalue && ivalue <= NPY_MAX_INTP)) {
2021-02-15T14:35:31.0281923Z                                            ~~ ^~~~~~~~~~~~
2021-02-15T14:35:31.0283215Z numpy\core\include\numpy/npy_common.h(297,30): note: expanded from macro 'NPY_MAX_INTP'
2021-02-15T14:35:31.0284588Z         #define NPY_MAX_INTP NPY_MAX_LONGLONG
2021-02-15T14:35:31.0285689Z                              ^~~~~~~~~~~~~~~~
2021-02-15T14:35:31.0286967Z numpy\core\include\numpy/npy_common.h(637,28): note: expanded from macro 'NPY_MAX_LONGLONG'
2021-02-15T14:35:31.0288182Z #  define NPY_MAX_LONGLONG NPY_MAX_INT64
2021-02-15T14:35:31.0289226Z                            ^~~~~~~~~~~~~
2021-02-15T14:35:31.0290720Z numpy\core\include\numpy/npy_common.h(449,23): note: expanded from macro 'NPY_MAX_INT64'
2021-02-15T14:35:31.0291996Z #define NPY_MAX_INT64 NPY_LONGLONG_SUFFIX(9223372036854775807)
2021-02-15T14:35:31.0293243Z                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2021-02-15T14:35:31.0294506Z numpy\core\include\numpy/npy_common.h(344,39): note: expanded from macro 'NPY_LONGLONG_SUFFIX'
2021-02-15T14:35:31.0295780Z #    define NPY_LONGLONG_SUFFIX(x)   (x##i64)
2021-02-15T14:35:31.0296924Z                                       ^~~~~~
2021-02-15T14:35:31.0297733Z <scratch space>(126,1): note: expanded from here
2021-02-15T14:35:31.0298576Z 9223372036854775807i64
2021-02-15T14:35:31.0299225Z ^~~~~~~~~~~~~~~~~~~~~~

@bashtage
MAINT: Correct code producing warnings
e2467e4 
Cast to avoid warnings
Correct function
@mattip
Copy link
Member

mattip commented Feb 18, 2021

I think even if it does not currently emit a warning it would be better to pre-emptively add a cast to NPY_MIN_INTP as well.

@bashtage
Copy link
Contributor Author

Easy enough to add.

mattip
mattip approved these changes Feb 18, 2021
View reviewed changes
Copy link
Member

@mattip mattip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM once CI passes

@bashtage
Copy link
Contributor Author

@mattip all green except coverage delta which is unavoidable.

@mattip mattip merged commit 740a8cf into numpy:master Feb 18, 2021
@mattip
Copy link
Member

mattip commented Feb 18, 2021

Thanks @bashtage

@bashtage bashtage deleted the clean-c-warnings branch April 21, 2021 14:03
bashtage added a commit to bashtage/numpy that referenced this pull request May 4, 2021
@bashtage
CLN: Revert warning patch
72aafbd 
Revert changes for warnings awaiting numpy#18432
Small clean up of commented code
bashtage added a commit to bashtage/numpy that referenced this pull request Jun 2, 2021
@bashtage
CLN: Revert warning patch
023777c 
Revert changes for warnings awaiting numpy#18432
Small clean up of commented code
bashtage added a commit to bashtage/numpy that referenced this pull request Jun 8, 2021
@bashtage
CLN: Revert warning patch
216f0ed 
Revert changes for warnings awaiting numpy#18432
Small clean up of commented code
bashtage added a commit to bashtage/numpy that referenced this pull request Jul 1, 2021
@bashtage
CLN: Revert warning patch
d1169d3 
Revert changes for warnings awaiting numpy#18432
Small clean up of commented code
bashtage added a commit to bashtage/numpy that referenced this pull request Sep 2, 2021
@bashtage
CLN: Revert warning patch
8e56f7d 
Revert changes for warnings awaiting numpy#18432
Small clean up of commented code
bashtage added a commit to bashtage/numpy that referenced this pull request Nov 4, 2021
@bashtage
CLN: Revert warning patch
34e033b 
Revert changes for warnings awaiting numpy#18432
Small clean up of commented code
bashtage added a commit to bashtage/numpy that referenced this pull request Nov 9, 2021
@bashtage
ENH: Add clang-cl optimization
58305d7 
Add optimization
Revert changes for warnings awaiting numpy#18432
Small clean up of commented code
bashtage added a commit to bashtage/numpy that referenced this pull request Nov 9, 2021
@bashtage
ENH: Add clang-cl optimization
0147a89 
Add optimization
Revert changes for warnings awaiting numpy#18432
Small clean up of commented code
bashtage added a commit to bashtage/numpy that referenced this pull request Nov 11, 2021
@bashtage
ENH: Add clang-cl optimization
7bace9b 
Add optimization
Revert changes for warnings awaiting numpy#18432
Small clean up of commented code
bashtage added a commit to bashtage/numpy that referenced this pull request Nov 19, 2021
@bashtage
ENH: Add clang-cl optimization
f36eeef 
Add optimization
Revert changes for warnings awaiting numpy#18432
Small clean up of commented code
bashtage added a commit to bashtage/numpy that referenced this pull request Nov 30, 2021
@bashtage
ENH: Add clang-cl optimization
10f7726 
Add optimization
Revert changes for warnings awaiting numpy#18432
Small clean up of commented code
bashtage added a commit to bashtage/numpy that referenced this pull request Jan 20, 2022
@bashtage
ENH: Add clang-cl optimization
0aae3c6 
Add optimization
Revert changes for warnings awaiting numpy#18432
Small clean up of commented code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattip mattip mattip approved these changes

Assignees
No one assigned
Labels
03 - Maintenance
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bashtage @mattip