Skip to content

Commit 522a9e9

Browse files
ppislppisl
committed
[GR-11981] str.count() returns wrong results.
1 parent d5f83db commit 522a9e9

File tree

3 files changed

+181
-71
lines changed

3 files changed

+181
-71
lines changed

‎graalpython/com.oracle.graal.python.test/src/tests/test_string.py‎

Lines changed: 115 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,12 @@
77
import sys
88

99

10+
class MyIndexable(object):
11+
def __init__(self, value):
12+
self.value = value
13+
def __index__(self):
14+
return self.value
15+
1016
def test_find():
1117
assert "teststring".find("test") == 0
1218
assert "teststring".find("string") == 4
@@ -23,6 +29,31 @@ def test_find():
2329
assert "teststring".find("tst", None, 2) == -1
2430
assert "teststring".find("st", None, 4) == 2
2531

32+
s = 'ahoj cau nazadar ahoj'
33+
assert s.find('ahoj') == 0
34+
assert s.find('ahoj', 4) == 17
35+
assert s.find('ahoj', -3) == -1
36+
assert s.find('ahoj', -21) == 0
37+
assert s.find('cau', -21) == 5
38+
assert s.find('cau', -36, -10) == 5
39+
assert s.find('cau', None) == 5
40+
assert s.find('ahoj', None) == 0
41+
assert s.find('cau', None, 8) == 5
42+
assert s.find('cau', None, 7) == -1
43+
assert s.find('u', 3) == 7
44+
assert s.find('u', 3, 7) == -1
45+
assert s.find('u', 3, 8) == 7
46+
assert s.find('u', -18, -13) == 7
47+
assert s.find('u', -18, -12) == 7
48+
assert s.find('u', -18, -14) == -1
49+
assert s.find('u', -14, -13) == 7
50+
assert s.find('u', -12, -13) == -1
51+
assert s.find('cau', MyIndexable(4)) == 5
52+
assert s.find('cau', MyIndexable(5)) == 5
53+
assert s.find('cau', MyIndexable(5), None) == 5
54+
assert s.find('cau', MyIndexable(5), MyIndexable(8)) == 5
55+
assert s.find('cau', None, MyIndexable(8)) == 5
56+
2657

2758
def test_rfind():
2859
assert "test string test".rfind("test") == 12
@@ -34,6 +65,20 @@ def test_rfind():
3465
assert "test string test".rfind("test", 4, 14) == -1
3566
assert "test string test".rfind("test", None, 14) == 0
3667

68+
s = 'ahoj cau nazdar ahoj'
69+
assert s.rfind('cau', None, None) == 5
70+
assert s.rfind('cau', -25, None) == 5
71+
assert s.rfind('cau', -25, -3) == 5
72+
assert s.rfind('cau', -25, -12) == 5
73+
assert s.rfind('cau', -25, -13) == -1
74+
assert s.rfind('cau', -15, -12) == 5
75+
assert s.rfind('cau', -14, -12) == -1
76+
assert s.rfind('ahoj', -14) == 16
77+
assert s.rfind('ahoj', -4) == 16
78+
assert s.rfind('ahoj', -3) == -1
79+
assert s.rfind('ahoj', 16) == 16
80+
assert s.rfind('ahoj', 16, 20) == 16
81+
assert s.rfind('ahoj', 16, 19) == -1
3782

3883
def test_format():
3984
assert "{}.{}".format("part1", "part2") == "part1.part2"
@@ -707,13 +752,6 @@ def test_zfill(self):
707752

708753
def test_zfill_specialization(self):
709754
self.checkequal('123', '123', 'zfill', True)
710-
711-
class MyIndexable(object):
712-
def __init__(self, value):
713-
self.value = value
714-
def __index__(self):
715-
return self.value
716-
717755
self.checkequal('0123', '123', 'zfill', MyIndexable(4))
718756

