-
-
Notifications
You must be signed in to change notification settings - Fork 31.5k
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bpo-37001: Makes symtable.symtable have parity with compile for input #13483
Conversation
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA). Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. You can check yourself to see if the CLA has been received. Thanks again for your contribution, we look forward to reviewing it! |
for source code as compile()
Sorry, I don't review C code any more. Hopefully Serhiy will review this. Good luck! |
Modules/symtablemodule.c
Outdated
@@ -14,7 +14,7 @@ module _symtable | |||
/*[clinic input] | |||
_symtable.symtable | |||
|
|||
str: str | |||
str: object |
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.
I suggest to rename str
to source
.
return NULL; | ||
} | ||
t = (PyObject *)st->st_top; | ||
Py_INCREF(t); | ||
PyMem_Free((void *)st->st_future); |
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.
Leaked source_copy
.
rename str->source - lso matching compile, after verifying we don't currently accept kw arguments
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.
LGTM, but you need to regenerate generated files. make regen-all
@@ -0,0 +1,2 @@ | |||
symtable.symtable now accepts the same input types for source code as the |
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.
symtable.symtable now accepts the same input types for source code as the | |
:func:`symtable.symtable` now accepts the same input types for source code as the |
@@ -0,0 +1,2 @@ | |||
symtable.symtable now accepts the same input types for source code as the | |||
built-in compile function. |
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.
built-in compile function. | |
built-in :func:`compile` function. |
@DinoV: Please replace |
…python#13483) * Makes symtable.symtable have parity for accepted datatypes for source code as compile() * Add NEWS blurb
This PR modifies symtable.symtable() to allow bytes objects as source input similar to how they're accepted in compile().
Currently if you were parsing code with ast.parse(), and attempting to understand the name bindings with symtable.symtable(), you have one of two options:
1) Only accept strings
2) Use the tokenize module to go through a separate code path to attempt to parse the PEP 263 coding string.
Given that symtable.symtable ultimately goes down to the same underlying APIs as compile() it seems silly to require these extra steps. So this unifies exec/eval/compile/symtable.symtable to use a single code path (_Py_SourceAsString) for doing the conversion. This is the same as the existing source_as_string which is simply renamed and moved in with the other various parsing functions.
https://bugs.python.org/issue37001