-
-
Notifications
You must be signed in to change notification settings - Fork 7.4k
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 compilation warning with GCC #14470
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #14470 +/- ##
=======================================
Coverage 98.39% 98.39%
=======================================
Files 161 161
Lines 21204 21204
=======================================
Hits 20864 20864
Misses 340 340 ☔ View full report in Codecov by Sentry. |
Code size report:
|
Hi, your first few commits looked like good cleanups of compiler warnings thanks! |
You are right. I've made an error during the pull request creation. The three commits that start with "yoctopuce specific" must be ignored. Sorry for this mistake. Do you want that I create a clean new pull request? |
It shouldn't need a new pull request, you could start a new branch for the extra commits, then reset this branch to the commits meant to share. |
Signed-off-by: Yoctopuce <dev@yoctopuce.com>
Signed-off-by: Yoctopuce <dev@yoctopuce.com>
Signed-off-by: Yoctopuce <dev@yoctopuce.com>
Signed-off-by: Yoctopuce <dev@yoctopuce.com>
That should be right now. Sorry for the extra work. I'm not use to Git. |
Signed-off-by: Yoctopuce <dev@yoctopuce.com>
#define mp_const_float_inf MP_ROM_PTR((mp_obj_t)(((0x7f800000 & ~3) | 2) + 0x80800000)) | ||
#define mp_const_float_nan MP_ROM_PTR((mp_obj_t)(((0xffc00000 & ~3) | 2) + 0x80800000)) | ||
#define mp_const_float_inf MP_ROM_PTR((mp_obj_t)(((0x7f800000 & ~3) | 2) + 0x80800000ull)) | ||
#define mp_const_float_nan MP_ROM_PTR((mp_obj_t)(((0xffc00000 & ~3) | 2) + 0x80800000ull)) |
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.
What exactly is the error/warning that's lead to this change? And why isn't the change also needed for tau
, e
and pi
constants?
Is appending a u
enough, or maybe just ul
?
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.
Actually the warning does not come form gcc but from Microsoft cl compiler.
The exact output is:
../../../mpy/micropython/py/modmath.c(376): warning C4307: '+': integral constant overflow
../../../mpy/micropython/py/modmath.c(377): warning C4307: '+': integral constant overflow
We have fix only these two line because they are the ones that generate the warnings, but
It may be a good idea to update all the definitions.
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.
I guess it's because these constants start with 0x7f
and 0xff
, but the others start with 0x40
.
Is it possible to use u
or ul
instead of ull
?
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.
Nop. Only ull
will work.
Which is the way to specify an unsigned 64 bit integer.
https://gcc.gnu.org/onlinedocs/gcc/Long-Long.html
No description provided.