This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: About the "assert" bytecode
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.9
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: ZackerySpytz, eric.smith, nascheme, serhiy.storchaka, steven.daprano, thautwarm, vtheno athena
Priority: normal Keywords: patch

Created on 2018-10-03 06:58 by vtheno athena, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 15073 merged ZackerySpytz, 2019-08-01 16:49
Messages (15)
msg326942 - (view) Author: vtheno athena (vtheno athena) Date: 2018-10-03 06:58
When I was involved in the YaPyPy project, I found a problem, An source "assert 0" will compile to an bytecode, On that bytecode sequence, it always raise a "AssertionError" from global, when we rewrite global "AssertionError" like here:
```
class AssertionError(Exception):
    def __init__(self,msg):
        self.msg = f"User AssertionError: {msg}"
        # other code
``` 
so, "assert" is meta-programming?
msg326944 - (view) Author: Steven D'Aprano (steven.daprano) * (Python committer) Date: 2018-10-03 07:16
If I have understood you correctly, you are asking whether it is expected behaviour for assert to use the global AssertionError instead of the built-in AssertionError.

I don't see why it shouldn't. In any case, that behaviour goes back to Python 1.5 and perhaps older.

I don't think this is a bug, so I'm going to close this. If you disagree, please re-open it with more detail explaining why you think it is a bug.
msg326954 - (view) Author: thautwarm (thautwarm) * Date: 2018-10-03 09:49
Steven, this problem is quite interesting because it just allows users to cause fatal errors without any invalid operations.

```
AssertionError = 1
assert 1 == 2, 2
---------------------------------------------------------------------------
TypeError: 'int' object is not callable

```

It's better to deal with it seriously.
msg326955 - (view) Author: thautwarm (thautwarm) * Date: 2018-10-03 09:52
If we could generate `LOAD_CONST` instructions for `assert` statements, any prospective dangers could be avoided.


```
from dis import dis
@dis
def f():
    assert x
    
  3           0 LOAD_GLOBAL              0 (x)
              2 POP_JUMP_IF_TRUE         8
              4 LOAD_GLOBAL              1 (AssertionError)
              6 RAISE_VARARGS            1
        >>    8 LOAD_CONST               0 (None)
             10 RETURN_VALUE
```
msg326957 - (view) Author: Steven D'Aprano (steven.daprano) * (Python committer) Date: 2018-10-03 10:01
On Wed, Oct 03, 2018 at 09:49:06AM +0000, thautwarm wrote:
> Steven, this problem is quite interesting because it just allows users 
> to cause fatal errors without any invalid operations.

How is that different from every other case of shadowing a builtin?

len = 45
print(len("hello world"))

The ability to shadow builtins is a feature, not a bug.

I have no specific objection to changing assert so that it raises the 
actual honest-to-goodness AssertionError, but that would be an 
enhancement, not a bug-fix.

