gh-120593: Fix const qualifier in pyatomic.h

[vstinner]

Remove the 'const' qualifier of the argument of functions:

• _Py_atomic_load_ptr()
• _Py_atomic_load_ptr_relaxed()
• _Py_atomic_load_ptr_acquire()

• Issue: pyatomic_gcc.h with GCC discards const qualifier  #120593

[picnixz]

I'm not sure if you like when we comment on a draft so feel free to ignore my comment!

[picnixz]

I don't understand this change. Shouldn't you rather remove the const from the signature?

[vstinner]

Without this change, the compiler emits a warning.

[picnixz]

Yes, but isn't it because the object being passed to _Py_atomic_load_uintptr_acquire is a const uintptr_t *?

Now that I look a bit around, all the signatures take a const T * but I'm not sure if it's better to remove the inner cast or to fix the cast itself.

For instance,

static inline void *
_Py_atomic_load_ptr_acquire(const void *obj)
{ return (void *)__atomic_load_n((void **)obj, __ATOMIC_ACQUIRE); }

could be fixed as

static inline void *
_Py_atomic_load_ptr_acquire(const void *obj)
{ return (void *)__atomic_load_n((void * const*)obj, __ATOMIC_ACQUIRE); }

instead of changing the signature from const void * to void * (also, the cast on the return type should not needed I think). What do you think of that? at least, we would keep the same logic for signatures.

[vstinner]

Yes, but isn't it because the object being passed to _Py_atomic_load_uintptr_acquire is a const uintptr_t *?

The compiler warns if you cast from "const TYPE" to "TYPE". My change removes the cast. It just works.

instead of changing the signature from const void * to void *

void* is special. I gave up trying to fix the warning, so I changed the parameters type of the 3 "pointer" functions instead.

Did you try your proposed fix? When I tried, it didn't work for me.

[picnixz]

Well yes, it worked for me actually.. that's why I suggested it. I changed this:

// L491 in pyatomic_gcc.h
static inline void *
_Py_atomic_load_ptr_acquire(const void *obj)
{ return (void *)__atomic_load_n((void **)obj, __ATOMIC_ACQUIRE); }

into

static inline void *
_Py_atomic_load_ptr_acquire(const void *obj)
{ return __atomic_load_n((void * const*)obj, __ATOMIC_ACQUIRE); }

and I did not have any warning for this specific function (there were more but not for this one) (I also tried with a cast on the return and the warning still did not appear).

My GCC version is 7.5.0 by the way.

[vstinner]

Maybe I tried const void * const *, I don't recall.

Anyway, I wrote a new PR instead: PR gh-121055.

[picnixz]

Maybe I tried const void * const *,

It's highly possible because the first fix I suggested on the issue was actually with a return type of const void * and not void * (hence the const void * const *). So it's partly my fault!

[vstinner]

Thank you for your nice advice, it worked well at the end 😉

[vstinner]

@colesbury: Would you be ok with these changes to avoid compiler warnings with -Wcast-qual? Or does it look suspicious to you?

cc @pitrou

[pitrou]

Why not use something like this instead of removing the const qualifiers altogether?

[colesbury]

I don't think we should remove the const qualifiers. They are useful for the load functions both to signify to callers that we don't modify the value and to avoid extra casts at the call sites.

My preference in rough order is:

• We should fix the cast qualifiers when possible, like @pitrou suggests
• We shouldn't be chasing useless warning safety, like -Wcast-qual when it makes our headers worse. People shouldn't be applying their project's warnings to Python.h. I forget if this is just a matter of #include <Python.h> (vs. #include "Python.h") or something else.
• Or finally, suppress the warnings with the appropriate #pragma gcc directive in pyatomic_gcc.h

[vstinner]

I don't think we should remove the const qualifiers.

Ok, I abandon this PR in favor of a different approach: PR gh-121055.