MDAnalysis / mdanalysis

MDAnalysis is a Python library to analyze molecular dynamics simulations.
https://mdanalysis.org
Other
1.26k stars 641 forks source link

Build fails: incompatible function pointer types initializing #4589

Closed yurivict closed 1 month ago

yurivict commented 1 month ago
MDAnalysis/lib/formats/src/xdrfile.c:2535:3: error: incompatible function pointer types initializing 'unsigned int (*)(XDR *)' (aka 'unsigned int (*)(struct XDR *)') with an expression of type 'int64_t (XDR *)' (aka 'long (struct XDR *)') [-Wincompatible-function-pointer-types]
 2535 |                 xdrstdio_getpos,                /* get offset in the stream */
      |                 ^~~~~~~~~~~~~~~
MDAnalysis/lib/formats/src/xdrfile.c:2536:3: error: incompatible function pointer types initializing 'int (*)(XDR *, unsigned int)' (aka 'int (*)(struct XDR *, unsigned int)') with an expression of type 'int (XDR *, int64_t, int)' (aka 'int (struct XDR *, long, int)') [-Wincompatible-function-pointer-types]
 2536 |                 xdrstdio_setpos,                /* set offset in the stream */
      |                 ^~~~~~~~~~~~~~~

Current version of MDAnalysis

hmacdope commented 1 month ago

Hi @yurivict this was fixed in #4408, try cloning a developer copy of the repo and installing the latest develop. Let me know how that goes?

yurivict commented 1 month ago

Ok, thanks,

I'll wait for the next release.

Yuri

hmacdope commented 1 month ago

I'll leave this open for anyone experiencing similar issues.

yurivict commented 1 month ago

People can only build this code successfully due to compiler bugs that allow invalid code. You should probably consider to make a release with this fix soon. (I myself applied a similar patch but others would likely suffer from this.)

IAlibay commented 1 month ago

People can only build this code successfully due to compiler bugs that allow invalid code.

I just want to correct this:

1) GNU compiler compilations are not affected as far as we know 2) Nothing in our test suite fails nor do we have any evidence that these clang compilations lead to incorrect results

A release will come soon, although not until at the very least June.

yurivict commented 1 month ago

The GNU compiler version that you use is buggy. It is probably very old too. Functions of incompatible signatures are allowed to be assigned, which should be an undefined behavior. Recent clang-17 fails to compile it.

IAlibay commented 1 month ago

The compiler versions we use are those that come as default in GitHub actions runners and those that have been chosen as appropriate over conda forge.

In any case, because this discussion can cast doubt on the scientific validity of MDAnalysis, I again want to emphasize that we have no evidence that the validity of any results is in any way affected.

orbeckst commented 1 month ago

Given that this issue is a duplicate of #4397, which we already closed because of merged PR #4408, I would suggest to also close this issue.

I understand https://github.com/MDAnalysis/mdanalysis/issues/4589#issuecomment-2097098749 but I feel it's more consistent to close duplicates together with the original issues. @hmacdope I leave it to you to make the decision.

orbeckst commented 1 month ago

P.S.: Cool to see FreeBSD — gave me a reason to add a new OpSys-FreeBSD label :-)