Skip to content

Reorganize the re module sources #91308

Description

@serhiy-storchaka
BPO 47152
Nosy @gvanrossum, @vstinner, @ezio-melotti, @serhiy-storchaka, @animalize, @asottile
PRs
  • bpo-47152: Convert the re module into a package #32177
  • [WIP] bpo-23689: re module, fix memory leak when a match is terminated by a signal #32188
  • bpo-47152: Move sources of the _sre module into a subdirectory #32290
  • bpo-47152: Remove unused import in re #32298
  • 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 2022-03-29.15:54:32.145>
    labels = ['expert-regex', 'library', '3.11']
    title = 'Reorganize the re module sources'
    updated_at = <Date 2022-04-05.08:10:39.512>
    user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2022-04-05.08:10:39.512>
    actor = 'serhiy.storchaka'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Library (Lib)', 'Regular Expressions']
    creation = <Date 2022-03-29.15:54:32.145>
    creator = 'serhiy.storchaka'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 47152
    keywords = ['patch']
    message_count = 26.0
    messages = ['416268', '416294', '416320', '416328', '416471', '416497', '416502', '416523', '416543', '416545', '416547', '416548', '416551', '416557', '416563', '416591', '416593', '416595', '416615', '416657', '416659', '416667', '416676', '416693', '416747', '416761']
    nosy_count = 8.0
    nosy_names = ['gvanrossum', 'vstinner', 'ezio.melotti', 'mrabarnett', 'serhiy.storchaka', 'malin', 'Anthony Sottile', 'dom1310df']
    pr_nums = ['32177', '32188', '32290', '32298']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue47152'
    versions = ['Python 3.11']

    Activity

    1. serhiy-storchaka commented on Mar 29, 2022

      @serhiy-storchaka
      MemberAuthor

      I proposed it several years ago on the Python-Dev mailing list and that change was approved in general. The reorganization was deferred because there were several known bugs in the RE engine (fixes for which could potentially be backported) and there were not merged patches waiting for review. Now the patch for atomic groups was merged and bugs was fixed (thanks to Ma Lin).

      Both the C code and the Python code for the re module are distributed on few files, which lie down in directories Modules and Lib. It makes difficult to work with all related files because they are intermixed with source files of different modules.

      The following changes are planned:

      1. Convert the re module into a package. Make sre_* modules its submodules.
      2. Move C sources for the _sre module into a separate directory.
      3. Extract the code for generating definitions of C constants from definitions of Python constants into a separate script and add it in the Tools/scripts directory (there are precedences: generate_token.py, etc).
    2. dom1310df commented on Mar 29, 2022

      dom1310dfmannequin
      Mannequin

      Could the sre_parse and sre_constants modules be kept with public names (i.e. without the leading underscore) but within the re namespace? I use them to tokenize and then syntax highlight regular expressions.

      I did a quick search and found a few other users of the modules:

      • pydoctor uses them for regex syntax highlighting[1], although it has its own copy of the sre_parse source rather than importing from stdlib.
      • lark uses sre_parse to find minimum and maximum length of matching strings[2]
      • sre_yield uses them to determine all strings that will match a regex[3]

      The whole modules don't necessarily need exposing, but certainly sre_parse.parse, sre_parse.parse_template, and the opcodes from sre_constants would be the most useful.

      [1] https://github.com/twisted/pydoctor/blob/c86273dffade5455890570142c8b7b068f5dffd1/pydoctor/epydoc/markup/_pyval_repr.py#L776
      [2] https://github.com/lark-parser/lark/blob/85ea92ebf4e983e9997f9953a9c1463bb3d1c6cc/lark/utils.py#L120
      [3] https://github.com/google/sre_yield/blob/3af063a0054c4646608b43b941fbfcbe4e01214a/sre_yield/__init__.py

    3. animalize commented on Mar 30, 2022

      animalizemannequin
      Mannequin

      Please don't merge too close to the 3.11 beta1 release date, I'll submit PRs after this merged.

    4. serhiy-storchaka commented on Mar 30, 2022

      @serhiy-storchaka
      MemberAuthor

      It turns out that pip uses sre_constants in its copy of pyparsing. The problem is already fixed in the upstream of pyparsing and soon should be fixed in pip. We still need to keep sre_constants and maybe other sre_* modules, but deprecate them.

      Could the sre_parse and sre_constants modules be kept with public names (i.e. without the leading underscore) but within the re namespace?

      It is a good idea which will allow to minimize breakage in short term. You can write "from re import sre_parse", and it would work in old and new versions because sre_parse and sre_compile were imported in the re module. This trick does not work with sre_constants, you still need try/except.

      But the code that depends on these modules is fragile and can be broken by other ways.

      Please don't merge too close to the 3.11 beta1 release date, I'll submit PRs after this merged.

      I am going to implement step 2 only after merging your changes for bpo-23689.

    5. vstinner commented on Apr 1, 2022

      @vstinner
      Member

      sre_constants, sre_compile and sre_parse are not tested and are not documented. I don't consider them as public API currently.

      If someone has good reason to use them, IMO we must clearly define which exact API is needed, properly document and test it.

      If we expose something, I don't think that the API would be exposed as re.sre_xxx.xxx, but as re.xxx.

      I suggest to hide sre_xxx submodules by adding an underscore to their name. Moreover, the "sre_" prefix is now redundant. I suggest renaming:

      • sre_constants => re._constants
      • sre_compile => re._compile
      • sre_parse => re._parse
    6. gvanrossum commented on Apr 1, 2022

      @gvanrossum
      Member

      I don't mind reorganizing this, but I would insist that we keep code using old undocumented things (like the sre_* modules) working for several releases, using the standard deprecation approach.

    7. serhiy-storchaka commented on Apr 1, 2022

      @serhiy-storchaka
      MemberAuthor

      Modules with old names are kept (deprecated). The questions are:

      1. Should we keep the sre_ prefix in new submodules? Should we prefix them with underscores?
      2. Should we keep only non-underscored names in the sre_* modules or undescored names too?
    8. gvanrossum commented on Apr 1, 2022

      @gvanrossum
      Member
      1. If we're reorganizing anyway, I see no reason to keep the old names.
      2. For maximum backwards compatibility, I'd say keep as much as you can, as long as keeping it won't interfere with the reorganization.
    9. serhiy-storchaka commented on Apr 2, 2022

      @serhiy-storchaka
      MemberAuthor

      New changeset 1be3260 by Serhiy Storchaka in branch 'main':
      bpo-47152: Convert the re module into a package (GH-32177)
      1be3260

    10. 42 remaining items

    11. added a commit that references this issue on Feb 22, 2023
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      3.11only security fixesstdlibStandard Library Python modules in the Lib/ directorytopic-regex

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions