bpo-32911: Add new AST node for docstring by methane · Pull Request #5927 · python/cpython
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added few nitpicks, but LGTM in general!
| } | ||
| } | ||
| return NULL; | ||
| return 0; |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functions in this file usually return 1 on success and 0 on error (except functions which need to return a non-negative index on success).
| if (find_ann(stmts)) { | ||
| ADDOP(c, SETUP_ANNOTATIONS); | ||
| } | ||
| if (!asdl_seq_LEN(stmts)) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe move this to the bottom of this function?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be before asdl_seq_GET.
And this code is copied back from 3.6. (There are extra blank line, I'll remove it)
| if (!asdl_seq_LEN(stmts)) | |
| return 1; | |
| st = (stmt_ty)asdl_seq_GET(stmts, 0); |
|
|
||
| def _DocString(self, t): | ||
| self.fill() | ||
| self.write(repr(t.s)) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not fill()?