Skip to content
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

PyCompilerFlags got a new cf_feature_version field #81434

Closed
vstinner opened this issue Jun 12, 2019 · 12 comments
Closed

PyCompilerFlags got a new cf_feature_version field #81434

vstinner opened this issue Jun 12, 2019 · 12 comments
Labels
3.8 only security fixes 3.9 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs)

Comments

@vstinner
Copy link
Member

BPO 37253
Nosy @gvanrossum, @vstinner, @miss-islington
PRs
  • bpo-37253: Add _PyCompilerFlags_INIT macro #14018
  • bpo-37253: Document PyCompilerFlags.cf_feature_version #14019
  • bpo-37253: Remove PyAST_obj2mod_ex() function #14020
  • [3.8] bpo-37253: Document PyCompilerFlags.cf_feature_version (GH-14019) #14035
  • bpo-37253: Fix typo in PyCompilerFlags doc #14036
  • [3.8] bpo-37253: Add _PyCompilerFlags_INIT macro (GH-14018) #14037
  • [3.8] bpo-37253: Document PyCompilerFlags.cf_feature_version (GH-14019) #14038
  • [3.8] bpo-37253: Remove PyAST_obj2mod_ex() function (GH-14020) #14044
  • 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 2019-06-13.12:17:00.286>
    created_at = <Date 2019-06-12.15:15:01.314>
    labels = ['interpreter-core', '3.8', '3.9']
    title = 'PyCompilerFlags got a new cf_feature_version field'
    updated_at = <Date 2019-06-13.12:17:00.285>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2019-06-13.12:17:00.285>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2019-06-13.12:17:00.286>
    closer = 'vstinner'
    components = ['Interpreter Core']
    creation = <Date 2019-06-12.15:15:01.314>
    creator = 'vstinner'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 37253
    keywords = ['patch']
    message_count = 12.0
    messages = ['345368', '345371', '345398', '345431', '345434', '345435', '345436', '345439', '345440', '345465', '345469', '345505']
    nosy_count = 3.0
    nosy_names = ['gvanrossum', 'vstinner', 'miss-islington']
    pr_nums = ['14018', '14019', '14020', '14035', '14036', '14037', '14038', '14044']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue37253'
    versions = ['Python 3.8', 'Python 3.9']

    @vstinner
    Copy link
    Member Author

    The commit dcfcd14 of bpo-35766 added a new cf_feature_version field to PyCompilerFlags. Each PyCompilerFlags variable must properly initialize this new field to PY_MINOR_VERSION. I propose to add a new _PyCompilerFlags_INIT macro to statically initialize such variable. I'm not sure if it should be public or not.

    The PyCompilerFlags structure is excluded from the stable ABI (PEP-384), but it's documented in the "The Very High Level Layer" C API documentation:
    https://docs.python.org/dev/c-api/veryhigh.html#c.PyCompilerFlags

    Structure fields are documented there:

    struct PyCompilerFlags {
        int cf_flags;
    }

    The doc is outdated. I'm not sure if it's on purpose or not.

    Moreover, the new PyCompilerFlags.cf_feature_version field is not documented in https://docs.python.org/dev/whatsnew/3.8.html#changes-in-the-c-api whereas C extensions using PyCompilerFlags should initialize cf_feature_version to PY_MINOR_VERSION?

    I'm not sure if C extensions really *must* initialize cf_feature_version, since the field is only used if PyCF_ONLY_AST flag is set in the cf_flags field.

    Something else, ast.parse() has been modified to use a (major, minor) version tuple rather an integer to specify the Python version in feature_version, but PyCompilerFlags still only uses the minor major. This API will be broken once the Python major version will be increased to 4, no? Would it make sense to use PY_VERSION_HEX format which includes the major version instead?

    Or cf_feature_version could be a structure with major and minor fields, but that might be overkill?

    @vstinner vstinner added 3.8 only security fixes 3.9 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Jun 12, 2019
    @vstinner
    Copy link
    Member Author

    Something else, ast.parse() has been modified to use a (major, minor) version tuple rather an integer to specify the Python version in feature_version, but PyCompilerFlags still only uses the minor major. This API will be broken once the Python major version will be increased to 4, no? Would it make sense to use PY_VERSION_HEX format which includes the major version instead?

    I can work on a PR to change cf_feature_version format, but I would prefer to agree here on what is the best format ;-)

    Note: compile() has a private keyword-only _feature_version which is also the Python minor version (int): don't include the major version. If we change cf_feature_version, we may also change compile(_feature_version=N) format.

    @gvanrossum
    Copy link
    Member

    It's fine to document the current state. I don't think you should spend any time *changing* the API to "future-proof" it.

    I am hoping that larger changes to the compiler implementation will happen before Python 4, which will make the whole API moot (including the "parser" module, which should be deprecated ASAP). The compiler is excluded from the ABI for a reason.

    @vstinner
    Copy link
    Member Author

    New changeset 2c9b498 by Victor Stinner in branch 'master':
    bpo-37253: Document PyCompilerFlags.cf_feature_version (GH-14019)
    2c9b498

    @vstinner
    Copy link
    Member Author

    New changeset 37d66d7 by Victor Stinner in branch 'master':
    bpo-37253: Add _PyCompilerFlags_INIT macro (GH-14018)
    37d66d7

    @vstinner
    Copy link
    Member Author

    New changeset a04ea4f by Victor Stinner in branch 'master':
    bpo-37253: Fix typo in PyCompilerFlags doc (GH-14036)
    a04ea4f

    @vstinner
    Copy link
    Member Author

    Many PyRun_xxx() functions of the public C API accept a "PyCompilerFlags* flags" parameter. So yeah, I documented the new cf_feature_version. PyCompilerFlags structure is kind of public ;-)

    @miss-islington
    Copy link
    Contributor

    New changeset 92e836c by Miss Islington (bot) in branch '3.8':
    bpo-37253: Add _PyCompilerFlags_INIT macro (GH-14018)
    92e836c

    @vstinner
    Copy link
    Member Author

    New changeset f9445a3 by Victor Stinner in branch '3.8':
    [3.8] bpo-37253: Document PyCompilerFlags.cf_feature_version (GH-14019) (GH-14038)
    f9445a3

    @vstinner
    Copy link
    Member Author

    New changeset 022ac0a by Victor Stinner in branch 'master':
    bpo-37253: Remove PyAST_obj2mod_ex() function (GH-14020)
    022ac0a

    @miss-islington
    Copy link
    Contributor

    New changeset 032bf30 by Miss Islington (bot) in branch '3.8':
    bpo-37253: Remove PyAST_obj2mod_ex() function (GH-14020)
    032bf30

    @vstinner
    Copy link
    Member Author

    It's fine to document the current state. I don't think you should spend any time *changing* the API to "future-proof" it.

    Ok.

    I am hoping that larger changes to the compiler implementation will happen before Python 4, which will make the whole API moot (including the "parser" module, which should be deprecated ASAP). The compiler is excluded from the ABI for a reason.

    Aha, that sounds exciting :-)

    I created bpo-37268 to propose to deprecate the parser module in Python 3.8.

    --

    PyCompilerFlags changes are now documented. I made the small code changes that I wanted to do in 3.8 and master. I close the issue.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.8 only security fixes 3.9 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs)
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants