ALL_CAPS for macro parameter names?

Core Development
1

Macro names are usually written in ALL_CAPS. It is an old tradition that predates Python, and PEP 7 only explicitly specifies that [public] macros should have the MixedCase prefix before ALL_CAPS.

But what about macro parameter names? In all examples that I seen before, in PEP 7 examples, in the CPython code, parameter names are in lower case. Some styleguides recomend to add the underscore suffix or prefix, but CPython code does not always follow this.

@erlendaasland advocates for using ALL_CAPS style for macro parameter names and says that it is already used by other core developers. I haven’t seen it yet, is it a new tendency? Should it be explicitly specified in PEP 7? Otherwise we will find that the half of the code uses the upper case and the other half uses the lowr case, and it will be difficult to read.

2

I learned the ALL_CAPS style from the CPython code base. I don’t remember which extension module it was right now. I’ve come to prefer it since it visually separates the macro parameters from variables that are implicitly used in the macro body. A macro parameter is not a C variable; it is text that is being replaced by the C pre-processor. Visually making the macro parameters stand out makes a macro easier to understand and reason about, IMO.

I don’t think it is a problem with two styles. There are already different macro styles, for example when it comes to the placement of line continuation characters. (For those, I prefer to align them; IMO it makes the code less cluttered.)

1 Like
4

I understand the reasons. But it is so unusual, it stands out so much. Until now, this meant that you had to look elsewhere for the definition of a name as a macro. Only names defined with #define used ALL_CAPS.

We need to decide what style to use in the new macros and document it. This does not mean that existing macros should be rewritten right now. Style guides exist to avoid style debates.

1 Like
5

FWIW, I’ve done it both ways. IIRC, at first I’d always do it ALL_CAPS, following the prior art I’d seen. However, when I ran into existing macros with lower-case parameters, I mostly started adjusting to match nearby macros.

I’m not sure it matters much, though I do think it’s a little easier for readers to identify the ALL_CAPS names in the macro body.

Regardless, clarity on a consistent pattern (i.e. a PEP 7 note) would be nice; it would be one less thing to think about.

1 Like
6

I prefer lowercase for macro args. The ALL_CAPS style is used in a few places and I can’t get used to it.

2 Likes
7

Aligned with @erlendaasland on all points (including the pretty lined-up-slashes).

I use caps. My mental model is that macro parameters work just as macros do: using token replacement. It’s really a macro-within-a-macro, to me at least.

3 Likes
8

For comparison, here’s a macro from CPython’s specialization stats machinery:

#define ADD_STAT_TO_DICT(res, field) \
    do { \
        PyObject *val = PyLong_FromUnsignedLongLong(stats->field); \
        if (val == NULL) { \
            Py_DECREF(res); \
            return NULL; \
        } \
        if (PyDict_SetItemString(res, #field, val) == -1) { \
            Py_DECREF(res); \
            Py_DECREF(val); \
            return NULL; \
        } \
        Py_DECREF(val); \
    } while(0);

field is purely a vehicle for macro magic, not actually a struct member. It’s also harder for me to notice that there is a code path where res is evaluated twice (especially when contrasted with val, which doesn’t have this concern).

I find this all much easier to reason about in my preferred rewriting:

#define ADD_STAT_TO_DICT(RES, FIELD)                               \
    do {                                                           \
        PyObject *val = PyLong_FromUnsignedLongLong(stats->FIELD); \
        if (val == NULL) {                                         \
            Py_DECREF((RES));                                      \
            return NULL;                                           \
        }                                                          \
        if (PyDict_SetItemString((RES), #FIELD, val) == -1) {      \
            Py_DECREF((RES));                                      \
            Py_DECREF(val);                                        \
            return NULL;                                           \
        }                                                          \
        Py_DECREF(val);                                            \
    } while (0)

(I would say the lined-up slashes are preferred, but not required. They can be a bit painful to write and maintain, and can clutter up diffs a bit.)

9 Likes
9

This is a rather compelling example indeed. The fact that FIELD is uppercase in stats->FIELD immediately hints that the struct field’s name is macro-expanded, while stats->field could leave the reader dumbfounded.

So, yes, +1 from me on uppercase macro parameter names.

7 Likes
10

Agreed, as long as we’re not encouraging folks to make all macros “consistent”.

5 Likes
11

There’s another style suggestion hiding here: if the macro argument is a variable/expression, it should be enclosed in parentheses.
IMO, that’s a good rule of thumb. Kind of like redundant braces on single-line if/loop bodies.

4 Likes
12

On Py_DECREF((RES)), double parenthesis are redundant since there are outter parenthesis for Py_DECREF(...) call.

13

Expanding a little bit on this digression[1]; for do...while(0) macros, I prefer to add the do { on the first line, because it saves an indent and a line.

Example 
#define ADD_STAT_TO_DICT(res, field) \
    do { \
        PyObject *val = PyLong_FromUnsignedLongLong(stats->field); \
        if (val == NULL) { \
            Py_DECREF(res); \
            return NULL; \
        } \
        if (PyDict_SetItemString(res, #field, val) == -1) { \
            Py_DECREF(res); \
            Py_DECREF(val); \
            return NULL; \
        } \
        Py_DECREF(val); \
    } while(0);
My preferences applied 
#define ADD_STAT_TO_DICT(RES, FIELD) do {                      \
    PyObject *val = PyLong_FromUnsignedLongLong(stats->FIELD); \
    if (val == NULL) {                                         \
        Py_DECREF((RES));                                      \
        return NULL;                                           \
    }                                                          \
    if (PyDict_SetItemString((RES), #FIELD, val) == -1) {      \
        Py_DECREF((RES));                                      \
        Py_DECREF(val);                                        \
        return NULL;                                           \
    }                                                          \
    Py_DECREF(val);                                            \
} while (0)

  1. “other style suggestions” ↩︎

1 Like
14

Based on the reactions on Brandt, Antoine and Guido’s posts, there seems to be a consensus here. I created a PR in the PEP repo. Let the nitpicking begin :slight_smile:

1 Like
15

I don’t mind, but it surprises me that this is the first time I encounter such a style.