(To be honest, that's what I assumed it would do, until I tried it.)
msg326959 - (view) Author: thautwarm (thautwarm) * Date: 2018-10-03 10:29
Reply to this:
> How is that different from every other case of shadowing a builtin?
> 
> len = 45
> print(len("hello world"))

```
AssertionError = 42
assert 1 != 2
```

`assert` implicitly invokes `AssertionError`, while `len` does that explicitly.  That is to say, simply changing a global variable breaks the work of a keyword.

Another difference is that shadow builtins could be resumed in the 
nested functions without something like `globals()` or `exec(..., {})`, while you cannot perform this to the breakage of `assert`:

```

len = 1
def g():
   from builtins import len

   return len([1, 2, 3])

g() # => 3

AssertionError = +1

def f():
   from builtins import AssertionError
   assert False

f() # boooom


```
msg326969 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2018-10-03 13:55
I think this is a bug that should be fixed. This is similar to how f-strings used to work: the generated byte code would call format(something-or-other), and if you'd defined a name "format" in your code it would fail.

Now admittedly "format" is more common than "AssertionError", but in any event I don't think assert should fail because of a name you happen to be using. That's a surprising action-at-a-distance, in my mind.

And I especially think that's true in the case of assert: when an assert fires, the last thing I want is something that I normally wouldn't have tested for causing me to not see what the assertion is.

And, I think a broader discussion on python-dev might be useful, too, in order to get more opinions.
msg326983 - (view) Author: Steven D'Aprano (steven.daprano) * (Python committer) Date: 2018-10-03 15:36
On Wed, Oct 03, 2018 at 01:56:00PM +0000, Eric V. Smith wrote:
> I think this is a bug that should be fixed.

Supporting this position, shadowing other exceptions doesn't change the 
exception generated by the interpreter:

py> TypeError = None
py> 1 + "a"
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: unsupported operand type(s) for +: 'int' and 'str'

On the other hand, this has been the behaviour going back to Python 1.5, 
it is hard to see why this is worse than any other example of shadowing. 
I don't think there's anything in the documentation that says that 
assert *shouldn't* do a LOAD_GLOBAL on AssertionError.

Hence this would be an enhancement rather than a bug fix.

> And, I think a broader discussion on python-dev might be useful, too, 
> in order to get more opinions.

I agree. I think we need to clarify the intent here, and then decide 
that if it is a bug, should we bother fixing it in pre-3.8 versions?
msg326987 - (view) Author: Steven D'Aprano (steven.daprano) * (Python committer) Date: 2018-10-03 16:02
> And, I think a broader discussion on python-dev might be useful, too, in order to get more opinions.

Done:

https://mail.python.org/pipermail/python-dev/2018-October/155410.html

Changing the status to Pending until we have more clarity on whether this is a bug, feature, accident or whatnot.
msg326988 - (view) Author: Neil Schemenauer (nascheme) * (Python committer) Date: 2018-10-03 16:10
Assuming it is not crazy complicated to fix, I would like to to be changed.  It should work like the TypeError example.  That fact that it has worked the current way since Python 1.5 isn't a strong argument.  I think no one should be surprised if it changes.  Also, expecting other Python implementations to match the LOAD_GLOBAL behavior seems to put unnecessary burden on them.  If someone writes code that relies on that behavior, they should not be surprised if it gets broken, IMHO.
msg327119 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2018-10-05 10:12
A middle ground might be to copy the behavior of ``__import__``: it is loaded from the builtins module where specific hacks can change its definition, but it is not loaded from the globals.
msg327361 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-10-08 16:46
This can't be changed in 3.6. There are two ways of changing it:

1. Make the marshal module supporting AssertionError as it supports booleans, None, Ellipsis and StopIteration. This will require adding the new marshal format version.

2. Add a new opcode in bytecode.

The latter way is easier. Bytecode is changed in every feature release, and it was already changed in 3.8. New marshal versions are added less often.

There was similar issue with StopAsyncIteration. See issue33041.
msg327427 - (view) Author: thautwarm (thautwarm) * Date: 2018-10-09 16:57
Hi, Serhiy, there could be an another way to fix all this sort of problems IMO.

We can figure out all the cases that implicitly require shadow builtins, and then change symtable visitor to add corresponding free variables with name mangling, for instance:

1. implicitly add free variable ".AssertionError"

```
def f():
   assert 1
```

where ".AssertionError" is a name-mangled free variable and is assigned once the module is executed.

The same to `StopAsyncIteration`, `TypeError`,  `__build_class__` and so on.
msg350444 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-08-25 09:44
New changeset ce6a070414ed1e1374d1e6212bfbff61b6d5d755 by Serhiy Storchaka (Zackery Spytz) in branch 'master':
bpo-34880: Add the LOAD_ASSERTION_ERROR opcode. (GH-15073)
https://github.com/python/cpython/commit/ce6a070414ed1e1374d1e6212bfbff61b6d5d755
msg350452 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-08-25 13:11
Thank you Zackery for your contribution.

> where ".AssertionError" is a name-mangled free variable and is assigned once the module is executed.

But this still allows to overriding builtin AssertionError before importing a module. And this solution would be much more complex that adding a new opcode.
History
Date User Action Args
2022-04-11 14:59:06adminsetgithub: 79061
2019-08-25 13:11:49serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg350452

stage: patch review -> resolved
2019-08-25 09:44:12serhiy.storchakasetmessages: + msg350444
2019-08-01 21:45:22arigosetnosy: - arigo
2019-08-01 16:51:26ZackerySpytzsetnosy: + ZackerySpytz

components: + Interpreter Core
versions: + Python 3.9, - Python 3.8
2019-08-01 16:49:31ZackerySpytzsetkeywords: + patch
stage: patch review
pull_requests: + pull_request14820
2018-10-09 16:57:07thautwarmsetmessages: + msg327427
2018-10-08 16:46:00serhiy.storchakasetnosy: + serhiy.storchaka

messages: + msg327361
versions: + Python 3.8, - Python 3.6
2018-10-05 10:12:05arigosetnosy: + arigo
messages: + msg327119
2018-10-03 16:10:43naschemesetstatus: pending -> open
nosy: + nascheme
messages: + msg326988

2018-10-03 16:02:41steven.dapranosetstatus: closed -> pending
resolution: not a bug -> (no value)
messages: + msg326987

stage: resolved -> (no value)
2018-10-03 15:36:57steven.dapranosetmessages: + msg326983
2018-10-03 13:55:59eric.smithsetnosy: + eric.smith
messages: + msg326969
2018-10-03 10:29:30thautwarmsetmessages: + msg326959
2018-10-03 10:01:33steven.dapranosetmessages: + msg326957
2018-10-03 09:52:38thautwarmsetmessages: + msg326955
2018-10-03 09:49:05thautwarmsetnosy: + thautwarm
messages: + msg326954
2018-10-03 07:16:44steven.dapranosetstatus: open -> closed

nosy: + steven.daprano
messages: + msg326944

resolution: not a bug
stage: resolved
2018-10-03 06:58:30vtheno athenacreate