msg254117 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2015-11-05 16:46 |
Proposed patch converts some dynamic assert to static asserts (Py_BUILD_ASSERT_EXPR). This allows to check static invariants at compile time. |
|
|
msg254120 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2015-11-05 16:53 |
+ (void)Py_BUILD_ASSERT_EXPR(INT_MAX <= _PyTime_MAX / SEC_TO_NS); Hum, maybe the existing macro should be renamed to Py_BUILD_ASSERT_EXPR and a new Py_BUILD_ASSERT_EXPR macro should add the (void) to ignore the result? It would avoid to have to repeat (void) everywhere. What do you think? |
|
|
msg254123 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2015-11-05 16:56 |
This is a public name and can be used in third-party code. |
|
|
msg254124 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2015-11-05 16:57 |
> This is a public name and can be used in third-party code. Do you mean that a library can really rely on the result!? It would be insane :-) |
|
|
msg254125 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2015-11-05 16:59 |
Hum, maybe I wasn't clear: I propose attached macro.patch. |
|
|
msg254126 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2015-11-05 17:03 |
A library can follow the example in the comment. #define foo_to_char(foo) \ ((char *)(foo) \ + Py_BUILD_ASSERT_EXPR(offsetof(struct foo, string) == 0)) |
|
|
msg254128 - (view) |
Author: Stefan Krah (skrah) *  |
Date: 2015-11-05 17:18 |
Serhiy, could you please not change stuff that I maintain? I know you have the best intentions, but I really don't like these kinds of changes (just like you don't like trailing whitespace :). |
|
|
msg254129 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2015-11-05 17:20 |
OK, I'll exclude Modules/_decimal/_decimal.c. |
|
|
msg254130 - (view) |
Author: Stefan Krah (skrah) *  |
Date: 2015-11-05 17:22 |
Thank you! |
|
|
msg254175 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2015-11-06 09:21 |
New changeset ad44d551c13c by Serhiy Storchaka in branch '3.5': Issue #25558: Refactoring OrderedDict iteration. https://hg.python.org/cpython/rev/ad44d551c13c New changeset 51f3547da99c by Serhiy Storchaka in branch 'default': Issue #25558: Refactoring OrderedDict iteration. https://hg.python.org/cpython/rev/51f3547da99c |
|
|
msg254176 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2015-11-06 09:25 |
Wrong issue. The correct one is . |
|
|
msg254177 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2015-11-06 09:26 |
Oh, no, the correct one is . |
|
|
msg254201 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2015-11-06 15:36 |
use_Py_BUILD_ASSERT_EXPR.patch looks good to me. But you should revert the change on decimal, as asked by Stefan, and I suggested to move an assertion inside the related function (see my comment on Rietveld). """ A library can follow the example in the comment. #define foo_to_char(foo) \ ((char *)(foo) \ + Py_BUILD_ASSERT_EXPR(offsetof(struct foo, string) == 0)) """ Hum ok, I know understand the "_EXPR" suffix of the macro name. Maybe it's worth to add a new #define Py_BUILD_ASSERT(expr) (void)Py_BUILD_ASSERT_EXPR(expr)" macro? By the way, I don't know what happens if you pass a variable to Py_BUILD_ASSERT_EXPR() rather than a constant. Maybe we could use __builtin_constant_p() on GCC? Maybe it's overcomplexicated :-) |
|
|
msg254269 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2015-11-07 13:43 |
New changeset 45a404d33c2d by Serhiy Storchaka in branch 'default': Issue #25558: Use compile-time asserts. https://hg.python.org/cpython/rev/45a404d33c2d |
|
|
msg254270 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2015-11-07 14:18 |
> By the way, I don't know what happens if you pass a variable to Py_BUILD_ASSERT_EXPR() rather than a constant. If the compiler can't calculate it at compile time (e.g. int_var <= INT_MAX), your are out of luck. > Maybe we could use __builtin_constant_p() on GCC? Don't know if it will help. |
|
|
msg254465 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2015-11-10 22:31 |
This issue can now be closed, no? (I don't think that it's worth to add a new macro and make the existing macro more strict.) |
|
|