Skip to content

bpo-29622: make AST constructor accepts less than enough number of positional arguments #249

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Feb 23, 2017
Merged

bpo-29622: make AST constructor accepts less than enough number of positional arguments #249

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
edc202c
bpo-29622: loosen positional argument check of AST constructor
methane Feb 23, 2017
d29ca67
remove test about argument is not enough
methane Feb 23, 2017
03940b1
remove redundant check
methane Feb 23, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions Lib/test/test_ast.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,12 +373,8 @@ def test_nodeclasses(self):
self.assertEqual(x.right, 3)
self.assertEqual(x.lineno, 0)

# node raises exception when not given enough arguments
self.assertRaises(TypeError, ast.BinOp, 1, 2)
# node raises exception when given too many arguments
self.assertRaises(TypeError, ast.BinOp, 1, 2, 3, 4)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep these and change to assertNotRaises ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, too late, that was merged while I was reviewing :-)

# node raises exception when not given enough arguments
self.assertRaises(TypeError, ast.BinOp, 1, 2, lineno=0)
# node raises exception when given too many arguments
self.assertRaises(TypeError, ast.BinOp, 1, 2, 3, 4, lineno=0)

Expand Down
36 changes: 17 additions & 19 deletions Parser/asdl_c.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -664,29 +664,27 @@ def visitModule(self, mod):
if (numfields == -1)
goto cleanup;
}

res = 0; /* if no error occurs, this stays 0 to the end */
if (PyTuple_GET_SIZE(args) > 0) {
if (numfields != PyTuple_GET_SIZE(args)) {
PyErr_Format(PyExc_TypeError, "%.400s constructor takes %s"
"%zd positional argument%s",
Py_TYPE(self)->tp_name,
numfields == 0 ? "" : "either 0 or ",
numfields, numfields == 1 ? "" : "s");
if (numfields < PyTuple_GET_SIZE(args)) {
PyErr_Format(PyExc_TypeError, "%.400s constructor takes at most "
"%zd positional argument%s",
Py_TYPE(self)->tp_name,
numfields, numfields == 1 ? "" : "s");
res = -1;
goto cleanup;
}
for (i = 0; i < PyTuple_GET_SIZE(args); i++) {
/* cannot be reached when fields is NULL */
PyObject *name = PySequence_GetItem(fields, i);
if (!name) {
res = -1;
goto cleanup;
}
for (i = 0; i < PyTuple_GET_SIZE(args); i++) {
/* cannot be reached when fields is NULL */
PyObject *name = PySequence_GetItem(fields, i);
if (!name) {
res = -1;
goto cleanup;
}
res = PyObject_SetAttr(self, name, PyTuple_GET_ITEM(args, i));
Py_DECREF(name);
if (res < 0)
goto cleanup;
}
res = PyObject_SetAttr(self, name, PyTuple_GET_ITEM(args, i));
Py_DECREF(name);
if (res < 0)
goto cleanup;
}
if (kw) {
i = 0; /* needed by PyDict_Next */
Expand Down
36 changes: 17 additions & 19 deletions Python/Python-ast.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -551,29 +551,27 @@ ast_type_init(PyObject *self, PyObject *args, PyObject *kw)
if (numfields == -1)
goto cleanup;
}

res = 0; /* if no error occurs, this stays 0 to the end */
if (PyTuple_GET_SIZE(args) > 0) {
if (numfields != PyTuple_GET_SIZE(args)) {
PyErr_Format(PyExc_TypeError, "%.400s constructor takes %s"
"%zd positional argument%s",
Py_TYPE(self)->tp_name,
numfields == 0 ? "" : "either 0 or ",
numfields, numfields == 1 ? "" : "s");
if (numfields < PyTuple_GET_SIZE(args)) {
PyErr_Format(PyExc_TypeError, "%.400s constructor takes at most "
"%zd positional argument%s",
Py_TYPE(self)->tp_name,
numfields, numfields == 1 ? "" : "s");
res = -1;
goto cleanup;
}
for (i = 0; i < PyTuple_GET_SIZE(args); i++) {
/* cannot be reached when fields is NULL */
PyObject *name = PySequence_GetItem(fields, i);
if (!name) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

out of curiosity what's the prefered python style !variable or == NULL

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't care about !name or name == NULL. And I only removed indent.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I saw. It was a genuine question like if you are coding something new, what's the prefered style in the CPython codebase?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

res = -1;
goto cleanup;
}
for (i = 0; i < PyTuple_GET_SIZE(args); i++) {
/* cannot be reached when fields is NULL */
PyObject *name = PySequence_GetItem(fields, i);
if (!name) {
res = -1;
goto cleanup;
}
res = PyObject_SetAttr(self, name, PyTuple_GET_ITEM(args, i));
Py_DECREF(name);
if (res < 0)
goto cleanup;
}
res = PyObject_SetAttr(self, name, PyTuple_GET_ITEM(args, i));
Py_DECREF(name);
if (res < 0)
goto cleanup;
}
if (kw) {
i = 0; /* needed by PyDict_Next */
Expand Down