719757
def test_title(self):
@@ -774,6 +812,76 @@ def test_center_uni(self):
774812
self.assertEqual('x'.center(4, '\U0010FFFF'),
775813
'\U0010FFFFx\U0010FFFF\U0010FFFF')
776814

815+
# Whether the "contained items" of the container are integers in
816+
# range(0, 256) (i.e. bytes, bytearray) or strings of length 1
817+
# (str)
818+
contains_bytes = False
819+
820+
def test_count(self):
821+
self.checkequal(3, 'aaa', 'count', 'a')
822+
self.checkequal(0, 'aaa', 'count', 'b')
823+
self.checkequal(3, 'aaa', 'count', 'a')
824+
self.checkequal(0, 'aaa', 'count', 'b')
825+
self.checkequal(3, 'aaa', 'count', 'a')
826+
self.checkequal(0, 'aaa', 'count', 'b')
827+
self.checkequal(0, 'aaa', 'count', 'b')
828+
self.checkequal(2, 'aaa', 'count', 'a', 1)
829+
self.checkequal(0, 'aaa', 'count', 'a', 10)
830+
self.checkequal(1, 'aaa', 'count', 'a', -1)
831+
self.checkequal(3, 'aaa', 'count', 'a', -10)
832+
self.checkequal(1, 'aaa', 'count', 'a', 0, 1)
833+
self.checkequal(3, 'aaa', 'count', 'a', 0, 10)
834+
self.checkequal(2, 'aaa', 'count', 'a', 0, -1)
835+
self.checkequal(0, 'aaa', 'count', 'a', 0, -10)
836+
self.checkequal(3, 'aaa', 'count', '', 1)
837+
self.checkequal(1, 'aaa', 'count', '', 3)
838+
self.checkequal(0, 'aaa', 'count', '', 10)
839+
self.checkequal(2, 'aaa', 'count', '', -1)
840+
self.checkequal(4, 'aaa', 'count', '', -10)
841+
842+
self.checkequal(1, '', 'count', '')
843+
self.checkequal(0, '', 'count', '', 1, 1)
844+
self.checkequal(0, '', 'count', '', sys.maxsize, 0)
845+
846+
self.checkequal(0, '', 'count', 'xx')
847+
self.checkequal(0, '', 'count', 'xx', 1, 1)
848+
self.checkequal(0, '', 'count', 'xx', sys.maxsize, 0)
849+
850+
self.checkraises(TypeError, 'hello', 'count')
851+
852+
if self.contains_bytes:
853+
self.checkequal(0, 'hello', 'count', 42)
854+
else:
855+
self.checkraises(TypeError, 'hello', 'count', 42)
856+
857+
# For a variety of combinations,
858+
# verify that str.count() matches an equivalent function
859+
# replacing all occurrences and then differencing the string lengths
860+
charset = ['', 'a', 'b']
861+
digits = 7
862+
base = len(charset)
863+
teststrings = set()
864+
for i in range(base ** digits):
865+
entry = []
866+
for j in range(digits):
867+
i, m = divmod(i, base)
868+
entry.append(charset[m])
869+
teststrings.add(''.join(entry))
870+
teststrings = [self.fixtype(ts) for ts in teststrings]
871+
for i in teststrings:
872+
n = len(i)
873+
for j in teststrings:
874+
r1 = i.count(j)
875+
if j:
876+
r2, rem = divmod(n - len(i.replace(j, self.fixtype(''))),
877+
len(j))
878+
else:
879+
r2, rem = len(i)+1, 0
880+
if rem or r1 != r2:
881+
self.assertEqual(rem, 0, '%s != 0 for %s' % (rem, i))
882+
self.assertEqual(r1, r2, '%s != %s for %s' % (r1, r2, i))
883+
884+
777885
def test_same_id():
778886
empty_ids = set([id(str()) for i in range(100)])
779887
assert len(empty_ids) == 1

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/str/StringBuiltins.java‎

