Skip to content

MAINT: Correct code producing warnings#18432

Merged
mattip merged 2 commits intonumpy:masterfrom
bashtage:clean-c-warnings
Feb 18, 2021
Merged

MAINT: Correct code producing warnings#18432
mattip merged 2 commits intonumpy:masterfrom
bashtage:clean-c-warnings

Conversation

@bashtage
Copy link
Copy Markdown
Contributor

@bashtage bashtage commented Feb 17, 2021

Cast to avoid warnings
Correct function

@bashtage
Copy link
Copy Markdown
Contributor Author

bashtage commented Feb 17, 2021

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
Copy Markdown
Contributor Author

bashtage commented Feb 17, 2021

Failure is due to coverage, not being wrong.

mattip
mattip reviewed Feb 17, 2021
View reviewed changes
numpy/core/src/multiarray/ctors.c Outdated
Copy link
Copy Markdown
Member

@mattip mattip Feb 17, 2021

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
Copy Markdown
Contributor Author

bashtage commented Feb 17, 2021

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
Copy Markdown
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
Copy Markdown
Contributor Author

bashtage commented Feb 18, 2021

Easy enough to add.

mattip
mattip approved these changes Feb 18, 2021
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

bashtage commented Feb 18, 2021

@mattip all green except coverage delta which is unavoidable.

@mattip mattip merged commit 740a8cf into numpy:master Feb 18, 2021
@mattip
Copy link
Copy Markdown
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