GeezerVison

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

The main naming alternative that occurred to me is "frame" and "frame data".

As far as porting goes, the extra layer of indirection is likely to be the biggest hassle, and a one time porting burden for code poking around in frame internals doesn't seem like a compelling reason to tolerate ongoing ambiguity across the code base as to which kind of structure is being used.

I posted the gist of the next iteration I'm going to try to bpo issue (including checking to see if the code still feels unambiguous if the variable names are changed while leaving the struct field names alone): https://bugs.python.org/issue44800#msg399381

For gdbinit, it's just an example file that has a few other problems with it at the moment, so I think it's OK to settle on a final field naming convention before fixing the affected pieces of it.

As far as porting goes, the extra layer of indirection is likely to be the biggest hassle, and a one time porting burden for code poking around in frame internals doesn't seem like a compelling reason to tolerate ongoing ambiguity across the code base as to which kind of structure is being used.

Where is the ambiguity? There are no field names shared between InterpreterFrame and PyFrameObject.

Where is the ambiguity? There are no field names shared between InterpreterFrame and PyFrameObject.

That is the ambiguity: situations where the only quick way to tell which kind of object you had was to have all the field names on each type memorised and infer the type from that, rather than from the types using different variable or field naming conventions. (Relying on the type declarations isn't good, as many of the affected functions are so long that most diffs in code reviews won't have the declaration readily accessible)

@markshannon I have made the requested changes; please review again.

I've updated the PR title and description, since the proposal has changed based on your initial review comments.

I quite like the way the now proposed "frame" (PyFrameObject) vs "frame data" (_Py_framedata) naming convention looks in the code.

For the field prefix selection on the frame data struct, of the 12 fields, 6 didn't have a prefix at all, while 6 shared the f_ prefix with PyFrameObject. Rather than the "maximum churn" option of my original PR (which affected all 12 fields by introducing the xf_ prefix), the latest iteration instead standardises on "no field prefix" for the frame data structs.

Thanks for making the requested changes!

@markshannon: please review the changes made to this pull request.

It's not currently clear why building and testing is current working locally on Fedora 35, but freezing importlib is failing when the CI goes to check that the generated files are all up to date.

Debug vs non-debug build issue (debug builds are unhappy at the moment)

Partially tracked down the debug build failure - somehow PyImport_Import is being called without the Python thread state being set up correctly first while freezing modules, and there's a PyEval_GetGlobals() call in there which isn't being correctly checked for errors (which can happen when there's no thread state set). In a release build, that error was being silently ignored, in a debug build, an assertion caught it. I've added the missing error check so that now always fails (that change should really be pulled out into its own issue report and PR...)

Still not clear how any of the name changes could have affected when the frame state gets populated, though.

As far as I can tell, it's actually legitimate to have no active Python frame when calling PyImport_Import(), as the root_cframe struct doesn't automatically get a Python frame allocated (that doesn't happen until the Python eval loop starts running).

So while I'm still not clear on why this branch is hitting that issue when the main branch doesn't, explicitly ignoring the error seems like the right thing to do.

Closing this one, see #31987 for a scaled back version of the proposed change that leaves local variable and function parameter names alone.