Skip to content

Derby: Convert the _sre module to use Argument Clinic #64347

Closed
Closed
Derby: Convert the _sre module to use Argument Clinic#64347
Assignees
serhiy-storchaka
Labels
extension-modulesC modules in the Modules dirtopic-argument-clinictopic-regextype-featureA feature request or enhancement

Description

@serhiy-storchaka
serhiy-storchaka
opened on Jan 6, 2014
BPO 20148
Nosy @larryhastings, @ezio-melotti, @serhiy-storchaka, @zooba
Dependencies
  • bpo-20141: Argument Clinic: broken support for 'O!'
  • bpo-20144: Argument Clinic doesn't support named constants as default values
  • bpo-20161: inspect.signature fails on some functions which use Argument Clinic
  • bpo-20283: Wrong keyword parameter name in regex pattern methods
    		•
    Files
  • sre_clinic.patch
  • sre_clinic_2.patch
  • sre_clinic_3.patch
  • sre_clinic_4.patch
  • sre_clinic_5.patch
  • larry.fix.sre.build.on.windows.diff.1.txt
    		•

    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 = 'https://github.com/serhiy-storchaka'
closed_at = <Date 2015-05-04.12:47:41.247>
created_at = <Date 2014-01-06.16:04:28.868>
labels = ['extension-modules', 'expert-regex', 'type-feature', 'expert-argument-clinic']
title = 'Derby: Convert the _sre module to use Argument Clinic'
updated_at = <Date 2015-05-04.12:47:41.246>
user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2015-05-04.12:47:41.246>
actor = 'serhiy.storchaka'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2015-05-04.12:47:41.247>
closer = 'serhiy.storchaka'
components = ['Extension Modules', 'Regular Expressions', 'Argument Clinic']
creation = <Date 2014-01-06.16:04:28.868>
creator = 'serhiy.storchaka'
dependencies = ['20141', '20144', '20161', '20283']
files = ['33527', '34529', '35438', '37335', '39092', '39281']
hgrepos = []
issue_num = 20148
keywords = ['patch']
message_count = 29.0
messages = ['207448', '207450', '207452', '207594', '207627', '207646', '208397', '214194', '231939', '241359', '242470', '242502', '242505', '242506', '242508', '242509', '242510', '242511', '242512', '242513', '242514', '242518', '242519', '242520', '242521', '242522', '242529', '242531', '242532']
nosy_count = 6.0
nosy_names = ['larry', 'ezio.melotti', 'mrabarnett', 'python-dev', 'serhiy.storchaka', 'steve.dower']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue20148'
versions = ['Python 3.5']

    Activity

    1. serhiy-storchaka commented on Jan 6, 2014

      @serhiy-storchaka
      serhiy-storchaka
      on Jan 6, 2014
      MemberAuthor

      Here is preliminary patch which converts the _sre module to use Argument Clinic. 22 functions are converted.

      There are thre problems:

      • bpo-20141. The code was manually edited after Argument Clinic (and should be edited again for next run of Argument Clinic).

      • bpo-20144. Literal 1000000000 is temporary used instead of PY_SSIZE_T_MAX.

      • Pydoc for _sre.compile (and only for this function) returns:

        _sre.compile =

    2. serhiy-storchaka
      added
      topic-regex
      extension-modulesC modules in the Modules dir
      type-featureA feature request or enhancement
      on Jan 6, 2014

    3. larryhastings commented on Jan 6, 2014

      @larryhastings
      larryhastings
      on Jan 6, 2014
      Contributor

      Obviously we can't live with manually editing the output from Argument Clinic, so I'll get you a fix for O! today.

      Maybe we could use a better literal value? Like 2**31 - 1?

      I don't understand the pydoc thing. Can you elaborate?

    4. serhiy-storchaka commented on Jan 6, 2014

      @serhiy-storchaka
      serhiy-storchaka
      on Jan 6, 2014
      MemberAuthor

      Maybe we could use a better literal value? Like 2**31 - 1?

      This only a placeholder. It will be replaced by named constant in final patch.
      2**31 - 1 is not better here than any other arbitrary value.

      I don't understand the pydoc thing. Can you elaborate?

      Pydoc for _sre.compile doesn't output signature and docstring, but only
      mentioned above line. There is similar issue in audioop module (bpo-20133)
      for the ratecv function.

    5. larryhastings commented on Jan 7, 2014

      @larryhastings
      larryhastings
      on Jan 7, 2014
      Contributor

      Can you refresh the patch? I think all the problems you cited are fixed, and also the comments Argument Clinic uses were all changed. I'll review when you have a fresh patch.

    6. larryhastings
      changed the title [-]Convert the _sre module to use Argument Clinic[/-] [+]Derby: Convert the _sre module to use Argument Clinic[/+] on Jan 7, 2014

    7. serhiy-storchaka commented on Jan 7, 2014

      @serhiy-storchaka
      serhiy-storchaka
      on Jan 7, 2014
      MemberAuthor

      Here is refreshed patch.

    8. larryhastings commented on Jan 8, 2014

      @larryhastings
      larryhastings
      on Jan 8, 2014
      Contributor

      Serhiy: Assigning to you because you wrote a patch; if you don't want the issue, sorry, please undo it.

    9. larryhastings
      assigned
      serhiy-storchaka
      on Jan 8, 2014

    10. serhiy-storchaka commented on Jan 18, 2014

      @serhiy-storchaka
      serhiy-storchaka
      on Jan 18, 2014
      MemberAuthor

      Now all methods except Match.group() (which needs *args) use Argument Clinic.

      Most important change is that first parameters of some Pattern methods are renamed from "pattern" or "source" to "string". This was obvious bug (bpo-20283). "string" conforms to the documentation and to the name of the Match.string attribute.

    11. serhiy-storchaka commented on Mar 20, 2014

      @serhiy-storchaka
      serhiy-storchaka
      on Mar 20, 2014
      MemberAuthor

      Here is updated patch.

    12. serhiy-storchaka commented on Dec 1, 2014

      @serhiy-storchaka
      serhiy-storchaka
      on Dec 1, 2014
      MemberAuthor

      Synchronized with the tip again.

    13. serhiy-storchaka
      added
      topic-argument-clinic
      on Feb 25, 2015

    14. 9 remaining items

    15. larryhastings commented on May 3, 2015

      @larryhastings
      larryhastings
      on May 3, 2015
      Contributor

      Sorry--you can simply remove that line, it's no longer needed.

    16. serhiy-storchaka commented on May 3, 2015

      @serhiy-storchaka
      serhiy-storchaka
      on May 3, 2015
      MemberAuthor

      Does your patch just move type definition to the end of the file? I think this is only the way to make happy GCC and MS compiler.

      Strange that no IRC bot complained about compilation failure.

    17. larryhastings commented on May 3, 2015

      @larryhastings
      larryhastings
      on May 3, 2015
      Contributor

      Yes, it moves the type declaration to the bottom of the file, and adds forward static declarations for the types to the top of the file.

    18. zooba commented on May 3, 2015

      @zooba
      zooba
      on May 3, 2015
      Member

      AFAIK the only buildbot with a nearly-up-to-date MSVC is still marked as unstable (http://buildbot.python.org/all/builders/AMD64%20Windows8%203.x). I keep meaning to get it promoted, but haven't quite gotten around to it yet...

    19. larryhastings commented on May 3, 2015

      @larryhastings
      larryhastings
      on May 3, 2015
      Contributor

      Oh, this is only happening on the *beta* compiler? In that case, I genuinely suggest you file a bug.

      We can still check in the workaround, but I really do think MSVS's behavior is wrong here. (Why is it only for forward static declarations of arrays of unspecified size?)

    20. serhiy-storchaka commented on May 3, 2015

      @serhiy-storchaka
      serhiy-storchaka
      on May 3, 2015
      MemberAuthor

      Then the patch LGTM. It is easier to make these changes by hand than make a review for moved code on Rietveld.

      I'm not sure, but may be forward static declarations of arrays of unspecified size is C99-ism?

    21. zooba commented on May 3, 2015

      @zooba
      zooba
      on May 3, 2015
      Member

      It fails on VC10, 11, 12 and 14, so I doubt it's going to be changed. That said, it looks like the non-static forward definition may be some sort of extension, since it causes a redefinition error (mismatched storage class) on all versions when using /Za to disable extensions.

      It could be a C99 feature ("tentative definitions" I think), as Serhiy suggests, but if so then it's very likely not going to be added between the RC (released last week) and final versions of the compiler.

    22. zooba commented on May 3, 2015

      @zooba
      zooba
      on May 3, 2015
      Member

      According to http://compgroups.net/comp.std.c/why-does-the-standard-prohibit-static-int-a/2569729, it's invalid in both C89 and C99, but some compilers accept it as an extension.

      IMO we should avoid relying on compiler extensions, at least in the code files that are supposed to be portable.

    23. larryhastings commented on May 3, 2015

      @larryhastings
      larryhastings
      on May 3, 2015
      Contributor

      I agree. I looked it up in the C99 standard. 6.9.2.2 says:

      "If the declaration of an identifier for an object is a tentative definition and has internal linkage, the declared type shall not be an incomplete type."

      And if you'd hurry up and bless my patch for Modules/_tkinter.c in bug bpo-20168, I'd check this fix in ;-)

    24. larryhastings commented on May 3, 2015

      @larryhastings
      larryhastings
      on May 3, 2015
      Contributor

      (sorry, 6.9.2.3)

    25. zooba commented on May 3, 2015

      @zooba
      zooba
      on May 3, 2015
      Member

      Sorry, wasn't watching the other issue. :)

    26. python-dev commented on May 3, 2015

      @python-dev
      python-devmannequin
      on May 3, 2015
      Mannequin

      New changeset 7a76c462c7f6 by Larry Hastings in branch 'default':
      Fix Windows build breakage from checkins on Issues bpo-20148 and bpo-20168.
      https://hg.python.org/cpython/rev/7a76c462c7f6

    27. larryhastings commented on May 3, 2015

      @larryhastings
      larryhastings
      on May 3, 2015
      Contributor

      Steve, please close this issue when you've confirmed it's now building correctly on Windows.

    28. serhiy-storchaka
      closed this as completedon May 4, 2015
    29. ezio-melotti
      transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    Labels

    extension-modulesC modules in the Modules dirtopic-argument-clinictopic-regextype-featureA feature request or enhancement

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions