Transcrypt icon indicating copy to clipboard operation
Transcrypt copied to clipboard

context.__exit__ is not called if with block returns

Open kochelmonster opened this issue 3 years ago • 7 comments

Please transcrypt the following code:

class Context:
    def __enter__(self):
        print("enter")

    def __exit__(self, exc_type, exc_val, exc_tb):
        print("exit")
        return False


def main():
    with Context():
        print("exit is not called")
        return


document.addEventListener('DOMContentLoaded', main)

The main function ist transcripted to

export var main = function () {
	var __withid0__ = Context ();
	try {
		__withid0__.__enter__ ();
		print ('exit is not called');
		return ;
		__withid0__.__exit__ ();
	}
	catch (__except0__) {
		if (! (__withid0__.__exit__ (__except0__.name, __except0__, __except0__.stack))) {
			throw __except0__;
		}
	}
};

As you can see the return statement is generated before calling exit.

kochelmonster avatar Dec 07 '21 13:12 kochelmonster

Funny story, this kind of stuff is exactly why I coded with-as-a-function!

mentalisttraceur avatar Jun 10 '22 18:06 mentalisttraceur

Specifically, you'll want to grab the manual_no_traceback.py file, either from GitHub or the sdist package on PyPI and save it as with_.py in your Transcrypt-using Python project.

mentalisttraceur avatar Jun 10 '22 18:06 mentalisttraceur

Of course, using my with_ requires you to refactor your code a bit, so in this particular example it isn't much better than what you could do without with_.

Because both of these should work, so it arguably comes down to which you like more:

def main():
    should_return = False
    with Context():
        print("exit is called")
        should_return = True
    if should_return:
        return

or

from with_ import with_


def main():
    def _do_stuff(parameter_unused_in_this_case):
        print("exit is called")
        return True
    if with_(Context(), _do_stuff):
        return

Then again, I don't know what other broken edge cases that first one may have, but I am confident that the second one will be rock solid and exact to the PEP-343 specification because I put an unhealthy amount of time and care into making sure of it.

mentalisttraceur avatar Jun 10 '22 18:06 mentalisttraceur

More importantly, @JdeH and other maintainers are 100% welcomed to reuse any of my with_ implementation in the code they transpile with statements to!

mentalisttraceur avatar Jun 10 '22 18:06 mentalisttraceur

Some long-winded notes about the with-as-a-function workarounds.

So the implementation of my with_ that works on Pythons without a correct native with is:

def with_(manager, action):
    exit_ = type(manager).__exit__
    value = type(manager).__enter__(manager)
    try:
        result = action(value)
    except _BaseException as exception:
        if not exit_(manager, type(exception), exception, None):
            raise
        return None
    exit_(manager, None, None, None)
    return result

It will behave exactly per the PEP-343 specification, but looks different because

  1. it's using less language features to be more portable (for examples, I've seen some Python implementations have broken finally or else in their try), and
  2. it's a function that takes a callback instead of a template around an arbitrary code block, so this frees it from having to work with break and continue.

This is enough if you are willing to factor your code to work with this function-oriented style, but an arbitrary transpile of a with block needs to handle return, break, continue, yield, yield from, and await.

Now one way to handle all those control statements is to just implement all of PEP-343 in JavaScript. This would require moving closer to the implementation in the PEP and further from that implementation of with_ above.

But one simpler way to get halfway there: handle just the easy ones (return, break, and continue) by wrapping my with_.

First, the transpiler can define a few unique return values per with block:

const __withid0_done__ = Object.create(null)
const __withid0_break__ = Object.create(null)
const __withid0_continue__ = Object.create(null)
const __withid0_return__ = Object.create(null)

// For greater backwards compatibility, use `var` and
// `{}` instead of `const` and `Object.create(null)`.

Then the transpiler can

  1. wrap the with block in a function closure named for example __withid0_closure__,
  2. replace every
    • break with return [__withid0_break__, null]
    • continue with return [__withid0_continue__, null],
    • return with return [__withid0_return__, null], and
    • return ... with return [__withid0_return__, ...],
  3. replace every instance of what the context manager gives add a return [__withid0_done__, null] at the end of the closure.

Now the with itself can be transpiled into:

const __withid0_result__ = with_(__withid0__, __withid0_closure__)
if (__withid0_result__[0] === __withid0_break__)
    break
else if(__withid0_result__[0] === __withid0_continue__)
    continue
else if(__withid0_result__[0] === __withid0_return__)
    return __withid0_result__[1]

For yield and yield from, we would have to use the iterable and iterator protocol, since yielding can move data bidirectionally. In with-as-a-function I held off on supporting generators inside with because I didn't have a great vision for what that looks like, short of manually implementing yield from.

await is basically a yield from in Python, but an entirely separate thing in JavaScript, and I haven't thought much yet about what that looks like.

