This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author congma
Recipients congma
Date 2021-03-24.13:59:18
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1616594359.79.0.245413708193.issue43615@roundup.psfhosted.org>
In-reply-to
Content
(This is a summarized form of the commit message in the attached patch. I'm submitting a patch instead of a PR over GitHub, because it seems that the ``autoreconf`` output files are part of the repository. In order for the changes to take effect in the repo, I may have to run ``autoreconf`` and add the clobbered output files to the repo, which I don't think is a good idea. Also on my system the ``autoreconf`` can only work correctly if I add a missing M4 file "ax_check_compile_flag.m4" from the Autoconf Archive <https://www.gnu.org/software/autoconf-archive/ax_check_compile_flag.html> for the ``AX_CHECK_COMPILE_FLAG`` macro used in the existing ``configure.ac``. I don't think it's wise for me to introduce so many changes at once if most developers don't need to run ``autoreconf`` often.)

The problem
-----------

Definition of the ``Py_UNREACHABLE()`` macro relied on testing compiler versions in preprocessor directives. This is unreliable chiefly because compilers masquerade as each other.

The current implementation tests the ``__GNUC__`` and ``__GNUC_MINOR__`` macros as the logic (GCC version >= 4.5) for determining whether the compiler intrinsic ``__builtin_unreachable()`` is present (see commits eebaa9bf, 24ba3b0d). However, Clang defines these macros too and can cause confusion. Clang 11 pretends to be GCC 4.2.1 in its predefined macros. As a result, Clang won't use the intrinsic even if it's supported. This doesn't seem to match the intent behind the original implementation.

The solution
------------

Test the presence of the compiler-builtin ``__builtin_unreachable()`` at configure-time using Autoconf, and conditionally define the ``Py_UNREACHABLE()`` macro depending on the configuration.

The idea is based on the ``ax_gcc_builtin.m4`` code [0] by Gabriele Svelto.

Alternative ideas
-----------------

Recent versions of Clang and GCC support the ``__has_builtin()`` macro.
However, this may be unreliable before Clang 10 [1], while GCC support is only available as of GCC 10 and its semantics may not be the same as Clang's [2]. Therefore ``__has_builtin()`` may not be as useful as it seems.

We may attempt to improve the accuracy of version checking in ``#if`` directives, but this could be brittle and difficult to explain, verify, or maintain.

Links
-----

[0] https://www.gnu.org/software/autoconf-archive/ax_gcc_builtin.html
[1] https://clang.llvm.org/docs/LanguageExtensions.html#has-builtin
[2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66970#c24
History
Date User Action Args
2021-03-24 13:59:19congmasetrecipients: + congma
2021-03-24 13:59:19congmasetmessageid: <1616594359.79.0.245413708193.issue43615@roundup.psfhosted.org>
2021-03-24 13:59:19congmalinkissue43615 messages
2021-03-24 13:59:18congmacreate