bpo-29622: make AST constructor accepts less than enough number of positional arguments#249
bpo-29622: make AST constructor accepts less than enough number of positional arguments#249methane merged 3 commits intopython:masterfrom
Conversation
Parser/asdl_c.py
Outdated
| @@ -666,11 +666,10 @@ def visitModule(self, mod): | |||
| } | |||
| res = 0; /* if no error occurs, this stays 0 to the end */ | |||
| if (PyTuple_GET_SIZE(args) > 0) { | |||
There was a problem hiding this comment.
This test is redundant.
|
I implemented minimum number of arguments. |
|
Could I ask that we don't make arguments required, even if you need them to construct a valid AST? I have a pair of little side projects (astsearch and astcheck) which rely on the ability to instantiate incomplete AST objects. I reuse these as templates, so you can check for things like 'for loop with an else clause' without specifying any of the required pieces of a for loop. Obviously this isn't what the AST classes are intended for, but it works pretty nicely. And yes, I am being this guy: |
|
@takluyver Thank you. I decided to revert last commit. |
|
Thanks @methane :-) |
| self.assertEqual(x.lineno, 0) | ||
|
|
||
| # node raises exception when not given enough arguments | ||
| self.assertRaises(TypeError, ast.BinOp, 1, 2) |
There was a problem hiding this comment.
Keep these and change to assertNotRaises ?
There was a problem hiding this comment.
Well, too late, that was merged while I was reviewing :-)
| for (i = 0; i < PyTuple_GET_SIZE(args); i++) { | ||
| /* cannot be reached when fields is NULL */ | ||
| PyObject *name = PySequence_GetItem(fields, i); | ||
| if (!name) { |
There was a problem hiding this comment.
out of curiosity what's the prefered python style !variable or == NULL
There was a problem hiding this comment.
I didn't care about !name or name == NULL. And I only removed indent.
There was a problem hiding this comment.
Yes, I saw. It was a genuine question like if you are coding something new, what's the prefered style in the CPython codebase?
There was a problem hiding this comment.
https://www.python.org/dev/peps/pep-0007/
PEP 7 doesn't define it. But I think consistency in one file is recommended like PEP 7.
|
Thanks ! |
…n API Stackless does not support custom frame evaluation functions defined by PEP 523. If an extension module sets a custom frame evaluation function, Stackless now terminates to prevent undefined behavior. (cherry picked from commit d57b317)
…n API Stackless does not support custom frame evaluation functions defined by PEP 523. If an extension module sets a custom frame evaluation function, Stackless now terminates to prevent undefined behavior.
…athsep Honor '/'-separated names in ResourceContainer.joinpath.

Currently, AST constructor accepts
a. empty arguments
b. positional arguments, only when it's length is exactly same to number of fields
c. keyword arguments. No check for missing required fields.
Only (b) is strict. And it require argument even if matching field is optional.
This pull request removes the strict check.
Missing required field can be detected when compiling AST though.