Skip to content

bpo-36876: Moved Parser/listnode.c statics to interpreter state. #16328

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 2 commits into from
Nov 7, 2019
Merged

bpo-36876: Moved Parser/listnode.c statics to interpreter state. #16328

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3c82f2e
Moved Parser/listnode.c statics to interpreter state.
vsajip Sep 22, 2019
4c88f82
Addressed review comments.
vsajip Nov 7, 2019
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
9 changes: 9 additions & 0 deletions Include/internal/pycore_pystate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,15 @@ struct _is {
struct _warnings_runtime_state warnings;

PyObject *audit_hooks;
/*
* See bpo-36876: miscellaneous ad hoc statics have been moved here.
*/
struct {
Copy link
Member

Choose a reason for hiding this comment

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

If you think there will be any other parser/compiler-related state to move to PyInterpreterState then consider making this struct stand-alone and perhaps even creating pycore_compiler.h (or pycore_parser.h) for it. That way the code doesn't get too cluttered.

struct {
int level;
int atbol;
} listnode_data;
} parser_data;
};

PyAPI_FUNC(struct _is*) _PyInterpreterState_LookUpID(PY_INT64_T);
Expand Down
24 changes: 14 additions & 10 deletions Parser/listnode.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
/* List a node on a file */

#include "Python.h"
#include "pycore_pystate.h"
#include "token.h"
#include "node.h"

Expand All @@ -15,19 +16,21 @@ PyNode_ListTree(node *n)
listnode(stdout, n);
}

static int level, atbol;

static void
listnode(FILE *fp, node *n)
{
level = 0;
atbol = 1;
PyInterpreterState *interp = _PyInterpreterState_GET_UNSAFE();

interp->parser_data.listnode_data.level = 0;
Copy link
Member

Choose a reason for hiding this comment

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

I was going to point to the advantage of having stand-alone structs for places like this (so there isn't so much going on with each line). However, I actually kind of like how the line captures the indirection clearly. :)

interp->parser_data.listnode_data.atbol = 1;
list1node(fp, n);
}

static void
list1node(FILE *fp, node *n)
{
PyInterpreterState *interp;

if (n == NULL)
return;
if (ISNONTERMINAL(TYPE(n))) {
Expand All @@ -36,25 +39,26 @@ list1node(FILE *fp, node *n)
list1node(fp, CHILD(n, i));
}
else if (ISTERMINAL(TYPE(n))) {
interp = _PyInterpreterState_GET_UNSAFE();
switch (TYPE(n)) {
case INDENT:
++level;
interp->parser_data.listnode_data.level++;
break;
case DEDENT:
--level;
interp->parser_data.listnode_data.level--;
break;
default:
if (atbol) {
if (interp->parser_data.listnode_data.atbol) {
int i;
for (i = 0; i < level; ++i)
for (i = 0; i < interp->parser_data.listnode_data.level; ++i)
fprintf(fp, "\t");
atbol = 0;
interp->parser_data.listnode_data.atbol = 0;
}
if (TYPE(n) == NEWLINE) {
if (STR(n) != NULL)
fprintf(fp, "%s", STR(n));
fprintf(fp, "\n");
atbol = 1;
interp->parser_data.listnode_data.atbol = 1;
}
else
fprintf(fp, "%s ", STR(n));
Expand Down