bpo-39988: Remove ast.AugLoad and ast.AugStore node classes.#19038
Conversation
|
@serhiy-storchaka can you remove the first commit, I think it is unrelated to this bug. |
|
This PR depends on #19037. After merging #19037 and synchronizing this PR with master the final diff will look cleaner. For now you can choose what commits you want to see in https://github.com/python/cpython/pull/19038/files. |
Co-Authored-By: Batuhan Taşkaya <47358913+isidentical@users.noreply.github.com>
| case AugLoad: | ||
| case AugStore: | ||
| break; | ||
| default: |
There was a problem hiding this comment.
Could we keep the default just in case we receive some invalid value in the switch? (The same applies with the other ones). I may be missing the reason why you remove them, tough 🤔
There was a problem hiding this comment.
It is impossible to get invalid value in ctx. compiler_nameop() is called either with a literal value or with e->v.Name.ctx, and it is validated before setting to e->v.Name.ctx.
It is more robust to not add the default because the compiler will warn if we add a new member to the expr_context_ty enum but do not have a corresponding case in a switch. If keep the default, the compiler would not catch this error at compile time.
There was a problem hiding this comment.
Gotcha, thanks for the explanation!
pablogsal
left a comment
pablogsal
left a comment
There was a problem hiding this comment.
LGTM!
Thanks Serhiy, this is a great improvement!
| VISIT(c, expr, e->v.Attribute.value); | ||
| ADDOP(c, DUP_TOP); | ||
| ADDOP_NAME(c, LOAD_ATTR, e->v.Attribute.attr, names); |
There was a problem hiding this comment.
Nice! I find this code much cleaner and straightforward :)
|
|
||
| d_bytecode = (a->a_offset - a->a_lineno_off) * sizeof(_Py_CODEUNIT); | ||
| d_lineno = i->i_lineno - a->a_lineno; | ||
| if (d_lineno == 0) |
There was a problem hiding this comment.
Nit: Pep 7 (Ignore if you want, I think this is also in the other PR)
5 participants
https://bugs.python.org/issue39988