Description

scoder

opened on Oct 20, 2017

Issue body actions

BPO	31828
Nosy	@ncoghlan, @scoder, @embray, @ma8ma
PRs	bpo-31828: make Py_tss_NEEDS_INIT usable in non-static initialisers #4060 No issue #: Fix wording about Py_tss_NEEDS_INIT in docs #4096

^{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 = None
created_at = <Date 2017-10-20.16:07:56.415>
labels = ['extension-modules', 'interpreter-core', 'type-feature', '3.7']
title = 'Support Py_tss_NEEDS_INIT outside of static initialisation'
updated_at = <Date 2017-10-26.15:52:49.298>
user = 'https://github.com/scoder'

bugs.python.org fields:

activity = <Date 2017-10-26.15:52:49.298>
actor = 'erik.bray'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Extension Modules', 'Interpreter Core']
creation = <Date 2017-10-20.16:07:56.415>
creator = 'scoder'
dependencies = []
files = []
hgrepos = []
issue_num = 31828
keywords = ['patch']
message_count = 4.0
messages = ['304661', '304697', '304757', '304778']
nosy_count = 4.0
nosy_names = ['ncoghlan', 'scoder', 'erik.bray', 'masamoto']
pr_nums = ['4060', '4096']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue31828'
versions = ['Python 3.7']

Activity

1. scoder commented on Oct 20, 2017

   

   scoder

   on Oct 20, 2017

   ContributorAuthor

   More actions

   Following up on bpo-25658, it was found that the current definition of Py_tss_NEEDS_INIT restricts its use to initialisers in C and cannot be used for arbitrary assignments. It is currently declared as follows:

       #define Py_tss_NEEDS_INIT   {0}

   which results in a C compiler error for assignments like "x = Py_tss_NEEDS_INIT".

   I proposed to change this to

       #define Py_tss_NEEDS_INIT   ((Py_tss_t) {0})

   in compliance with GCC and C99, but that fails to compile in MSVC and probably other old C++-ish compilers.

   I'm not sure how to improve this declaration, but given that it's a public header file, restricting its applicability seems really unfortunate.
2. scoder

   added

   3.7 (EOL)end of lifeend of life

   extension-modulesC modules in the Modules dirC modules in the Modules dir

   interpreter-core(Objects, Python, Grammar, and Parser dirs)(Objects, Python, Grammar, and Parser dirs)

   type-featureA feature request or enhancementA feature request or enhancement

   on Oct 20, 2017
3. ncoghlan commented on Oct 21, 2017

   

   ncoghlan

   on Oct 21, 2017

   Contributor

   More actions

   Right, the cases we were aiming to cover were:

   • C variable declarations ("static Py_tss_t tss_key = Py_tss_NEEDS_INIT;")
   • dynamic allocation with PyThread_tss_alloc
   • resetting a key back to the uninitialised state with PyThread_tss_delete

   The problem we have is that the second field in Py_tss_t is platform dependent, and not all platforms define a safe "unused" value for their NATIVE_TSS_KEY_T, which means Py_tss_NEEDS_INIT ends up being only a partial initialiser (earlier versions of the PEP used a field initialiser, but we were asked to switch it to a partial initialiser in order to support more compilers).

   We could offer a PyThread_tss_reset (to reset a key back to Py_tss_NEEDS_INIT), but that's confusingly similar to PyThread_tss_delete.

   Another option would be to check for typed partial initialiser support in the configure script, and declare Py_tss_NEEDS_INIT accordingly. However, that wouldn't solve the problem for any clients that are themselves also attempting to write cross-platform code.
4. ma8ma commented on Oct 22, 2017

   

   ma8mamannequin

   on Oct 22, 2017

   Mannequin

   More actions

   Or Py_tss_NEEDS_INIT is removed from the C API document to become a private feature; in other words, the Python interpreter will recommend that the static allocation for the TSS key shouldn't be used in the API client codes except for the interpreter core and built-in modules. (okay, I know it's an unfair suggestion)

   BTW, it's my mistake which the word "default value" for Py_tss_NEEDS_INIT, actually it makes more sense "initializer". I'd like to fix the document and the PEP, would I open the PR with this issue?
5. ncoghlan commented on Oct 23, 2017

   

   ncoghlan

   on Oct 23, 2017

   Contributor

   More actions

   For the wording fix, since it's just a trivial docs update, we can go ahead and fix it without an issue reference from the PRs.

   As far as the API goes, I do want to support "static Py_tss_t my_tss_key = Py_tss_NEEDS_INIT;" in third party extension modules, which is what the current API gives us.

   The question is whether there's a reasonable compiler independent way to also support struct *assignment*, in addition to initialisation.
6. ezio-melotti

   transferred this issue fromon Apr 10, 2022

Issue actions