[PATCH] D56250: python compat - iterator protocol

serge via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 3 06:42:06 PST 2019


serge-sans-paille added inline comments.


================
Comment at: utils/gdb-scripts/prettyprinters.py:84
+    if sys.version_info.major == 2:
+        __next__ = next
 
----------------
michaelplatings wrote:
> Should be `next = __next__`
> Or don't make this change at all as the code should already work with Python 3.
I made the move to make it clear that python3 should be the default standard, and the ugly boilerplate is the python2 compatibility.


================
Comment at: utils/gdb-scripts/prettyprinters.py:90
+    if sys.version_info.major == 2:
+        __next__ = next
 
----------------
michaelplatings wrote:
> 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.
Correct! I've been in auto-fix mode for some of these commits, thanks *a lot* for spotting all these mistakes.


================
Comment at: utils/gdb-scripts/prettyprinters.py:175
 
-    def next(self):
+    def __next__(self):
       if self.cur == self.end:
----------------
michaelplatings wrote:
> 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.
For the reasons given above, I still made the change, and fixed the behavior below. I also checked the other next implementation.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56250/new/

https://reviews.llvm.org/D56250





More information about the llvm-commits mailing list