New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-39487: Merge duplicated _Py_IDENTIFIER identifiers in C code #18254

Open

shihai1991 wants to merge 1 commit into python:master from shihai1991:bpo_39487

+24 −33

Contributor

shihai1991 commented Jan 29, 2020 •

edited by bedevere-bot

https://bugs.python.org/issue39487


        Merge duplicated _Py_IDENTIFIER identifiers in C code

ca422bc

shihai1991 requested review from brettcannon, encukou, ericsnowcurrently, ncoghlan and warsaw as code owners

the-knights-who-say-ni added the CLA signed label

bedevere-bot added the awaiting review label

brettcannon added the skip news label

brettcannon approved these changes

bedevere-bot added awaiting merge and removed awaiting review labels

brettcannon requested changes

Python/import.c

  
      @@ -1568,7 +1570,6 @@ resolve_name(PyThreadState *tstate, PyObject *name, PyObject *globals, int level
    
        {

            _Py_IDENTIFIER(__spec__);

brettcannon Jan 29, 2020

Member

Suggested change

      
                 _Py_IDENTIFIER(__spec__);

bedevere-bot commented Jan 29, 2020

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

bedevere-bot added awaiting changes and removed awaiting merge labels

Contributor

ncoghlan commented Jan 29, 2020

We can't make this change, as it means the statics get initialised before the Python interpreter has been initialised, and won't be reinitialised if the interpreter is destroyed and recreated.

ncoghlan closed this

ncoghlan reopened this

Contributor

ncoghlan commented Jan 29, 2020

My apologies, my comment above was based on an outdated understanding of how the identifier structs get initialised (it's the usage that initialises them, not the declaration).

That means this is a useful refactoring to help identify blockers to full subinterpreter support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

You can’t perform that action at this time.