Lines changed: 47 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -522,42 +522,67 @@ public Object endsWith(Object self, Object prefix) {
522522
@TypeSystemReference(PythonArithmeticTypes.class)
523523
abstract static class FindBaseNode extends PythonBuiltinNode {
524524

525+
private @Child CastToIndexNode startNode;
526+
private @Child CastToIndexNode endNode;
527+
528+
private CastToIndexNode getStartNode() {
529+
if (startNode == null) {
530+
CompilerDirectives.transferToInterpreterAndInvalidate();
531+
startNode = insert(CastToIndexNode.createOverflow());
532+
}
533+
return startNode;
534+
}
535+
536+
private CastToIndexNode getEndNode() {
537+
if (endNode == null) {
538+
CompilerDirectives.transferToInterpreterAndInvalidate();
539+
endNode = insert(CastToIndexNode.createOverflow());
540+
}
541+
return endNode;
542+
}
543+
544+
private SliceInfo computeSlice(int length, long start, long end) {
545+
int step = start < end ? 1 : -1;
546+
PSlice tmpSlice = factory().createSlice(getStartNode().execute(start), getEndNode().execute(end), step);
547+
return tmpSlice.computeIndices(length);
548+
}
549+
550+
private SliceInfo computeSlice(int length, Object startO, Object endO) {
551+
int start = startO == PNone.NO_VALUE || startO == PNone.NONE ? 0 : getStartNode().execute(startO);
552+
int end = endO == PNone.NO_VALUE || endO == PNone.NONE ? length : getEndNode().execute(endO);
553+
int step = start < end ? 1 : -1;
554+
PSlice tmpSlice = factory().createSlice(start, end, step);
555+
return tmpSlice.computeIndices(length);
556+
}
557+
525558
@Specialization
526559
Object find(String self, String str, @SuppressWarnings("unused") PNone start, @SuppressWarnings("unused") PNone end) {
527560
return find(self, str);
528561
}
529562

530563
@Specialization
531564
Object find(String self, String str, long start, @SuppressWarnings("unused") PNone end) {
532-
return findGeneric(self, str, start, -1);
565+
int len = self.length();
566+
SliceInfo info = computeSlice(len, start, len);
567+
return findWithBounds(self, str, info.start, info.stop);
533568
}
534569

535570
@Specialization
536571
Object find(String self, String str, @SuppressWarnings("unused") PNone start, long end) {
537-
return findGeneric(self, str, -1, end);
572+
SliceInfo info = computeSlice(self.length(), 0, end);
573+
return findWithBounds(self, str, info.start, info.stop);
538574
}
539575

540576
@Specialization
541577
Object find(String self, String str, long start, long end) {
542-
return findGeneric(self, str, start, end);
578+
SliceInfo info = computeSlice(self.length(), start, end);
579+
return findWithBounds(self, str, info.start, info.stop);
543580
}
544581

545-
@Specialization(guards = {"isNumberOrNone(start)", "isNumberOrNone(end)"}, rewriteOn = ArithmeticException.class)
582+
@Specialization
546583
Object findGeneric(String self, String str, Object start, Object end) throws ArithmeticException {
547-
int startInt = getIntValue(start);
548-
int endInt = getIntValue(end);
549-
return findWithBounds(self, str, startInt, endInt);
550-
}
551-
552-
@Specialization(guards = {"isNumberOrNone(start)", "isNumberOrNone(end)"}, replaces = "findGeneric")
553-
Object findGenericOvf(String self, String str, Object start, Object end) {
554-
try {
555-
int startInt = getIntValue(start);
556-
int endInt = getIntValue(end);
557-
return findWithBounds(self, str, startInt, endInt);
558-
} catch (ArithmeticException e) {
559-
throw raise(ValueError, "cannot fit 'int' into an index-sized integer");
560-
}
584+
SliceInfo info = computeSlice(self.length(), start, end);
585+
return findWithBounds(self, str, info.start, info.stop);
561586
}
562587

563588
@Fallback
@@ -566,25 +591,6 @@ Object findFail(Object self, Object str, Object start, Object end) {
566591
throw raise(TypeError, "must be str, not %p", str);
567592
}
568593

569-
protected static boolean isNumberOrNone(Object o) {
570-
return o instanceof PInt || o instanceof PNone;
571-
}
572-
573-
private static int getIntValue(Object o) throws ArithmeticException {
574-
if (o instanceof Integer) {
575-
return (int) o;
576-
} else if (o instanceof Long) {
577-
return PInt.intValueExact((long) o);
578-
} else if (o instanceof Boolean) {
579-
return PInt.intValue((boolean) o);
580-
} else if (o instanceof PInt) {
581-
return ((PInt) o).intValueExact();
582-
} else if (o instanceof PNone) {
583-
return -1;
584-
}
585-
throw new IllegalArgumentException();
586-
}
587-
588594
@SuppressWarnings("unused")
589595
protected int find(String self, String findStr) {
590596
throw new AssertionError("must not be reached");
@@ -610,16 +616,8 @@ protected int find(String self, String findStr) {
610616
@Override
611617
@TruffleBoundary
612618
protected int findWithBounds(String self, String str, int start, int end) {
613-
if (start != -1 && end != -1) {
614-
int idx = self.lastIndexOf(str, end - str.length() - 1);
615-
return idx >= start ? idx : -1;
616-
} else if (start != -1) {
617-
int idx = self.lastIndexOf(str);
618-
return idx >= start ? idx : -1;
619-
} else {
620-
assert end != -1;
621-
return self.lastIndexOf(str, end - str.length() - 1);
622-
}
619+
int idx = self.lastIndexOf(str, end - str.length());
620+
return idx >= start ? idx : -1;
623621
}
624622
}
625623

@@ -637,16 +635,8 @@ protected int find(String self, String findStr) {
637635
@Override
638636
@TruffleBoundary
639637
protected int findWithBounds(String self, String str, int start, int end) {
640-
if (start != -1 && end != -1) {
641-
int idx = self.indexOf(str, start);
642-
return idx + str.length() <= end ? idx : -1;
643-
} else if (start != -1) {
644-
return self.indexOf(str, start);
645-
} else {
646-
assert end != -1;
647-
int idx = self.indexOf(str);
648-
return idx + str.length() <= end ? idx : -1;
649-
}
638+
int idx = self.indexOf(str, start);
639+
return idx + str.length() <= end ? idx : -1;
650640
}
651641
}
652642

‎graalpython/lib-graalpython/str.py‎

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -341,17 +341,29 @@ def __iter__(self):
341341
str.__iter__ = __iter__
342342

343343

344-
def strcount(self, sub, start=0, end=-1):
345-
if len(self) == 0:
344+
def strcount(self, sub, start=None, end=None):
345+
selfLeng = len(self)
346+
subLeng = len(sub)
347+
if start == None:
348+
start = 0
349+
if selfLeng == 0:
350+
if subLeng == 0 and start <= 0:
351+
return 1
346352
return 0
347-
if end < 0:
348-
end = (len(self) + end) % len(self)
349-
cnt = 0
353+
if end == None:
354+
end = selfLeng
355+
if subLeng == 0:
356+
if start <= selfLeng:
357+
return len(self[start:end]) + 1
358+
return 0
359+
350360
idx = self.find(sub, start, end)
351361
if idx < 0:
352362
return 0
353-
while idx < end and idx >= 0:
354-
start = idx + 1
363+
364+
cnt = 1
365+
while idx < selfLeng and idx >= 0 and cnt < selfLeng:
366+
start = idx + subLeng
355367
idx = self.find(sub, start, end)
356368
if idx >= 0:
357369
cnt += 1

0 commit comments

Comments
 (0)