Use static asserts in C code

[serhiy-storchaka]

BPO	25558
Nosy	@vstinner, @skrah, @serhiy-storchaka
Files	use_Py_BUILD_ASSERT_EXPR.patch macro.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2015-11-13.11:53:32.017>
created_at = <Date 2015-11-05.16:46:40.575>
labels = ['extension-modules', 'interpreter-core', 'type-feature']
title = 'Use static asserts in C code'
updated_at = <Date 2015-11-13.11:53:32.016>
user = 'https://github.com/serhiy-storchaka'

bugs.python.org fields:

activity = <Date 2015-11-13.11:53:32.016>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = True
closed_date = <Date 2015-11-13.11:53:32.017>
closer = 'serhiy.storchaka'
components = ['Extension Modules', 'Interpreter Core']
creation = <Date 2015-11-05.16:46:40.575>
creator = 'serhiy.storchaka'
dependencies = []
files = ['40955', '40956']
hgrepos = []
issue_num = 25558
keywords = ['patch']
message_count = 16.0
messages = ['254117', '254120', '254123', '254124', '254125', '254126', '254128', '254129', '254130', '254175', '254176', '254177', '254201', '254269', '254270', '254465']
nosy_count = 4.0
nosy_names = ['vstinner', 'skrah', 'python-dev', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue25558'
versions = ['Python 3.6']

[serhiy-storchaka]

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

[vstinner]

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

[serhiy-storchaka]

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

[vstinner]

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

[vstinner]

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

[serhiy-storchaka]

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

[skrah]

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

[serhiy-storchaka]

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

[skrah]

Thank you!

[python-dev]

New changeset ad44d551c13c by Serhiy Storchaka in branch '3.5':
Issue bpo-25558: Refactoring OrderedDict iteration.
https://hg.python.org/cpython/rev/ad44d551c13c

New changeset 51f3547da99c by Serhiy Storchaka in branch 'default':
Issue bpo-25558: Refactoring OrderedDict iteration.
https://hg.python.org/cpython/rev/51f3547da99c

[serhiy-storchaka]

Wrong issue. The correct one is bpo-24726.

[serhiy-storchaka]

Oh, no, the correct one is bpo-25410.

[vstinner]

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-dev]

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

[serhiy-storchaka]

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.

[vstinner]

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

[namniav]

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.

If the compiler supports variable-length arrays(VLA) then it is unspecified whether or not the size expression is evaluated and no assertion can be checked at compile time.
See #118124 and https://en.cppreference.com/w/c/language/sizeof