C++ macro in scope of if statement not compiling

Question

I have some code which is supposed to be a thread-safe python/c++ api. I am using the macros Py_BEGIN_ALLOW_THREADS and Py_END_ALLOW_THREADS, which expand to create save thread state and create a lock. I am releasing the lock just before method exit; once inside of if statement scope, and once at method scope.

Why does this not compile? It generates the error: error: _save was not declared in this scope at the second Py_END_ALLOW_THREADS macro.

uint8_t SerialBuffer::push_msg() { #if defined (UBUNTU) Py_BEGIN_ALLOW_THREADS #endif if (_type == ARRAY) { // array access } else if (_type == PRIORITY_QUEUE) { // queue access } else { // Placing the return statement in the preprocessor directive // has no effect. #if defined (UBUNTU) Py_END_ALLOW_THREADS #endif return FAIL; } #if defined (UBUNTU) Py_END_ALLOW_THREADS #endif return SUCCESS; }

I have also tried putting the return statement inside of the #if directive scope, and this generates the same error. However, this works:

uint8_t SerialBuffer::push_msg() { #if defined (UBUNTU) Py_BEGIN_ALLOW_THREADS #endif if (_type == ARRAY) { // array access } else if (_type == PRIORITY_QUEUE) { // queue access } else { // NOTE lack of #if directive here. // Even though if this code executes the code below will not. // Seems like a relatively simple problem for lambda calculus, no? return FAIL; } #if defined (UBUNTU) Py_END_ALLOW_THREADS #endif return SUCCESS; }

Edit: I'm aware that the second example does not do thread clean-up; however, it compiles.

Edit2: Py_BEGIN_ALLOW_THREADS expands to { PyThreadState *_save; _save = PyEval_SaveThread();

Py_END_ALLOW_THREADS expands to PyEval_RestoreThread(_save); } NOTE the scoping braces prepending BEGIN and appending END. Why is it the logical choice for the macro expansion to include scoping?

RichS · Accepted Answer · 2017-07-27 02:32:14Z

The preprocessor expands the macro, Py_BEGIN_ALLOW_THREADS, into code that creates a local object named _save.

The preprocessor expands the macro, Py_END_ALLOW_THREADS, into code that uses _save to do thread clean-up tasks.

If you put Py_BEGIN_ALLOW_THREADS inside the else block, the code created by Py_END_ALLOW_THREADS can't see the local _save object, so you get an error message.

On a related topic, I recommend putting Py_BEGIN_ALLOW_THREADS and Py_END_ALLOW_THREADS where if the first one executes, then so does the second one. Your second version of the function will not do the thread clean-up tasks for Py_END_ALLOW_THREADS if you have an array type or a priority-queue type.

Try this:

uint8_t SerialBuffer::push_msg() { #if defined (UBUNTU) Py_BEGIN_ALLOW_THREADS #endif uint8_t response = FAIL; if (_type == ARRAY) { // array access response = SUCCESS; } else if (_type == PRIORITY_QUEUE) { // queue access response = SUCCESS; } #if defined (UBUNTU) Py_END_ALLOW_THREADS #endif return response; }

In this version, the default response is FAIL, so you don't even need the final else section. The other if statements only set the response to SUCCESS if all goes well.

Do you mean a directive? Py_BEGIN_ALLOW_THREADS isn't in an if statement. I should think an #if directive is valid, as the preprocessor will see the same macro definition in all valid cases.
@errolflynn Thanks. I corrected the answer to say "the else block" instead of "an if statement".
Your style is good. However, there are other cases where I should like to return early to prevent excessive code from running. Do you know why my example doesn't work?
Py_BEGIN_ALLOW_THREADS isn't in the else block. It is in the method scope. The variable _save should be visible through the entire method.
@errolflynn Oof! I clearly misread the original code snippet! I have an idea. Send the preprocessed source code to a a file and then check the file for what the preprocessor puts inside that function. You can do that with this command line. g++ -E file.cpp > file.pp

CristiFati · Accepted Answer · 2020-06-12 08:18:01Z

From [Python.Docs]: Initialization, Finalization, and Threads - Py_BEGIN_ALLOW_THREADS (emphasis is mine):

This macro expands to { PyThreadState *_save; _save = PyEval_SaveThread();. Note that it contains an opening brace; it must be matched with a following Py_END_ALLOW_THREADS macro. See above for further discussion of this macro.

So, the answer to the compile error is pretty clear:

After preprocessing, the 2^nd Py_END_ALLOW_THREADS produces invalid code (and the enclosing in #if defined (UBUNTU) is irrelevant, as it will never work when UBUNTU is defined, and it will always work when it isn't):
- Referencing (inexistent) "_save"
- An extra closing brace ("}")

The common usecase for the 2 macros is also exemplified int he above page:

PyThreadState *_save; _save = PyEval_SaveThread(); ... Do some blocking I/O operation ... PyEval_RestoreThread(_save);

Why it was designed this way (to include scoping)? Probably to fail when used like you did, because that can cause hard to find errors (your example is pretty simple, but in a more complex piece code with many branches requiring just as many Py_END_ALLOW_THREADS, imagine what would mean to miss one, or to call it twice).

In order to fix your problem, you must redesign your code to either:

Not directly return on failure, but flag it, return (in one place) at the end, and place the Py_END_ALLOW_THREADS, just before returning
Call the macro pair inside each (or where required) of the if branches (ARRAY, PRIORITY_QUEUE, ...)
Use (the dreaded) goto

Collectives™ on Stack Overflow

C++ macro in scope of if statement not compiling

2 Answers 2

6 Comments

Comments

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

6 Comments

Comments

Related