This is an archive of the discontinued LLVM Phabricator instance.

Differential D56250

python compat - iterator protocol
ClosedPublic

Authored by serge-sans-paille on Jan 3 2019, 3:19 AM.

Details

Reviewers
michaelplatings
Commits
rGdd84c9d6638a: Python compat - iterator protocol
rL350326: Python compat - iterator protocol

Diff Detail

Repository
rL LLVM

Event Timeline

serge-sans-paille created this revision.Jan 3 2019, 3:19 AM
Herald added a subscriber: llvm-commits. · View Herald TranscriptJan 3 2019, 3:19 AM
michaelplatings requested changes to this revision.Jan 3 2019, 5:11 AM
michaelplatings added inline comments.
utils/gdb-scripts/prettyprinters.py
73 ↗(On Diff #180013)

I think this needs an associated

if sys.version_info.major == 2:
    next == __next__
172 ↗(On Diff #180013)

I think this needs an associated

if sys.version_info.major == 2:
    next == __next__
This revision now requires changes to proceed.Jan 3 2019, 5:11 AM
serge-sans-paille updated this revision to Diff 180048.Jan 3 2019, 6:04 AM
serge-sans-paille marked 2 inline comments as done.
michaelplatings requested changes to this revision.Jan 3 2019, 6:28 AM
michaelplatings added inline comments.
bindings/python/llvm/core.py
403 ↗(On Diff #180048)

== should be =

utils/gdb-scripts/prettyprinters.py
84 ↗(On Diff #180048)

Should be next = __next__
Or don't make this change at all as the code should already work with Python 3.

90 ↗(On Diff #180048)

This line is just wrong in the original. It creates a variable __next__ that is set to the built in next function. It serves no use and should be deleted.

175 ↗(On Diff #180048)

I see at line 183:
__next__ = next
which would be broken by this change. I suggest not making this change as the code should already work for Python 3.

This revision now requires changes to proceed.Jan 3 2019, 6:28 AM
serge-sans-paille updated this revision to Diff 180065.Jan 3 2019, 6:41 AM
serge-sans-paille marked 7 inline comments as done.
serge-sans-paille added inline comments.
utils/gdb-scripts/prettyprinters.py
84 ↗(On Diff #180048)

I made the move to make it clear that python3 should be the default standard, and the ugly boilerplate is the python2 compatibility.

90 ↗(On Diff #180048)

Correct! I've been in auto-fix mode for some of these commits, thanks *a lot* for spotting all these mistakes.

175 ↗(On Diff #180048)

For the reasons given above, I still made the change, and fixed the behavior below. I also checked the other next implementation.

michaelplatings requested changes to this revision.Jan 3 2019, 7:00 AM
michaelplatings added inline comments.
utils/gdb-scripts/prettyprinters.py
185 ↗(On Diff #180065)

makor -> major

This revision now requires changes to proceed.Jan 3 2019, 7:00 AM
serge-sans-paille updated this revision to Diff 180069.Jan 3 2019, 7:04 AM
serge-sans-paille marked an inline comment as done.
michaelplatings requested changes to this revision.Jan 3 2019, 7:24 AM
michaelplatings added inline comments.
bindings/python/llvm/core.py
248 ↗(On Diff #180069)

I had a poke around this file and found that self.function.next refers to the next property in the Function class. I don't think the Function class should be changed as it isn't intended to be an iterator, and therefore the change to this line should be reverted.

319 ↗(On Diff #180069)

Likewise, self.bb.next is a property of BasicBlock.
The change to this line should be reverted.

399 ↗(On Diff #180069)

Likewise, self.inst.next is a property of Instruction.
The change to this line should be reverted.

bindings/python/llvm/tests/test_core.py
101 ↗(On Diff #180069)

I think this refers to the next property of the Function class in core.py. The change on this line should be reverted.

This revision now requires changes to proceed.Jan 3 2019, 7:24 AM
serge-sans-paille updated this revision to Diff 180077.Jan 3 2019, 7:41 AM
serge-sans-paille marked 3 inline comments as done.
michaelplatings accepted this revision.Jan 3 2019, 7:44 AM

LGTM

This revision is now accepted and ready to land.Jan 3 2019, 7:44 AM
Closed by commit rL350326: Python compat - iterator protocol (authored by serge_sans_paille). · Explain WhyJan 3 2019, 7:46 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
llvm/
trunk/
bindings/
python/
llvm/
21 lines
utils/
gdb-scripts/
19 lines

Diff 180078

llvm/trunk/bindings/python/llvm/core.py

Show All 13 Lines
from . import enumerations from . import enumerations
from ctypes import POINTER from ctypes import POINTER
from ctypes import byref from ctypes import byref
from ctypes import c_char_p from ctypes import c_char_p
from ctypes import c_uint from ctypes import c_uint
import sys
__all__ = [ __all__ = [
"lib", "lib",
"Enums", "Enums",
"OpCode", "OpCode",
"MemoryBuffer", "MemoryBuffer",
"Module", "Module",
"Value", "Value",
"Function", "Function",
▲ Show 20 Lines • Show All 201 Lines • ▼ Show 20 Lines class __function_iterator(object):
if self.reverse: if self.reverse:
self.function = self.module.last self.function = self.module.last
else: else:
self.function = self.module.first self.function = self.module.first
def __iter__(self): def __iter__(self):
return self return self
def next(self): def __next__(self):
if not isinstance(self.function, Function): if not isinstance(self.function, Function):
raise StopIteration("") raise StopIteration("")
result = self.function result = self.function
if self.reverse: if self.reverse:
self.function = self.function.prev self.function = self.function.prev
else: else:
self.function = self.function.next self.function = self.function.next
return result return result
if sys.version_info.major == 2:
next = __next__
def __iter__(self): def __iter__(self):
return Module.__function_iterator(self) return Module.__function_iterator(self)
def __reversed__(self): def __reversed__(self):
return Module.__function_iterator(self, reverse=True) return Module.__function_iterator(self, reverse=True)
@property @property
def first(self): def first(self):
▲ Show 20 Lines • Show All 42 Lines • ▼ Show 20 Lines class __bb_iterator(object):
if self.reverse: if self.reverse:
self.bb = function.last self.bb = function.last
else: else:
self.bb = function.first self.bb = function.first
def __iter__(self): def __iter__(self):
return self return self
def next(self): def __next__(self):
if not isinstance(self.bb, BasicBlock): if not isinstance(self.bb, BasicBlock):
raise StopIteration("") raise StopIteration("")
result = self.bb result = self.bb
if self.reverse: if self.reverse:
self.bb = self.bb.prev self.bb = self.bb.prev
else: else:
self.bb = self.bb.next self.bb = self.bb.next
return result return result
if sys.version_info.major == 2:
next = __next__
def __iter__(self): def __iter__(self):
return Function.__bb_iterator(self) return Function.__bb_iterator(self)
def __reversed__(self): def __reversed__(self):
return Function.__bb_iterator(self, reverse=True) return Function.__bb_iterator(self, reverse=True)
def __len__(self): def __len__(self):
return lib.LLVMCountBasicBlocks(self) return lib.LLVMCountBasicBlocks(self)
▲ Show 20 Lines • Show All 51 Lines • ▼ Show 20 Lines class __inst_iterator(object):
if self.reverse: if self.reverse:
self.inst = self.bb.last self.inst = self.bb.last
else: else:
self.inst = self.bb.first self.inst = self.bb.first
def __iter__(self): def __iter__(self):
return self return self
def next(self): def __next__(self):
if not isinstance(self.inst, Instruction): if not isinstance(self.inst, Instruction):
raise StopIteration("") raise StopIteration("")
result = self.inst result = self.inst
if self.reverse: if self.reverse:
self.inst = self.inst.prev self.inst = self.inst.prev
else: else:
self.inst = self.inst.next self.inst = self.inst.next
return result return result
if sys.version_info.major == 2:
next = __next__
def __iter__(self): def __iter__(self):
return BasicBlock.__inst_iterator(self) return BasicBlock.__inst_iterator(self)
def __reversed__(self): def __reversed__(self):
return BasicBlock.__inst_iterator(self, reverse=True) return BasicBlock.__inst_iterator(self, reverse=True)
class Instruction(Value): class Instruction(Value):
▲ Show 20 Lines • Show All 229 Lines • Show Last 20 Lines

llvm/trunk/utils/gdb-scripts/prettyprinters.py

from __future__ import print_function from __future__ import print_function
import sys
import gdb.printing import gdb.printing
class Iterator: class Iterator:
def __iter__(self): def __iter__(self):
return self return self
# Python 2 compatibility if sys.version_info.major == 2:
def next(self): def next(self):
return self.__next__() return self.__next__()
def children(self): def children(self):
return self return self
def escape_bytes(val, l): def escape_bytes(val, l):
return '"' + val.string(encoding='Latin-1', length=l).encode('unicode_escape').decode() + '"' return '"' + val.string(encoding='Latin-1', length=l).encode('unicode_escape').decode() + '"'
class SmallStringPrinter: class SmallStringPrinter:
▲ Show 20 Lines • Show All 45 Lines • ▼ Show 20 Lines class _iterator:
def __init__(self, begin, end): def __init__(self, begin, end):
self.cur = begin self.cur = begin
self.end = end self.end = end
self.count = 0 self.count = 0
def __iter__(self): def __iter__(self):
return self return self
def next(self): def __next__(self):
if self.cur == self.end: if self.cur == self.end:
raise StopIteration raise StopIteration
count = self.count count = self.count
self.count = self.count + 1 self.count = self.count + 1
cur = self.cur cur = self.cur
self.cur = self.cur + 1 self.cur = self.cur + 1
return '[%d]' % count, cur.dereference() return '[%d]' % count, cur.dereference()
__next__ = next if sys.version_info.major == 2:
next = __next__
def __init__(self, val): def __init__(self, val):
self.val = val self.val = val
__next__ = next
def children(self): def children(self):
data = self.val['Data'] data = self.val['Data']
return self._iterator(data, data + self.val['Length']) return self._iterator(data, data + self.val['Length'])
def to_string(self): def to_string(self):
return 'llvm::ArrayRef of length %d' % (self.val['Length']) return 'llvm::ArrayRef of length %d' % (self.val['Length'])
def display_hint (self): def display_hint (self):
▲ Show 20 Lines • Show All 67 Lines • ▼ Show 20 Lines def advancePastEmptyBuckets(self):
# by const ref, and GDB # by const ref, and GDB
# It's also not entirely general - we should be accessing the "getFirst()" # It's also not entirely general - we should be accessing the "getFirst()"
# member function, not the 'first' member variable, but I've yet to figure # member function, not the 'first' member variable, but I've yet to figure
# out how to find/call member functions (especially (const) overloaded # out how to find/call member functions (especially (const) overloaded
# ones) on a gdb.Value. # ones) on a gdb.Value.
while self.cur != self.end and (is_equal(self.cur.dereference()['first'], empty) or is_equal(self.cur.dereference()['first'], tombstone)): while self.cur != self.end and (is_equal(self.cur.dereference()['first'], empty) or is_equal(self.cur.dereference()['first'], tombstone)):
self.cur = self.cur + 1 self.cur = self.cur + 1
def next(self): def __next__(self):
if self.cur == self.end: if self.cur == self.end:
raise StopIteration raise StopIteration
cur = self.cur cur = self.cur
v = cur.dereference()['first' if self.first else 'second'] v = cur.dereference()['first' if self.first else 'second']
if not self.first: if not self.first:
self.cur = self.cur + 1 self.cur = self.cur + 1
self.advancePastEmptyBuckets() self.advancePastEmptyBuckets()
self.first = True self.first = True
else: else:
self.first = False self.first = False
return 'x', v return 'x', v
__next__ = next if sys.version_info.major == 2:
next = __next__
def __init__(self, val): def __init__(self, val):
self.val = val self.val = val
def children(self): def children(self):
t = self.val.type.template_argument(3).pointer() t = self.val.type.template_argument(3).pointer()
begin = self.val['Buckets'].cast(t) begin = self.val['Buckets'].cast(t)
end = (begin + self.val['NumBuckets']).cast(t) end = (begin + self.val['NumBuckets']).cast(t)
▲ Show 20 Lines • Show All 134 Lines • Show Last 20 Lines