[Python-ideas] Make "yield" inside a with statement a SyntaxError

[Jacco van Dorp]

I don't think this is a major problem. In this case, the file will be
closed when the generator is garbage collected. So you'd also have to
leak the generator to actually get this problem. And if leaking
generators won't harm your application, neither will leaking the
occasional file handle.

Also, I think "Yielding inside a with block, therefore, doesn't make
sense and can only lead to obscure bugs." is somewhat of an
overstatement.

Also, the problem isn't the with block. The cause of the "issue" you
describe is that a with statement is a try...finally in disguise. With
the finally clause being guaranteed to be executed. IIRC, but I can't
remember where, I remember a statement from guide referencing this,
and calling it an issue that any fix will cause far more pain than the
issue ever will. Or something to that point.

2018-08-08 8:32 GMT+02:00 Chris Angelico <rosuav at gmail.com>:
> On Wed, Aug 8, 2018 at 4:14 PM, Ken Hilton <kenlhilton at gmail.com> wrote:
>> This mostly springs off of a comment I saw in some thread.
>>
>> The point of a with statement is that it ensures that some resource will be
>> disposed of, yes? For example, this:
>>
>>     with open(filename) as f:
>>         contents = f.read()
>>
>> is better than this:
>>
>>     contents = open(filename).read()
>>
>> because the former definitely closes the file while the latter relies on
>> garbage collection?
>
> Technically, the 'with' statement ensures that the file will be closed
> before the line of code following it is run. So in this example:
>
>     with open(filename, "w") as f:
>         f.write(...)
>     os.rename(filename, target_filename)
>
> you have a guarantee that the file is closed prior to the rename call.
>
>> Now, let's take a look at the following scenario:
>>
>>     def read_multiple(*filenames):
>>         for filename in filenames:
>>             with open(filename) as f:
>>                 yield f.read()
>>
>> Can you spot the problem? The "with open(filename)" statement is supposed to
>> ensure that the file object is disposed of properly. However, the "yield
>> f.read()" statement suspends execution within the with block, so if this
>> happened:
>>
>>     for contents in read_multiple('chunk1', 'chunk2', 'chunk3'):
>>         if contents == 'hello':
>>             break
>>
>> and the contents of "chunk2" were "hello" then the loop would exit, and
>> "chunk2" would never be closed! Yielding inside a with block, therefore,
>> doesn't make sense and can only lead to obscure bugs.
>
> This is only a problem if you consider it to be one. The value of the
> 'with' statement is not destroyed; for example, you're capped at _one_
> open file (whereas without the context manager, it's entirely possible
> for file-open in a loop to leak a large number of handles).
>
>> I believe all possible cases where one would yield inside a context manager
>> can be covered by saving anything required from the context manager and then
>> yielding the results outside. Therefore, I propose making a "yield" inside a
>> with block become a SyntaxError.
>
> What about this:
>
>     def read_words(*filenames):
>          for filename in filenames:
>              with open(filename) as f:
>                  for line in f:
>                      yield from line.split()
>
> It'll read from a series of files and yield individual words (ignoring
> annoying little things like punctuation and hyphenation for the sake
> of simplicity). You are assuming that every yield-inside-with is a
> *single* yield.
>
>> This means the first "read_multiple" definition I presented will become
>> illegal and fail at compile-time. However, it is still legal to define a
>> generator inside a with block:
>>
>>     def pass_file_chars(oldfunc):
>>         with open('secretfile') as f:
>>             contents = f.read()
>>             @functools.wraps
>>             def newfunc(*args, **kwargs):
>>                 for char in contents:
>>                     yield oldfunc(char, *args, **kwargs)
>>         return newfunc
>>
>> This is probably a bad example, but I hope it still explains why it should
>> be legal to define generators in context managers - as long as the with
>> block serves its purpose correctly, everything else should still work
>> normally.
>
> I have no idea what this is supposed to be doing, nor why you're
> defining the function this way. Perhaps a better example will
> illustrate your point more clearly?
>
>> For those concerned about backwards compatibility: I believe that those who
>> attempt to yield inside a context manager will already discover that results
>> are undefined when doing so; this will simply make it more obvious that
>> suspending execution in a with block is not meant to happen, and convert
>> undefined behavior into a straight-up SyntaxError.
>
> The current results are most certainly not undefined, and you're
> attempting to fix something which is not a problem.
>
> If you really find yourself running into situations like this, program
> your linter to warn you when you do it.
>
> ChrisA
> _______________________________________________
> Python-ideas mailing list
> Python-ideas at python.org
> https://mail.python.org/mailman/listinfo/python-ideas
> Code of Conduct: http://python.org/psf/codeofconduct/