re.error: nothing to repeat

[serhiy-storchaka]

BPO	18647
Nosy	@tim-one, @arigo, @pitrou, @larryhastings, @ezio-melotti, @serhiy-storchaka
Files	issue18647.patch

^{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 2013-08-25.15:37:45.729>
created_at = <Date 2013-08-03.20:42:07.237>
labels = ['expert-regex', 'type-bug', 'library']
title = 're.error: nothing to repeat'
updated_at = <Date 2013-08-25.15:37:45.729>
user = 'https://github.com/serhiy-storchaka'

bugs.python.org fields:

activity = <Date 2013-08-25.15:37:45.729>
actor = 'serhiy.storchaka'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2013-08-25.15:37:45.729>
closer = 'serhiy.storchaka'
components = ['Library (Lib)', 'Regular Expressions']
creation = <Date 2013-08-03.20:42:07.237>
creator = 'serhiy.storchaka'
dependencies = []
files = ['31159']
hgrepos = []
issue_num = 18647
keywords = ['patch']
message_count = 28.0
messages = ['194297', '194298', '194339', '194366', '194370', '194373', '194397', '194399', '194400', '194406', '194412', '194419', '194426', '194434', '194437', '194438', '194440', '194553', '194568', '194785', '194787', '194823', '194825', '194826', '194871', '194897', '195219', '195661']
nosy_count = 10.0
nosy_names = ['tim.peters', 'arigo', 'pitrou', 'larry', 'ezio.melotti', 'mrabarnett', 'Arfrever', 'eli.bendersky', 'python-dev', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue18647'
versions = ['Python 2.7', 'Python 3.3', 'Python 3.4']

[serhiy-storchaka]

Now all doctests failed on 32-bit platforms due to the unlucky coincidence of my patch with at least two bugs which were hided before.

SubPattern.getwidth() is wrong, it truncates resulted values to sys.maxsize (should be MAXREPEAT). As side effect of my patch (on 32-bit MAXREPEAT == sys.maxsize) it now returns correct value in some cases on 32-bit platforms. On other hand, the _simple() function in sre_compile.py checks if getwidth() returns (0, MAXREPEAT) and raise an error in such case. Perhaps it should guards against such patterns as '(x*)' (but it doesn't guards against '(x*y?)' or '(x*y*)' and can raise false positive). Now getwidth() returns (0, MAXREPEAT) for '(x*y?)' on 32-bit platforms (this is a correct result) and triggers the check. The doctest module uses regular expression pattern '(?:.*$\n?)*' which now causes an error.

Definitely SubPattern.getwidth() is wrong and should be fixed. At least one of two, the _simple() function or doctest pattern should be fixed too. The simplest fix is disable the 'nothing to repeat' check.

[python-dev]

New changeset c243896e12be by Serhiy Storchaka in branch '3.3':
Issue bpo-18647: Temporary disable the "nothing to repeat" check to make buildbots happy.
http://hg.python.org/cpython/rev/c243896e12be

New changeset 4faf9b73c3df by Serhiy Storchaka in branch 'default':
Issue bpo-18647: Temporary disable the "nothing to repeat" check to make buildbots happy.
http://hg.python.org/cpython/rev/4faf9b73c3df

[python-dev]

New changeset e2ba4592ce3a by Serhiy Storchaka in branch '2.7':
Issue bpo-18647: Temporary disable the "nothing to repeat" check to make buildbots happy.
http://hg.python.org/cpython/rev/e2ba4592ce3a

[elibendersky]

Would it not be better to temporarily-fix the test rather than the code?

[serhiy-storchaka]

All doctests affected.

[elibendersky]

Wonderfully terse, as usual. Can you be so kind to elaborate just a tiny bit more? Is the amount of doctests this affects so large that it's better to change the implementation? What are the plans for this "temporary" stage - is there an intention to fix the code "soon" and revert the disabling of this error check?

[tim-one]

Serhiy, I don't see the regexp '(?:.*$\n?)*' anywhere in doctest.py. Are you talking about the _EXAMPLE_RE regexp? That's the closest I see.

If that's the case, the "nothing to repeat" error is incorrect: _EXAMPLE_RE also contains a negative lookahead assertion '(?![ ]*$)' to ensure that the later '.*$\n?' part never tries to match an empty string.

That said, it takes some intelligence to realize that the negative lookahead assertion prevents repeating an empty match in this regexp, so it may not be easy to fix this false positive.

A compromise may be to replace

.*$\n?

with

.+$\n? | .*$\n

Both branches then "obviously" consume at least one character.

[serhiy-storchaka]

The doctest engine uses a regexp which contains subpattern which now considered as illegal be the regexp engine (due to unlucky coincidence MAXREPEAT == sys.maxsize on 32-bit platforms). We should rewrite the _simple() function in the re module to be more smart. But if this assumption is correct and this subpattern is really dangerous we should also rewrite a regular expression in the doctest module. In any case we should fix SubPattern.getwidth() so that the behavior on 32-bit and 64-bit platforms should be the same.

[tim-one]

Serhiy, I'm asking you to be very explicit about which regexp in doctest.py you're talking about. If you're talking about the _EXAMPLE_RE regexp, I already explained what's going on with that. If you're talking about some other regexp, I have no idea which one you're talking about.

[serhiy-storchaka]

Serhiy, I don't see the regexp '(?:.*$\n?)*' anywhere in doctest.py. Are you talking about the _EXAMPLE_RE regexp? That's the closest I see.

Yes, it is. In my previous message I answered Eli.

If that's the case, the "nothing to repeat" error is incorrect: _EXAMPLE_RE also contains a negative lookahead assertion '(?![ ]*$)' to ensure that the later '.*$\n?' part never tries to match an empty string.

Thank you for explanation. Unlucky the getwidth() method is not smart enough to detect that minimal length of matched string is not 0. We should made either the getwidth() method or the _simple() function (or both) more smart.

A compromise may be to replace
.*$\n?
with
.+$\n? | .*$\n

I'm sure similar patterns are used in third-party code. We shouldn't break them, therefore we should fix _simple()/getwidth().

[tim-one]

I'm afraid it's just too tricky for the code to deduce that a negative lookahead assertion can imply that a later match can't be empty. But I don't know how smart the re compilation code already is ;-)

It occurs to me now that the doctest regexp could worm around this very easily, via replacing:

.*$\n?

with:

.+$\n?

The success of the negative lookahead assertion here doesn't _just_ imply that

.*$\n?

will match a non-empty string, it also implies that

.+$

will succeed (and so also that .+$\n? will succeed).