[issue25558] Use static asserts in C code

[Serhiy Storchaka]

New submission from Serhiy Storchaka:

Proposed patch converts some dynamic assert to static asserts (Py_BUILD_ASSERT_EXPR). This allows to check static invariants at compile time.

----------
components: Extension Modules, Interpreter Core
files: use_Py_BUILD_ASSERT_EXPR.patch
keywords: patch
messages: 254117
nosy: haypo, serhiy.storchaka
priority: normal
severity: normal
stage: patch review
status: open
title: Use static asserts in C code
type: enhancement
versions: Python 3.6
Added file: http://bugs.python.org/file40955/use_Py_BUILD_ASSERT_EXPR.patch

_______________________________________
Python tracker <***@bugs.python.org>
<http://bugs.python.org/issue25558>
_______________________________________

[STINNER Victor]

STINNER Victor added the comment:

+ (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?

----------

_______________________________________
Python tracker <***@bugs.python.org>
<http://bugs.python.org/issue25558>
_______________________________________

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

This is a public name and can be used in third-party code.

----------

_______________________________________
Python tracker <***@bugs.python.org>
<http://bugs.python.org/issue25558>
_______________________________________

[STINNER Victor]

Post by Serhiy Storchaka
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 :-)

----------

_______________________________________
Python tracker <***@bugs.python.org>
<http://bugs.python.org/issue25558>
_______________________________________

[STINNER Victor]

STINNER Victor added the comment:

Hum, maybe I wasn't clear: I propose attached macro.patch.

----------
Added file: http://bugs.python.org/file40956/macro.patch

_______________________________________
Python tracker <***@bugs.python.org>
<http://bugs.python.org/issue25558>
_______________________________________

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

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))

----------

_______________________________________
Python tracker <***@bugs.python.org>
<http://bugs.python.org/issue25558>
_______________________________________

[Stefan Krah]

Stefan Krah added the comment:

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 :).

----------
nosy: +skrah

_______________________________________
Python tracker <***@bugs.python.org>
<http://bugs.python.org/issue25558>
_______________________________________

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

OK, I'll exclude Modules/_decimal/_decimal.c.

----------

_______________________________________
Python tracker <***@bugs.python.org>
<http://bugs.python.org/issue25558>
_______________________________________

[Stefan Krah]

Stefan Krah added the comment:

Thank you!

----------

_______________________________________
Python tracker <***@bugs.python.org>
<http://bugs.python.org/issue25558>
_______________________________________

[Roundup Robot]

Roundup Robot added the comment:

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

----------
nosy: +python-dev

_______________________________________
Python tracker <***@bugs.python.org>
<http://bugs.python.org/issue25558>
_______________________________________

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Wrong issue. The correct one is issue24726.

----------

_______________________________________
Python tracker <***@bugs.python.org>
<http://bugs.python.org/issue25558>
_______________________________________

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Oh, no, the correct one is issue25410.

----------

_______________________________________
Python tracker <***@bugs.python.org>
<http://bugs.python.org/issue25558>
_______________________________________

[STINNER Victor]

STINNER Victor added the comment:

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 :-)

----------

_______________________________________
Python tracker <***@bugs.python.org>
<http://bugs.python.org/issue25558>
_______________________________________

[Roundup Robot]

Roundup Robot added the comment:

New changeset 45a404d33c2d by Serhiy Storchaka in branch 'default':
Issue #25558: Use compile-time asserts.
https://hg.python.org/cpython/rev/45a404d33c2d

----------

_______________________________________
Python tracker <***@bugs.python.org>
<http://bugs.python.org/issue25558>
_______________________________________

[Serhiy Storchaka]

Post by STINNER Victor
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.

Post by STINNER Victor
Maybe we could use __builtin_constant_p() on GCC?

Don't know if it will help.

----------
stage: patch review -> resolved

_______________________________________
Python tracker <***@bugs.python.org>
<http://bugs.python.org/issue25558>
_______________________________________

[STINNER Victor]

STINNER Victor added the comment:

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.)

----------

_______________________________________
Python tracker <***@bugs.python.org>
<http://bugs.python.org/issue25558>
_______________________________________