You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Wanted to record this, not completely ready to declare it a "bug" yet.
In theory, an OverrideEnvironment is a lightweight copy-on-write proxy, so a builder call can be passed a few environment vars that differ from the active one - certainly a lot cheaper than cloning and modifying and env for that one call. The builder thinks it's getting a regular environment; the proxy serves up those values that have been set in the override, and if they haven't, fetches them from the base. Overrides can even be stacked, although not sure if I see much of a need for that.
However, the override can "leak through" to the base environment if something using it modifies the environment it's passed, and it happens to be an override. The proxy catches assignment through its __setitem__ method, so those are okay, e.g.
oenv['XXX'] ="x2"
will just put that value into the override dict, and the base is unchanged. It cannot, however, catch modification of the objects retrieved from the base if those objects are mutable. Thus, if CPPDEFINES in env is a list, as it often is, then
oenv.Append(CPPDEFINES='DEBUG']
will happily modify the CPPDEFINES entry in `env'.
The implementation also intentionally lets deletions leak through, which seems wrong. The __delitem__ method removes from the override, and then also removes from the base, and the unittests enforce that this behavior was expected (or at least, the test was coded to match the implementation). Deletion is an interesting case: if you're working with an env, and you deleted something from it, it should no longer be there. But the __getitem__ method will backfill from the base, and the __contains__ method will reach through for its answer, too. So you'd have the case where you deleted something, and it still seems to exist. The shallow answer was to delete it from the base too - then it looks deleted, but that action has affected the base.
Some of this can be prevented by a kind of ham-fisted approach: don't allow writing (or deleting) to an override, turn it into an init-only class. That seems to be its main intent, anyway. There's even new-ish Python feature that could help - types.MappingProxyType, which raises errors if you try to modify the base. But it doesn't fix the deepcopy problem (mutable container types are not protected).
It's tempting to say scanners, emitters and action functions should not modify an override. Maybe they shouldn't modify any env. But it's hard to see how to detect/enforce that.
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
-
Wanted to record this, not completely ready to declare it a "bug" yet.
In theory, an
OverrideEnvironment
is a lightweight copy-on-write proxy, so a builder call can be passed a few environment vars that differ from the active one - certainly a lot cheaper than cloning and modifying and env for that one call. The builder thinks it's getting a regular environment; the proxy serves up those values that have been set in the override, and if they haven't, fetches them from the base. Overrides can even be stacked, although not sure if I see much of a need for that.However, the override can "leak through" to the base environment if something using it modifies the environment it's passed, and it happens to be an override. The proxy catches assignment through its
__setitem__
method, so those are okay, e.g.will just put that value into the override dict, and the base is unchanged. It cannot, however, catch modification of the objects retrieved from the base if those objects are mutable. Thus, if
CPPDEFINES
inenv
is a list, as it often is, thenwill happily modify the
CPPDEFINES
entry in `env'.The implementation also intentionally lets deletions leak through, which seems wrong. The
__delitem__
method removes from the override, and then also removes from the base, and the unittests enforce that this behavior was expected (or at least, the test was coded to match the implementation). Deletion is an interesting case: if you're working with an env, and you deleted something from it, it should no longer be there. But the__getitem__
method will backfill from the base, and the__contains__
method will reach through for its answer, too. So you'd have the case where you deleted something, and it still seems to exist. The shallow answer was to delete it from the base too - then it looks deleted, but that action has affected the base.Some of this can be prevented by a kind of ham-fisted approach: don't allow writing (or deleting) to an override, turn it into an init-only class. That seems to be its main intent, anyway. There's even new-ish Python feature that could help -
types.MappingProxyType
, which raises errors if you try to modify the base. But it doesn't fix the deepcopy problem (mutable container types are not protected).It's tempting to say scanners, emitters and action functions should not modify an override. Maybe they shouldn't modify any env. But it's hard to see how to detect/enforce that.
Beta Was this translation helpful? Give feedback.
All reactions