bpo-32805: Fix compiler warnings in gcmodule.c

[jkloth]

Use a C data type that matches DTrace's data type size for long.

https://bugs.python.org/issue32805

[serhiy-storchaka]

This is bpo-32805.

But changing the signature of PyDTrace_GC_DONE breaks ABI.

[jkloth]

When compiled with dtrace support (on 64-bits) the signature is already a 64-bit long (see pydtrace.d). This change brings the two definitions in sync.

[serhiy-storchaka]

Should not it be C long as in pydtrace.d?

[jkloth]

Only on 64-bit platforms that have long as 64-bits wide (LP64). Platforms which use LLP64 (Windows) must use a data type that is 64-bits wide. That leaves either Py_ssize_t or Py_intptr_t or an ugly #if/else/endif block around the typedecl.

[vstinner]

Do we have DTrace experts around? @ambv maybe?

[jkloth]

For reference (http://dtrace.org/guide/chp-typeopexpr.html#chp-typeopexpr-2), the D programming language uses ILP32 and LP64 data models. This means we need to use a platform-agnostic signed integer type that matches those semantics since the inline functions are also used on LLP64 platforms (Win64).

[jkloth]

This is the last remaining warning when compiling on Windows (64- or 32-bit). It would be great to see it disappear as well. So, to reiterate, the change does nothing to DTrace support proper, but to the dummy functions used when DTrace support is not compiled in. DTrace itself uses empty macros when generating the dummy functions, but since Python is using inline functions the function arguments need to match what is expected of the functions when DTrace is being used.

This means that the DTrace long function argument MUST map to a 32-bit signed integer type on a 32-bit build and MUST map to a 64-bit signed integer type on a 64-bit build. The choices that exist in Python headers are Py_ssize_t or Py_intptr_t (technically, an #ifdef around the function argument would work too, but that gets ugly fast).

[vstinner]

LGTM.

I agree that Py_ssize_t is the right type here:

static Py_ssize_t
collect(int generation, Py_ssize_t *n_collected, Py_ssize_t *n_uncollectable,
        int nofail)
{
    ...
    Py_ssize_t m = 0; /* # objects collected */
    Py_ssize_t n = 0; /* # unreachable objects that couldn't be collected */
    ...
    if (PyDTrace_GC_DONE_ENABLED())
        PyDTrace_GC_DONE(n+m);
    ...
    return n+m;
}

[ZackerySpytz]

FWIW, this PR is an exact duplicate of the open PR #2852.

[vstinner]

Copy of @benjaminp's comment:
"We shouldn't just change the definition here. Either we need to change the type in pydtrace.d or just pass an int into the trace function in gcmodule.c."
#2852 (comment)

According to @jkloth, the current "probe gc__done(long);" is fine.

I don't know DTrace, so I cannot say. I chose to merge the PR because I really care of warnings, and this issue was the last known warning on Windows. This warning prevents me to more easily detect when I make a mistake (introduce a bug) on Windows.

I someone consider that Include/pydtrace.d should be updated to use "probe gc__done(ssize_t);", please go ahead. Sadly, I never used DTrace and I'm not sure if "ssize_t" type is available for DTrace :-(

cc @tiran