Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement yield-from in Python 2.x as a macro #686

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion .travis.yml
Expand Up @@ -3,7 +3,6 @@ python:
- "pypy"
- "2.6"
- "2.7"
- "3.2"
- "3.3"
- "3.4"
cache:
Expand Down
19 changes: 19 additions & 0 deletions hy/core/macros.hy
Expand Up @@ -179,6 +179,25 @@
~@body))))


(if-python2

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, I read about this PR in the dustycloud blogpost.

The idea of using macros to emulate yield from sounds wonderful. However, the implementation doesn't handle some edge cases described in the Formal Semantics section of PEP-380. Was this deliberate or is it just a proof-of-concept at the moment?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which edge cases does it not handle? I believed also that it was failling to handle exception propagation, but it turned out that this was not true (we now have a test that proves as such). If there are edge cases not being handled, please submit test cases and file bugs! Thanks! :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@superbobry makes a good point. Here's a concrete example of where the semantics differ:

hy 0.10.1 using CPython(default) 2.7.8 on Linux
=> (defn f []
...   (yield 1)
...   (try
...     (yield 2)
...     (catch [e ValueError] (yield 3))))
=> (defn g []
...   (yield-from (f))
...   (assert true))  ; see issue #691
=> (setv x (g))
=> (next x)
1L
=> (next x)
2L
=> (x.throw ValueError)
Traceback (most recent call last):
  File "<input>", line 1, in <module>
  File "<input>", line 257, in g  ; note that ValueError didn't even reach `f`
ValueError

as opposed to

hy 0.10.1 using CPython(default) 3.4.2 on Linux
...
=> (next x)
2
=> (x.throw ValueError)
3

This would pose a problem, for example, if one was to reimplement asyncio on top of yield-from, as it uses task.throw(CancelledError) to abort running tasks. Obviously, if this is not fixed, only the outermost task would be aborted; any tasks it may be waiting on will completely avoid being cancelled.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! File a bug! :)

(defmacro/g! yield-from [expr]
`(do (import types)
(setv ~g!iter (iter ~expr))
(setv ~g!return nil)
(setv ~g!message nil)
(while true
(try (if (isinstance ~g!iter types.GeneratorType)
(setv ~g!message (yield (.send ~g!iter ~g!message)))
(setv ~g!message (yield (next ~g!iter))))
(catch [~g!e StopIteration]
(do (setv ~g!return (if (hasattr ~g!e "value")
(. ~g!e value)
nil))
(break)))))
~g!return))
nil)


(defmacro defmain [args &rest body]
"Write a function named \"main\" and do the if __main__ dance"
(let [[retval (gensym)]]
Expand Down
2 changes: 1 addition & 1 deletion make.bat
Expand Up @@ -87,7 +87,7 @@ goto :EOF
if "%1" == "tox" (
:tox
call :venv
tox -e "py26,py27,py32,py33,flake8"
tox -e "py26,py27,py33,py34,flake8"
goto :EOF
)

Expand Down
8 changes: 8 additions & 0 deletions tests/native_tests/native_macros.hy
Expand Up @@ -233,5 +233,13 @@
(assert (= (tda-a2) :bazinga))
(assert (= tda-main tda-a1 tda-a2)))

(defn test-yield-from []
"NATIVE: testing yield from"
(defn yield-from-test []
(for* [i (range 3)]
(yield i))
(yield-from [1 2 3]))
(assert (= (list (yield-from-test)) [0 1 2 1 2 3])))

(defn test-botsbuildbots []
(assert (> (len (first (Botsbuildbots))) 50)))
15 changes: 0 additions & 15 deletions tests/native_tests/py3_only_tests.hy
Expand Up @@ -4,21 +4,6 @@
(import [hy._compat [PY33]])
(import [hy.errors [HyCompileError]])

(defn test-yield-from []
"NATIVE: testing yield from"

(try
(eval
'(do (defn yield-from-test []
(for* [i (range 3)]
(yield i))
(yield-from [1 2 3]))
(assert (= (list (yield-from-test)) [0 1 2 1 2 3]))))
(catch [e HyCompileError]
;; Yield-from is supported in py3.3+ only
(assert (not PY33)))
(else (assert PY33))))



(defn test-exception-cause []
Expand Down
2 changes: 1 addition & 1 deletion tox.ini
@@ -1,5 +1,5 @@
[tox]
envlist = py26,py27,pypy,py32,py33,flake8
envlist = py26,py27,pypy,py33,flake8
skipsdist = True

[testenv]
Expand Down