bpo-32911: Remove the docstring attribute of AST types #7121
Conversation
There was a problem hiding this comment.
Thanks for this Serhiy. In addition to the b5 NEWS entry, the only other item I see missing here is a magic number bump for the bytecode (so any debug and opt-level 1 bytecode generated with the earlier alphas and betas gets regenerated with docstrings included again).
|
When you're done making the requested changes, leave the comment: |
|
Note that while the previous 3.7.0a1 NEWS entry is modified by the current patch, there'll need to be a dedicated NEWS entry for the reversion that will then appear under 3.7.0b5. |
| return 1; | ||
| } | ||
| int docstring = isdocstring((stmt_ty)asdl_seq_GET(stmts, 0)); | ||
| CALL_SEQ(astfold_stmt, stmt_ty, stmts); |
There was a problem hiding this comment.
Loop from 1 seems simpler than JoinedStr hack.
for (int i = docstring; i < asdl_seq_LEN(stmts); i++) {
stmt_ty st = (stmt_ty)asdl_seq_GET(stmts, i);
if (st != NULL && !astfold_stmt(st, ctx_, optimize_)) {
return 0;
}
}There was a problem hiding this comment.
A JoinedStr hack is needed for preventing interpreting a new constant string created by optimization as a docstring:
def foo():
'Not a' + ' docstring'
passbut we still want to apply optimizations to the first statement.
|
Sorry, I don't know Git well still, and loss a news entry when applied Inada's patch. I didn't think that a magic number bumping is needed, because this PR affects AST. But it also affects the first line number of nodes with docstrings. I'll bump a magic number. |
| @@ -0,0 +1,4 @@ | |||
| Reverted :issue:`29463`. ``docstring`` field is removed from Module, ClassDef, | |||
| FunctionDef, and AsyncFunctionDef ast nodes which is added in 3.7a1. Docstring | |||
| @@ -0,0 +1,4 @@ | |||
| Reverted :issue:`29463`. ``docstring`` field is removed from Module, ClassDef, | |||
There was a problem hiding this comment.
We should say why we are doing this. Perhaps start the entry with:
"Due to unexpected compatibility issues discovered during downstream beta testing, reverted ... "
|
When you're done making the requested changes, leave the comment: |
|
I have made the requested changes; please review again. |
|
Thanks for making the requested changes! @methane, @ned-deily, @ncoghlan: please review the changes made to this pull request. |
Remove the docstring attribute of AST types and restore docstring expression as a first stmt in their body. Co-authored-by: INADA Naoki <methane@users.noreply.github.com>
|
Thanks @serhiy-storchaka! |
Earlier development versions of Python 3.7 added the docstring field to AST nodes. This was later reverted in Python 3.7.0b5. https://bugs.python.org/issue29463 python/cpython#7121
Earlier development versions of Python 3.7 added the docstring field to AST nodes. This was later reverted in Python 3.7.0b5. https://bugs.python.org/issue29463 python/cpython#7121
It is based on #5927 and goes one step further to Python 3.6. It doesn't introduce a new AST type DocString.
https://bugs.python.org/issue32911