Having thought this through, I personally don't like not having a good plan for implementing those three control flow constructs, so I think it's worth trying to implement PEP-343 properly and fully in JavaScript, not "as a function" like I did but the more intuitive way of just pasting the with body into the middle of it, and then deciding based on the result of that effort.

But if people are happy with just supporting break, continue, and return, then I think just transpiling my with_ function and then transpiling each with statement as described in this post is a robust solution.

mentalisttraceur avatar Sep 02 '22 06:09 mentalisttraceur

More long-winded notes about the with-as-a-function workarounds.

I figured out how to extend my workaround to yield and yield from.

Write a iterator variant of with_ which wraps iteration on an iterable, use that "iwith" when the `with` body has yields.
def with_(manager, action):
    exit_ = type(manager).__exit__
    value = type(manager).__enter__(manager)
    try:
        result = (yield from action(value))
    except _BaseException as exception:
        if not exit_(manager, type(exception), exception, None):
            raise
        return None
    exit_(manager, None, None, None)
    return result

(Normally I am against naming things with cryptic single-letter abbreviations, but the use of i prefix to indicate the iterator version of something has precedent in Python's standard library - off the top of my head, at least islice from itertools.)

For clarity, here's what the with_ and iwith are implementing:

def with_(manager, action):
    with manager as value:
        return action(value)
    return None

def iwith(manager, action):
    with manager as value:
        return (yield from action(value))
    return None

Extremely simple, all that other code was just to manually reimplement the with statement behavior in a portable way.

So we add in iwith, you can also rewrite any with statement that features yield and yield from keywords.

Here's a comprehensive example:

def a_generator(foo, bar):
    while foo:
        with AContextManager(foo, 'whatever') as context:
            qux = a_function(context)
            if qux < 0:
                continue
            elif qux == 0:
                return "a string for some reason"
            foo = foo.update(qux)
            for x in an_iterator():
                if x > 20:
                    continue
                elif qux + x < 40:
                    break
            if foo < 0:
                break
            yield foo
            yield from bar
        logger.debug("this loop's foo: %r", foo)
    logger.debug("final qux: %r", qux)
    return qux

This example includes the following things in the with block:

  1. yield from
  2. yield
  3. return
  4. break (for a loop outside the with)
  5. continue (for a loop outside the with)
  6. break (for a loop inside the with)
  7. continue (for a loop inside the with)
  8. values of foo and bar from outside the with block are used
  9. use of the context manager's context value
  10. values of foo and qux are set in a way visible outside the with block

So here's one way we could rewrite it with iwait:

def a_generator(foo, bar):
    with_continue = object()
    with_break = object()
    with_return = object()
    with_done = object()
    def with_body(context):
        nonlocal foo
        nonlocal qux
        qux = a_function(context)
        if qux < 0:
            return with_continue, None
        elif qux == 0:
            return with_return, "a string for some reason"
        for x in an_iterator():
            if x > 20:
                continue
            elif qux + x < 40:
                break
        foo = foo.update(qux)
        if foo < 0:
            return with_break, None
        yield foo
        yield from bar
        return with_done, None

    while foo:
        with_control, with_return_value = (
            yield from iwith(AContextManager(foo, 'whatever'), with_body)
        )
        if with_control is with_break:
            break
        elif with_control is with_continue:
            continue
        elif with_control is with_return:
            return with_return_value
        assert with_control is with_done
    logger.debug("final qux: %r", qux)
    return qux

Though one downside I realized about "fixing" with by building on top of my with-as-a-function implementations... The transpiler would need to do a little bit of bookkeeping as it parses every with statement:

  • replace break or continue? need to know if it is inside a loop nested inside that with.
  • replace return? need to know if it inside a function definition nested within that with.
  • use iwith instead of with_? need to first find yield or yield from in the with body, and need to know if those are inside a nested function definition.
That leaves just await, async with, and async for (unless I'm missing something else)

If one of those occurs in a with block, we'd need yetmore with_ variants, which are themselves async, but I think the solution shape is the same.

The more annoying thing is the amount of permutations that are needed - both awith and aiwith variants, and each of the action, iterator and context manager could be sync or async. Since each of those variations needs a syntactical difference, if you wanted to handle it in one function, you'd need to branch on various type checks.

But in principle, the interface could require all-async things, for both manager and action and the iterator to be async, and if people need to use sync stuff in them, you can always wrap a sync thing in an async thing - still blocking, but now compatible with async code paths.

mentalisttraceur avatar Sep 05 '22 07:09 mentalisttraceur

I would fix the code generation like this:

	var __withid0__ = Context ();
        var __withid0_threw__ = false;
	try {
		__withid0__.__enter__ ();
		// with body
	}
	catch (__except0__) {
                __withid0_threw__ = true;
		if (! (__withid0__.__exit__ (__except0__.name, __except0__, __except0__.stack))) {
			throw __except0__;
		}
	}
        finally {
                if (! __withid0_threw__) {
                        __withid0__.__exit__ ();
                }
        }

mentalisttraceur avatar Sep 05 '22 09:09 mentalisttraceur