Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32k
bpo-32911: Remove the docstring attribute of AST types#7121
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
bpo-32911: Remove the docstring attribute of AST types#7121
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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).
bedevere-bot commentedMay 27, 2018
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Loop from 1 seems simpler than JoinedStr hack.
for (inti=docstring;i<asdl_seq_LEN(stmts);i++) {stmt_tyst= (stmt_ty)asdl_seq_GET(stmts,i);if (st!=NULL&& !astfold_stmt(st,ctx_,optimize_)) {return0; } }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
A JoinedStr hack is needed for preventing interpreting a new constant string created by optimization as a docstring:
deffoo():'Not a'+' docstring'pass
but 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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
A couple of wording improvements
| @@ -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 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
"which was added"
| @@ -0,0 +1,4 @@ | |||
| Reverted :issue:`29463`. ``docstring`` field is removed from Module, ClassDef, | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We should say why we are doing this. Perhaps start the entry with:
"Due to unexpected compatibility issues discovered during downstream beta testing, reverted ... "
bedevere-bot commentedMay 28, 2018
When you're done making the requested changes, leave the comment: |
I have made the requested changes; please review again. |
bedevere-bot commentedMay 29, 2018
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 docstringexpression 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 toAST nodes. This was later reverted in Python 3.7.0b5.https://bugs.python.org/issue29463python/cpython#7121
Earlier development versions of Python 3.7 added the docstring field toAST nodes. This was later reverted in Python 3.7.0b5.https://bugs.python.org/issue29463python/cpython#7121
Uh oh!
There was an error while loading.Please reload this page.
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