git.net

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Python-Dev] [PEP 558] thinking through locals() semantics


I re-ran your examples and found that some of them fail.

On Mon, May 27, 2019 at 8:17 AM Nathaniel Smith <njs at pobox.com> wrote:

> First, I want to say: I'm very happy with PEP 558's changes to
> f_locals. It solves the weird threading bugs, and exposes the
> fundamental operations you need for debugging in a simple and clean
> way, while leaving a lot of implementation flexibility for future
> Python VMs. It's a huge improvement over what we had before.
>
> I'm not as sure about the locals() parts of the proposal. It might be
> fine, but there are some complex trade-offs here that I'm still trying
> to wrap my head around. The rest of this document is me thinking out
> loud to try to clarify these issues.
>
>
> ##### What are we trying to solve?
>
> There are two major questions, which are somewhat distinct:
> - What should the behavior of locals() be in CPython?
> - How much of that should be part of the language definition, vs
> CPython implementation details?
>
> The status quo is that for locals() inside function scope, the
> behavior is quite complex and subtle, and it's entirely implementation
> defined. In the current PEP draft, there are some small changes to the
> semantics, and also it promotes them becoming part of the official
> language semantics.
>
> I think the first question, about semantics, is the more important
> one. If we're promoting them to the language definition, the main
> effect is just to make it more important we get the semantics right.
>
>
> ##### What are the PEP's proposed semantics for locals()?
>
> They're kinda subtle. [Nick: please double-check this section, both
> for errors and because I think it includes some edge cases that the
> PEP currently doesn't mention.]
>
> For module/class scopes, locals() has always returned a mapping object
> which acts as a "source of truth" for the actual local environment ?
> mutating the environment directly changes the mapping object, and
> vice-versa. That's not going to change.
>
> In function scopes, things are more complicated. The *local
> environment* is conceptually well-defined, and includes:
> - local variables (current source of truth: "fast locals" array)
> - closed-over variables (current source of truth: cell objects)
> - any arbitrary key/values written to frame.f_locals that don't
> correspond to local or closed-over variables, e.g. you can do
> frame.f_locals[object()] = 10, and then later read it out again.
>
> However, the mapping returned by locals() does not directly reflect
> this local environment. Instead, each function frame has a dict
> associated with it. locals() returns this dict. The dict always holds
> any non-local/non-closed-over variables, and also, in certain
> circumstances, we write a snapshot of local and closed-over variables
> back into the dict.
>
> Specifically, we write back:
>
> - Whenever locals() is called
> - Whenever exec() or eval() is called without passing an explicit
> locals argument
> - After every trace/profile event, if a Python-level tracing/profiling
> function is registered.
>
> (Note: in CPython, the use of Python-level tracing/profiling functions
> is extremely rare. It's more common in alternative implementations
> like PyPy. For example, the coverage package uses a C-level tracing
> function on CPython, which does not trigger locals updates, but on
> PyPy it uses a Python-level tracing function, which does trigger
> updates.)
>
> In addition, the PEP doesn't say, but I think that any writes to
> f_locals immediately update both the environment and the locals dict.
>
> These semantics have some surprising consequences. Most obviously, in
> function scope (unlike other scopes), mutating locals() does not
> affect the actual local environment:
>
>   def f():
>       a = 1
>       locals()["a"] = 2
>       assert a == 1
>
> The writeback rules can also produce surprising results:
>
>   def f():
>       loc1 = locals()
>       # Since it's a snapshot created at the time of the call
>       # to locals(), it doesn't contain 'loc1':
>       assert "loc1" not in loc1
>       loc2 = locals()
>       # Now loc1 has changed:
>       assert "loc1" in loc1
>
> However, the results here are totally different if a Python-level
> tracing/profiling function is installed ? in particular, the first
> assertion fails.
>
> The interaction between f_locals and and locals() is also subtle:
>
>   def f():
>       a = 1
>       loc = locals()
>       assert "loc" not in loc
>       # Regular variable updates don't affect 'loc'
>       a = 2
>       assert loc["a"] == 1
>       # But debugging updates do:
>       sys._getframe().f_locals["a"] = 3
>       assert a == 3
>

