Issue 25558: Use static asserts in C code (original) (raw)

Created on 2015-11-05 16:46 by serhiy.storchaka, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
use_Py_BUILD_ASSERT_EXPR.patch serhiy.storchaka,2015-11-05 16:46 review
macro.patch vstinner,2015-11-05 16:59 review
Messages (16)
msg254117 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) Date: 2015-11-05 16:59
Hum, maybe I wasn't clear: I propose attached macro.patch.
msg254126 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) 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) * (Python committer) 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) * (Python committer) Date: 2015-11-05 17:20
OK, I'll exclude Modules/_decimal/_decimal.c.
msg254130 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2015-11-05 17:22
Thank you!
msg254175 - (view) Author: Roundup Robot (python-dev) (Python triager) 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) * (Python committer) Date: 2015-11-06 09:25
Wrong issue. The correct one is .
msg254177 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-11-06 09:26
Oh, no, the correct one is .
msg254201 - (view) Author: STINNER Victor (vstinner) * (Python committer) 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) (Python triager) 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) * (Python committer) 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) * (Python committer) 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.)
History
Date User Action Args
2022-04-11 14:58:23 admin set github: 69744
2015-11-13 11:53:32 serhiy.storchaka set status: open -> closedresolution: fixed
2015-11-10 22:31:39 vstinner set messages: +
2015-11-07 14🔞53 serhiy.storchaka set messages: + stage: patch review -> resolved
2015-11-07 13:43:07 python-dev set messages: +
2015-11-06 15:36:57 vstinner set messages: +
2015-11-06 09:26:39 serhiy.storchaka set messages: +
2015-11-06 09:25:10 serhiy.storchaka set messages: +
2015-11-06 09:21:56 python-dev set nosy: + python-devmessages: +
2015-11-05 17:22:51 skrah set messages: +
2015-11-05 17:20:53 serhiy.storchaka set messages: +
2015-11-05 17🔞57 skrah set nosy: + skrahmessages: +
2015-11-05 17:03:07 serhiy.storchaka set messages: +
2015-11-05 16:59:37 vstinner set files: + macro.patchmessages: +
2015-11-05 16:57:23 vstinner set messages: +
2015-11-05 16:56:40 serhiy.storchaka set messages: +
2015-11-05 16:53:35 vstinner set messages: +
2015-11-05 16:46:40 serhiy.storchaka create