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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:59:06 | admin | set | github: 79061 |
2019-08-25 13:11:49 | serhiy.storchaka | set | status: open -> closed resolution: fixed messages:
+ msg350452
stage: patch review -> resolved |
2019-08-25 09:44:12 | serhiy.storchaka | set | messages:
+ msg350444 |
2019-08-01 21:45:22 | arigo | set | nosy:
- arigo
|
2019-08-01 16:51:26 | ZackerySpytz | set | nosy:
+ ZackerySpytz
components:
+ Interpreter Core versions:
+ Python 3.9, - Python 3.8 |
2019-08-01 16:49:31 | ZackerySpytz | set | keywords:
+ patch stage: patch review pull_requests:
+ pull_request14820 |
2018-10-09 16:57:07 | thautwarm | set | messages:
+ msg327427 |
2018-10-08 16:46:00 | serhiy.storchaka | set | nosy:
+ serhiy.storchaka
messages:
+ msg327361 versions:
+ Python 3.8, - Python 3.6 |
2018-10-05 10:12:05 | arigo | set | nosy:
+ arigo messages:
+ msg327119
|
2018-10-03 16:10:43 | nascheme | set | status: pending -> open nosy:
+ nascheme messages:
+ msg326988
|
2018-10-03 16:02:41 | steven.daprano | set | status: closed -> pending resolution: not a bug -> (no value) messages:
+ msg326987
stage: resolved -> (no value) |
2018-10-03 15:36:57 | steven.daprano | set | messages:
+ msg326983 |
2018-10-03 13:55:59 | eric.smith | set | nosy:
+ eric.smith messages:
+ msg326969
|
2018-10-03 10:29:30 | thautwarm | set | messages:
+ msg326959 |
2018-10-03 10:01:33 | steven.daprano | set | messages:
+ msg326957 |
2018-10-03 09:52:38 | thautwarm | set | messages:
+ msg326955 |
2018-10-03 09:49:05 | thautwarm | set | nosy:
+ thautwarm messages:
+ msg326954
|
2018-10-03 07:16:44 | steven.daprano | set | status: open -> closed
nosy:
+ steven.daprano messages:
+ msg326944
resolution: not a bug stage: resolved |
2018-10-03 06:58:30 | vtheno athena | create | |