That assert fails; `a` is still 2 here for me.


>       assert loc["a"] == 3
>       # But it's not a full writeback
>       assert "loc" not in loc
>       # Mutating 'loc' doesn't affect f_locals:
>       loc["a"] = 1
>       assert sys._getframe().f_locals["a"] == 1
>       # Except when it does:
>       loc["b"] = 3
>       assert sys._getframe().f_locals["b"] == 3
>

All of this can be explained by realizing `loc is
sys._getframe().f_locals`. IOW locals() always returns the dict in f_locals.


> Again, the results here are totally different if a Python-level
> tracing/profiling function is installed.
>
> And you can also hit these subtleties via 'exec' and 'eval':
>
>   def f():
>       a = 1
>       loc = locals()
>       assert "loc" not in loc
>       # exec() triggers writeback, and then mutates the locals dict
>       exec("a = 2; b = 3")
>       # So now the current environment has been reflected into 'loc'
>       assert "loc" in loc
>       # Also loc["a"] has been changed to reflect the exec'ed assignments
>       assert loc["a"] == 2
>       # But if we look at the actual environment, directly or via
>       # f_locals, we can see that 'a' has not changed:
>       assert a == 1
>       assert sys._getframe().f_locals["a"] == 1
>       # loc["b"] changed as well:
>       assert loc["b"] == 3
>       # And this *does* show up in f_locals:
>       assert sys._getframe().f_locals["b"] == 3
>

This works indeed. My understanding is that the bytecode interpreter, when
accessing the value of a local variable, ignores f_locals and always uses
the "fast" array. But exec() and eval() don't use fast locals, their code
is always compiled as if it appears in a module-level scope.

While the interpreter is running and no debugger is active, in a function
scope f_locals is not used at all, the interpreter only interacts with the
fast array and the cells. It is initialized by the first locals() call for
a function scope, and locals() copies the fast array and the cells into it.
Subsequent calls in the same function scope keep the same value for
f_locals and re-copy fast and cells into it. This also clears out deleted
local variables and emptied cells, but leaves "strange" keys (like "b" in
the examples) unchanged.

The truly weird case happen when Python-level tracers are present, then the
contents of f_locals is written back to the fast array and cells at certain
points. This is intended for use by pdb (am I the only user of pdb left in
the world?), so one can step through a function and mutate local variables.
I find this essential in some cases.


> Of course, many of these edge cases are pretty obscure, so it's not
> clear how much they matter. But I think we can at least agree that
> this isn't the one obvious way to do it :-).
>
>
> ##### What's the landscape of possible semantics?
>
> I did some brainstorming, and came up with 4 sets of semantics that
> seem plausible enough to at least consider:
>
> - [PEP]: the semantics in the current PEP draft.
>

To be absolutely clear this copies the fast array and cells to f_locals
when locals() is called, but never copies back, except when Python-level
tracing/profiling is on.


> - [PEP-minus-tracing]: same as [PEP], except dropping the writeback on
> Python-level trace/profile events.
>

But this still copies the fast array and cells to f_locals when a Python
trace function is called, right? It just doesn't write back.


> - [snapshot]: in function scope, each call to locals() returns a new,
> *static* snapshot of the local environment, removing all this
> writeback stuff. Something like:
>
>   def locals():
>       frame = get_caller_frame()
>       if is_function_scope(frame):
>           # make a point-in-time copy of the "live" proxy object
>           return dict(frame.f_locals)
>       else:
>           # in module/class scope, return the actual local environment
>           return frame.f_locals
>

This is the most extreme variant, and in this case there is no point in
having f_locals at all for a function scope (since nothing uses it). I'm
not 100% sure that you understand this.


