Disable GCC warnings/errors about wrong self casts in final function calls

[scoder]

See #2747

[hroncok]

I plan to test this today in Fedora with lxml, scikit-learn and jnius.

[hroncok]

lxml works.

jnius 1.6.1 passed with the gentoo patch but fails with this one:

[1/1] Cythonizing jnius/jnius.pyx
building 'jnius' extension
creating build/temp.linux-x86_64-cpython-312
creating build/temp.linux-x86_64-cpython-312/jnius
gcc -fno-strict-overflow -Wsign-compare -DDYNAMIC_ANNOTATIONS_ENABLED=1 -DNDEBUG -fexceptions -fcf-protection -fexceptions -fcf-protection -fexceptions -fcf-protection -O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -march=x86-64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -fPIC -I/usr/lib/jvm/java-17-openjdk-17.0.10.0.7-1.fc40.x86_64/include -I/usr/lib/jvm/java-17-openjdk-17.0.10.0.7-1.fc40.x86_64/include/linux -I/usr/include/python3.12 -c jnius/jnius.c -o build/temp.linux-x86_64-cpython-312/jnius/jnius.o
jnius/jnius.c: In function ‘__pyx_f_5jnius_get_jnienv’:
jnius/jnius.c:22575:76: error: passing argument 2 of ‘(*__pyx_v_5jnius_jvm)->AttachCurrentThread’ from incompatible pointer type [-Wincompatible-pointer-types]
22575 |   (void)((__pyx_v_5jnius_jvm[0])->AttachCurrentThread(__pyx_v_5jnius_jvm, (&__pyx_v_env), NULL));
      |                                                                           ~^~~~~~~~~~~~~
      |                                                                            |
      |                                                                            const struct JNINativeInterface_ ***
jnius/jnius.c:22575:76: note: expected ‘void **’ but argument is of type ‘const struct JNINativeInterface_ ***’
jnius/jnius.c: In function ‘__pyx_f_5jnius_convert_jstring_to_python’:
jnius/jnius.c:31008:19: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
31008 |   __pyx_v_j_chars = (__pyx_v_j_env[0])->GetStringChars(__pyx_v_j_env, __pyx_v_j_string, NULL);
      |                   ^
jnius/jnius.c: In function ‘__pyx_pf_5jnius_13MetaJavaClass_8resolve_class’:
jnius/jnius.c:43722:30: error: assignment to ‘jobject’ {aka ‘struct _jobject *’} from incompatible pointer type ‘struct _jobject **’ [-Wincompatible-pointer-types]
43722 |           (__pyx_v_jargs[0]) = ((jobject *)__pyx_v_classLoader);
      |                              ^
jnius/jnius.c:43731:30: error: assignment to ‘jobject’ {aka ‘struct _jobject *’} from incompatible pointer type ‘struct _jobject **’ [-Wincompatible-pointer-types]
43731 |           (__pyx_v_jargs[1]) = __pyx_v_interfaces;
      |                              ^
jnius/jnius.c: In function ‘__pyx_f_5jnius_create_proxy_instance’:
jnius/jnius.c:61438:36: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
61438 |   (__pyx_v_invoke_methods[0]).name = ((char const *)"invoke0");
      |                                    ^
jnius/jnius.c:61447:41: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
61447 |   (__pyx_v_invoke_methods[0]).signature = ((char const *)"(Ljava/lang/Object;Ljava/lang/reflect/Method;[Ljava/lang/Object;)Ljava/lang/Object;");
      |                                         ^
error: command '/usr/bin/gcc' failed with exit code 1

[hroncok]

scikit-learn says:

sklearn/tree/_tree.cpp:25797:34: warning: option ‘-Wincompatible-pointer-types’ is valid for C/ObjC but not for C++ [-Wpragmas]
25797 |   #pragma GCC diagnostic ignored "-Wincompatible-pointer-types"
      |                                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

But compiles :)

[hroncok]

I suppose scikit-learn has both C and C++ sources and the -Wincompatible-pointer-types thing only makes it fail on C sources. Perhaps we should only add the pragma if it is C.

[da-woods]

To me the error in jnius looks like it's in their code rather than in Cython. (I could be wrong though.)

[hroncok]

Yes, the jnius error seems different.

What about the C++ thing? I was looking at the code trying to figure out if we are in C++ file, but I think the function does not know that.

[mgorny]

What about the C++ thing? I was looking at the code trying to figure out if we are in C++ file, but I think the function does not know that.

Perhaps #ifdef __cplusplus would work.

[hroncok]

Thanks. Retrying.

[hroncok]

lxml good, scikit-learn good, jnius the same.

Looking good!

[kloczek]

Hmm .. but why those warnings are swiped under the carpet? 🤔

[scoder]

Hmm .. but why those warnings are swiped under the carpet? 🤔

Because they don't help anyone and now turn into errors in recent gcc versions, which is even more unhelpful.

[dalcinl]

@scoder There are serious issues with this fix

1. Didn't you forgot to #pragma GCC diagnostic push before #pragma GCC diagnostic ignore ...?
2. What about non-GCC compilers like MSVC? Tons of warnings about unknown pragmas on Windows [logs]

While (1) would be trivial to fix with just one added line, (2) requires a refactor. Given that this issue is for a a new GCC version, you can conditionally define two begin/end macros using C99 _Pragma operator to bracket the code where want the warning disabled.

[hroncok]

There is no push needed before ignore. Pop restores the default.

[dalcinl]

@hroncok What pull are you talking about?

[da-woods]

https://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Pragmas.html

If a pop has no matching push, the command-line options are restored.

I think we all agree this is only a temporary solution though

[da-woods]

pull are you talking about?

I think this was just a typo for pop

[dalcinl]

Suppose a user includes external C code with a line

  #pragma GCC diagnostic ignored "-Wincompatible-pointer-types"

because she wants to ignore the warning for whatever reason.

Afterwards, Cython generates code with

  #pragma GCC diagnostic ignored "-Wincompatible-pointer-types"
  ...
  #pragma GCC diagnostic pop

Therefore, without an push, the pop line will reset warnings to command line, effectively cleaning what the user intended.

[hroncok]

Apologies, when I said "pull" I meant "pop".

[scoder]

Suppose a user includes external C code with a line

  #pragma GCC diagnostic ignored "-Wincompatible-pointer-types"

I think this is going to be really rare. It's much easier to pass the argument globally via CFLAGS than to try to apply it locally with a C code include.

[dalcinl]

If this is a temporary workaround as @da-woods said, then I'll not make a big deal out of it and we can all forget about my original complaint. However, if it is going to be a permanent thing, then the implementation has to improve. Let's not second-guess users and/or find excuses for bad/sloppy code, even more when the fix is as trivial as generating an extra line with #pragma GCC diagnostic push.

[mgorny]

I would also like to point out that prior code (in Cython/Utility/TypeConversion.c) consistently does push/pop approach, so I find the inconsistency in new code surprising.

[scoder]

There's going to be a new 3.0.x release at some point – stop fighting and send me a PR already. :)