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

serhiy-storchaka · 2014-01-06T16:04:29Z

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']

serhiy-storchaka · 2014-01-06T16:04:29Z

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 =

larryhastings · 2014-01-06T16:17:08Z

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?

serhiy-storchaka · 2014-01-06T16:36:10Z

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.

larryhastings · 2014-01-07T20:53:45Z

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.

serhiy-storchaka · 2014-01-07T23:57:18Z

Here is refreshed patch.

larryhastings · 2014-01-08T00:25:39Z

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

serhiy-storchaka · 2014-01-18T12:23:32Z

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.

serhiy-storchaka · 2014-03-20T09:30:25Z

Here is updated patch.

serhiy-storchaka · 2014-12-01T14:09:42Z

Synchronized with the tip again.

serhiy-storchaka · 2015-04-17T20:11:25Z

Updated to the tip.

python-dev · 2015-05-03T13:18:49Z

New changeset d5f78a855355 by Serhiy Storchaka in branch 'default':
Issue bpo-20148: Converted the _sre module to Argument Clinic.
https://hg.python.org/cpython/rev/d5f78a855355

zooba · 2015-05-03T19:37:44Z

This broke Windows builds because of unnecessary "static" qualifiers on the forward declarations at lines 1429, 2472 and 2715 (as discussed on bpo-20323).

Removing "static" from these lines fixes the build.

larryhastings · 2015-05-03T19:48:59Z

Removing "static" breaks GCC on Linux (gcc 4.9.2 on Ubuntu 15.04 x86-64):

./Modules/_sre.c:2780:20: error: static declaration of ‘pattern_methods’ follows non-static declaration
static PyMethodDef pattern_methods[] = {
^
./Modules/_sre.c:1429:13: note: previous declaration of ‘pattern_methods’ was here
PyMethodDef pattern_methods[];
^

IMO MSVS is the one being unreasonable here. But obviously we need to find some way of doing this that makes both compilers happy.

larryhastings · 2015-05-03T19:54:56Z

Steve, does this patch fix the build for Windows? By all rights it oughta.

zooba · 2015-05-03T19:59:56Z

This line (1429 before patch, 1433 after patch) is still being troublesome:

static PyMethodDef pattern_methods[];

zooba · 2015-05-03T20:00:40Z

But looks like it's unnecessary and just wasn't removed in the patch. Everything builds fine without it

larryhastings · 2015-05-03T20:01:17Z

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

serhiy-storchaka · 2015-05-03T20:01:58Z

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.

larryhastings · 2015-05-03T20:03:07Z

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.

zooba · 2015-05-03T20:08:58Z

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...

larryhastings · 2015-05-03T20:10:49Z

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?)

serhiy-storchaka · 2015-05-03T20:22:24Z

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?

zooba · 2015-05-03T20:32:23Z

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.

zooba · 2015-05-03T20:42:12Z

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.

larryhastings · 2015-05-03T20:47:32Z

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 ;-)

larryhastings · 2015-05-03T20:48:32Z

(sorry, 6.9.2.3)

zooba · 2015-05-03T21:44:19Z

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

python-dev · 2015-05-03T21:49:52Z

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

larryhastings · 2015-05-03T21:53:53Z

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

serhiy-storchaka added topic-regex extension-modules C modules in the Modules dir type-feature A feature request or enhancement labels Jan 6, 2014

larryhastings changed the title ~~Convert the _sre module to use Argument Clinic~~ Derby: Convert the _sre module to use Argument Clinic Jan 7, 2014

larryhastings assigned serhiy-storchaka Jan 8, 2014

serhiy-storchaka added the topic-argument-clinic label Feb 25, 2015

serhiy-storchaka closed this as completed May 3, 2015

zooba reopened this May 3, 2015

serhiy-storchaka closed this as completed May 4, 2015

ezio-melotti transferred this issue from another repository Apr 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

serhiy-storchaka commented Jan 6, 2014

serhiy-storchaka commented Jan 6, 2014

larryhastings commented Jan 6, 2014

serhiy-storchaka commented Jan 6, 2014

larryhastings commented Jan 7, 2014

serhiy-storchaka commented Jan 7, 2014

larryhastings commented Jan 8, 2014

serhiy-storchaka commented Jan 18, 2014

serhiy-storchaka commented Mar 20, 2014

serhiy-storchaka commented Dec 1, 2014

serhiy-storchaka commented Apr 17, 2015

python-dev mannequin commented May 3, 2015

zooba commented May 3, 2015

larryhastings commented May 3, 2015

larryhastings commented May 3, 2015

zooba commented May 3, 2015

zooba commented May 3, 2015

larryhastings commented May 3, 2015

serhiy-storchaka commented May 3, 2015

larryhastings commented May 3, 2015

zooba commented May 3, 2015

larryhastings commented May 3, 2015

serhiy-storchaka commented May 3, 2015

zooba commented May 3, 2015

zooba commented May 3, 2015

larryhastings commented May 3, 2015

larryhastings commented May 3, 2015

zooba commented May 3, 2015

python-dev mannequin commented May 3, 2015

larryhastings commented May 3, 2015

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

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

Comments

serhiy-storchaka commented Jan 6, 2014

serhiy-storchaka commented Jan 6, 2014

larryhastings commented Jan 6, 2014

serhiy-storchaka commented Jan 6, 2014

larryhastings commented Jan 7, 2014

serhiy-storchaka commented Jan 7, 2014

larryhastings commented Jan 8, 2014

serhiy-storchaka commented Jan 18, 2014

serhiy-storchaka commented Mar 20, 2014

serhiy-storchaka commented Dec 1, 2014

serhiy-storchaka commented Apr 17, 2015

python-dev mannequin commented May 3, 2015

zooba commented May 3, 2015

larryhastings commented May 3, 2015

larryhastings commented May 3, 2015

zooba commented May 3, 2015

zooba commented May 3, 2015

larryhastings commented May 3, 2015

serhiy-storchaka commented May 3, 2015

larryhastings commented May 3, 2015

zooba commented May 3, 2015

larryhastings commented May 3, 2015

serhiy-storchaka commented May 3, 2015

zooba commented May 3, 2015

zooba commented May 3, 2015

larryhastings commented May 3, 2015

larryhastings commented May 3, 2015

zooba commented May 3, 2015

python-dev mannequin commented May 3, 2015

larryhastings commented May 3, 2015