> - [proxy]: Simply return the .f_locals object, so in all contexts
> locals() returns a live mutable view of the actual environment:
>
>   def locals():
>       return get_caller_frame().f_locals
>

So this is PEP without any writeback. But there is still copying from the
fast array and cells to f_locals. Does that only happen when locals() is
called? Or also when a Python-level trace/profiling function is called?

My problem with all variants except what's in the PEP is that it would
leave pdb *no* way (short of calling into the C API using ctypes) of
writing back local variables.


>
> ##### How to evaluate our options?
>
> I can think of a lot of criteria that all-else-being-equal we would
> like Python to meet. (Of course, in practice they conflict.)
>
> Consistency across APIs: it's surprising if locals() and
> frame.f_locals do different things. This argues for [proxy].
>
> Consistency across contexts: it's surprising if locals() has acts
> differently in module/class scope versus function scope. This argues
> for [proxy].
>
> Consistent behavior when the environment shifts, or small maintenance
> changes are made: it's nice if code that works today keeps working
> tomorrow. On this criterion, I think [snapshot] > [proxy] >
> [PEP-minus-tracing] >>> [PEP]. [PEP] is particularly bad here because
> a very rare environmental change that almost no-one tests and mostly
> only happens when debugging (i.e., enabling tracing) causes a radical
> change in semantics.
>
> Simplicity of explaining to users: all else being equal, it's nice if
> our docs are short and clear and the language fits in your head. On
> this criterion, I think: [proxy] > [snapshot] > [PEP-minus-tracing] >
> [PEP]. As evidence that the current behavior is confusing, see:
>
> - Ned gets confused and writes a long blog post after he figures it
> out: https://nedbatchelder.com/blog/201211/tricky_locals.html
> - A linter that warns against mutating locals():
> https://lgtm.com/rules/10030096/
>
> Simplicity of implementation: "If the implementation is easy to
> explain, it may be a good idea." Since we need the proxy code anyway
> to implement f_locals, I think it's: [proxy] (free) > [snapshot] (one
> 'if' statement) > ([PEP] = [PEP-minus-tracing]).
>
> Impact on other interpreter implementations: all else being equal,
> we'd like to give new interpreters maximal freedom to do clever
> things. (And local variables are a place where language VMs often
> expend a lot of cleverness.) [proxy] and [snapshot] are both easily
> implemented in terms of f_locals, so they basically don't constrain
> alternative implementations at all. I'm not as sure about [PEP] and
> [PEP-minus-tracing]. I originally thought they must be horrible. On
> further thought, I'm not convinced they're *that* bad, since the need
> to support people doing silly stuff like frame.f_locals[object()] = 10
> means that implementations will already need to sometimes attach
> something like a dict object to their function frames. But perhaps
> alternative implementations would like to disallow this, or are OK
> with making it really slow but care about locals() performance more.
> Anyway, it's definitely ([proxy] = [snapshot]) > ([PEP] =
> [PEP-minus-tracing]), but I'm not sure whether the '>' is large or
> small.
>
> Backwards compatibility: help(locals) says:
>
>   NOTE: Whether or not updates to this dictionary will affect
>   name lookups in the local scope and vice-versa is
>   *implementation dependent* and not covered by any backwards
>   compatibility guarantees.
>
> So that claims that there are ~no backwards compatibility issues here.
> I'm going to ignore that; no matter what the docs say, we still don't
> want to break everyone's code. And unfortunately, I can't think of any
> realistic way to do a gradual transition, with like deprecation
> warnings and all that (can anyone else?), so whatever we do will be a
> flag-day change.
>
> Of our four options, [PEP] is intuitively the closest to what CPython
> has traditionally done. But what exactly breaks under the different
> approaches? I'll split this off into its own section.
>
>
> ##### Backwards compatibility
>
> I'll split this into three parts: code that treats locals() as
> read-only, exec()/eval(), and code that mutates locals().
>
> I believe (but haven't checked) that the majority of uses of locals()
> are in simple cases like:
>
>   def f():
>       ....
>       print("{a} {b}".format(**locals()))
>
> Luckily, this code remains totally fine under all four of our candidates.
>
> exec() and eval() are an interesting case. In Python 2, exec'ing some
> assignments actually *did* mutate the local environment, e.g. you
> could do:
>
>   # Python 2
>   def f():
>       exec "a = 1"
>       assert a == 1
>
> In Python 3, this was changed, so now exec() inside a function cannot
> mutate the enclosing scope. We got some bug reports about this change,
> and there are a number of questions on stackoverflow about it, e.g.:
>
> - https://bugs.python.org/issue4831
> -
> https://stackoverflow.com/questions/52217525/how-can-i-change-the-value-of-variable-in-a-exec-function
> -
> https://stackoverflow.com/questions/50995581/eval-exec-with-assigning-variable-python
>
> In all released versions of Python, eval() was syntactically unable to
> rebind variables, so eval()'s interaction with the local environment
> was undefined. However, in 3.8, eval() *will* be able to rebind
> variables using the ':=' operator, so this interaction will become
> user-visible. Presumably we'll want eval() to match exec().
>
> OK, with that background out of the way, let's look at our candidates.
>
> If we adopt [proxy], then that will mean exec()-inside-functions will
> go back to the Python 2 behavior, where executing assignments in the
> enclosing scope actually changes the enclosing scope. This will likely
> break some code out there that's relying on the Python 3 behavior,
> though I don't know how common that is. (I'm guessing not too common?
> Using the same variable inside and outside an 'exec' and trusting that
> they *won't* be the same seems like an unusual thing to do. But I
> don't know.)
>
> With [PEP] and [PEP-minus-tracing], exec() is totally unchanged. With
> [snapshot], there's technically a small difference: if you call
> locals() and then exec(), the exec() no longer triggers an implicit
> writeback to the dict that locals() returned. I think we can ignore
> this, and say that for all three of these, exec() is unlikely to
> produce backwards compatibility issues.
>
> OK, finally, let's talk about code that calls locals() and then
> mutates the return value. The main difference between our candidates
> is how they handle mutation, so this seems like the most important
> case to focus on.
>
> Conveniently, Mark Shannon and friends have statically analyzed a
> large corpus of Python code and made a list of cases where people do
> this: https://lgtm.com/rules/10030096/alerts/
> Thanks! I haven't gone through the whole list, but I read through the
> first few in the hopes of getting a better sense of what kind of code
> does this in the real world and how it would be impacted by our
> different options.
>
>
> https://lgtm.com/projects/g/pydata/xarray/snapshot/a2ac6af744584c8afed3d56d00c7d6ace85341d9/files/xarray/plot/plot.py?sort=name&dir=ASC&mode=heatmap#L701
> Current: raises if a Python-level trace/profile function is set
> [PEP]: raises if a Python-level trace/profile function is set
> [PEP-minus-tracing]: ok
> [snapshot]: ok
> [proxy]: always raises
> Comment: uses locals() to capture a bunch of passed in kwargs so it
> can pass them as **kwargs to another function, and treats them like a
> snapshot. The authors were clearly aware of the dangers, because this
> pattern appears multiple times in this file, and all the other places
> make an explicit copy of locals() before using it, but this place
> apparently got missed. Fix is trivial: just do that here too.
>
>
> https://github.com/swagger-api/swagger-codegen/blob/master/modules/swagger-codegen/src/main/resources/python/api.mustache
> Current: ok
> [PEP]: ok
> [PEP-minus-tracing]: ok
> [snapshot]: ok
> [proxy]: ok
> Comment: this is inside a tool used to generate Python wrappers for
> REST APIs. The vast majority of entries in the lgtm database are from
> code generated by this tool. This was tricky to analyze, because it's
> complex templated code, and it does mutate locals(). But I'm pretty
> confident that the mutations end up just... not mattering, and all
> possible generated code works under all of our candidates. Which is
> lucky, because if we did have to fix this it wouldn't be trivial:
> fixing up the code generator itself wouldn't be too hard, but it'll
> take years for everyone to regenerate their old wrappers.
>
>
> https://lgtm.com/projects/g/saltstack/salt/snapshot/bb0950e5eafbb897c8e969e3f20fd297d8ba2006/files/salt/utils/thin.py?sort=name&dir=ASC&mode=heatmap#L258
> Current: ok
> [PEP]: ok
> [PEP-minus-tracing]: ok
> [snapshot]: raises
> [proxy]: ok
> Comment: the use of locals() here is totally superfluous ? it
> repeatedly reads and writes to locals()[mod], and in all cases this
> could be replaced by a simple variable, or any other dict. And it's
> careful not to assign to any name that matches an actual local
> variable, so it works fine with [proxy] too. But it does assume that
> multiple calls to locals() return the same dict, so [snapshot] breaks
> it. In this case the fix is trivial: just use a variable.
>
>
> https://lgtm.com/projects/g/materialsproject/pymatgen/snapshot/fd6900ed1040a4d35f2cf2b3506e6e3d7cdf77db/files/pymatgen/ext/jhu.py?sort=name&dir=ASC&mode=heatmap#L52
> Current: buggy if a trace/profile function is set
> [PEP]: buggy if a trace/profile function is set
> [PEP-minus-tracing]: ok
> [snapshot]: ok
> [proxy]: raises
> Comment: Another example of collecting kwargs into a dict. Actually
> this code is always buggy, because they seem to think that
> dict.pop("a", "b") removes both "a" and "b" from the dict... but it
> would raise an exception on [proxy], because they use one of those
> local variables after popping it from locals(). Fix is trivial: take
> an explicit snapshot of locals() before modifying it.
>
>
> ##### Conclusion so far
>
> [PEP-minus-tracing] seems to strictly dominate [PEP]. It's equal or
> better on all criteria, and actually *more* compatible with all the
> legacy code I looked at, even though it's technically less consistent
> with what CPython used to do. Unless someone points out some new
> argument, I think we can reject the writeback-when-tracing part of the
> PEP draft.
>
> Choosing between the remaining three is more of a judgement call.
>
> I'm leaning towards saying that on net, [snapshot] beats
> [PEP-minus-tracing]: it's dramatically simpler, and the backwards
> incompatibilities that we've found so far seem pretty minor, on par
> with what we do in every point release. (In fact, in 3/4 of the cases
> I looked at, [snapshot] is actually what users seemed to trying to use
> in the first place.)
>
> For [proxy] versus [snapshot], a lot depends on what we think of
> changing the semantics of exec(). [proxy] is definitely more
> consistent and elegant, and if we could go back in time I think it's
> what we'd have done from the start. Its compatibility is maybe a bit
> worse than [snapshot] on non-exec() cases, but this seems pretty minor
> overall (it often doesn't matter, and if it does just write
> dict(locals()) instead of locals(), like you would in non-function
> scope). But the change in exec() semantics is an actual language
> change, even though it may not affect much real code, so that's what
> stands out for me.
>
> I'd very much like to hear about any considerations I've missed, and
> any opinions on the "judgement call" part.
>

To me it looks as if you're working with the wrong mental model of how it
currently works. Unfortunately there's no way to find out how it currently
works without reading the source code, since from Python code you cannot
access the fast array and cells directly, and by just observing f_locals
you get a slightly wrong picture.

Perhaps we should give the two operations that copy from/to the fast array
and cells names, so we can talk about them more easily.

Sorry I didn't snip stuff.

-- 
--Guido van Rossum (python.org/~guido)
*Pronouns: he/him/his **(why is my pronoun here?)*
<http://feministing.com/2015/02/03/how-using-they-as-a-singular-pronoun-can-change-the-world/>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/python-dev/attachments/20190527/b6f6ba58/attachment-0001.html>