[R-pkg-devel] [Re] warning: type of ‘zhpevx_’ does not match original declaration [-Wlto-type-mismatch]
On 12/19/20 12:57 AM, Pierre Lafaye de Micheaux wrote:
Thank you Tomas, For those interested, here is (attached file) how I modified the Cpp file following the (last?) advice I received from Tomas. I plan to investigate the iso_c_binding approach suggested by Ivan in a future version of the package, but I just wanted to give it a try with this approach given all the time I spent trying to make it work. Maybe this will be useful to others as well who plan to use complex numbers.
Dear Pierre,
I meant rather something like this (in plain C, there is no need to use
C++ for this):
---------------
#include <stddef.h>??????? /* for size_t (FC_LEN_T) */
#include <Rconfig.h>?????? /* for FC_LEN_T, FCONE */
#include <R_ext/BLAS.h>??? /* for FCONE */
#include <R_ext/RS.h>????? /* for F77_... */
#include <R_ext/Complex.h> /* for Rcomplex */
void F77_NAME(zhpevx)(char *jobz, char *range, char *uplo,
????????????????????? const int *n, Rcomplex *ap, const double *vl,
????????????????????? const double *vu, const int *il, const int *iu,
????????????????????? const double *abstol, int *m, double *w,
????????????????????? Rcomplex *z, const int *ldz, Rcomplex *work,
double *rwork,
????????????????????? int *iwork, int *ifail, int *info,
#ifdef FC_LEN_T
????????????????????? FC_LEN_T jobz_len, FC_LEN_T range_len, FC_LEN_T
uplo_len
#endif
????????????????????? );
void zhpevxC(char **jobz, char **range, char **uplo, int *n, Rcomplex *ap,
???????????? double *vl, double *vu, int *il, int *iu, double *abstol,
int *m,
???????????? double *w, Rcomplex *z, int *ldz, Rcomplex *work, double
*rwork,
???????????? int *iwork, int *ifail, int *info) {
??? F77_CALL(zhpevx)(*jobz, *range, *uplo, n, ap, vl, vu, il, iu,
abstol, m, w,
???????????????????? z, ldz, work, rwork, iwork, ifail, info FCONE
FCONE FCONE);
}
-----------------
with gfortran 8.3, you still get the warnings due to size_t vs "long
int", and due to the complex type (the compiler complains between the
difference between _Complex and Rcomplex). With newer gfortran (9.3)
there are no warnings.
On a side note, my intention was not to do "runtime tricks (such as copying the memory, etc) to silence benign compiler warnings" but only to remove all warnings that prevent the CRAN from accepting my package (I'm sure they have good reasons to do so). I believe these warnings should not appear with gcc/gfortran 8.3.0 which is the standard version on Debian Buster (the current stable version of Debian which I am sure is installed on many production servers).
When you are copying the memory from Rcomplex array to _Complex array, you are still assuming the memory layout is compatible. But in addition, you have more code, use more memory, spend execution time copying (that's what I meant by runtime tricks). In case the memory layouts were actually different, there would still be a problem and it would be harder to find, because you have silenced the warning. I think it is strictly better to use a version that is not hiding this potential problem, along the lines I've written above. Also there should be no need to copy the incoming characters as in the original version. And, one can just write "n" instead of "&n[0]". A portable version that would work for older and newer compilers (and other compilers) could only be created using iso_c_binding. If you just wanted to silence the warnings (but, really, I think that would not be a good thing to do), you in principle still do that by redefining FC_LEN_T and Rcomplex, without any runtime overhead, but that would be hiding potential problems as well. Best Tomas
Thank you again all for sharing your precious time. Kind regards, Pierre L. ------------------------------------------------------------------------ *From:* Tomas Kalibera <tomas.kalibera at gmail.com> *Sent:* Friday, 18 December 2020 19:14 *To:* Pierre Lafaye de Micheaux <lafaye at unsw.edu.au>; Ivan Krylov <krylov.r00t at gmail.com> *Cc:* R Package Devel <r-package-devel at r-project.org> *Subject:* Re: [R-pkg-devel] [Re] warning: type of ?zhpevx_? does not match original declaration [-Wlto-type-mismatch] On 12/18/20 8:25 AM, Pierre Lafaye de Micheaux wrote:
Dear Ivan, Thank you for your comment. And also for the previous one. I indeed made a mistake with JOBZ, RANGE and UPLO, now changed to: ? ? const char *CJOBZ = jobz[0]; ? ? const char *CRANGE = range[0]; ? ? const char *CUPLO = uplo[0]; ... ? ? delete[] CJOBZ; ? ? delete[] CRANGE; ? ? delete[] CUPLO;
Dear Pierre, this unfortunately still does not look right. You are calling "delete[]" on something you have not allocated with "new". This code will not work (or if so, only by coincidence only sometimes, but crash other times or produce incorrect results) and you should be seeing a number of compiler warnings if not errors. "jobz" is a "char *" (unless something changed from you previous example), so jobz[0] is a "char", so not a pointer at all.
I did a bit of F77 programming almost 20 years ago when I did my Ph.D. thesis, but I never programmed in Fortran 2003. I remember having tried a few months back to use the iso_c_binding approach (before your previous email) with no success. I will give it a try to see if I can make it work.
That would be best, but I would certainly advise against doing runtime tricks (such as copying the memory, etc) to silence benign compiler warnings. The version I was looking at seemed ok to me, and a recent compiler would not emit warnings - perhaps you could just upgrade your OS?? If you just extended the use of the FC_ macros there to make that work also when the hidden character arguments were not used, that would be good enough for now I think. For the iso_c_binding, there is an example in Writing R Extensions (but does not include the complex type). Best, Tomas
Have a nice day. Kind regards, Pierre ------------------------------------------------------------------------ *From:* Ivan Krylov <krylov.r00t at gmail.com> <mailto:krylov.r00t at gmail.com> *Sent:* Friday, 18 December 2020 18:16 *To:* Pierre Lafaye de Micheaux <lafaye at unsw.edu.au> <mailto:lafaye at unsw.edu.au> *Cc:* Tomas Kalibera <tomas.kalibera at gmail.com> <mailto:tomas.kalibera at gmail.com>; Prof Brian Ripley <ripley at stats.ox.ac.uk> <mailto:ripley at stats.ox.ac.uk>; R Package Devel <r-package-devel at r-project.org> <mailto:r-package-devel at r-project.org> *Subject:* Re: [R-pkg-devel] [Re] warning: type of ?zhpevx_? does not match original declaration [-Wlto-type-mismatch] Dear Pierre L., I think that the zhpevxC wrapper, as written, may result in undefined behaviour:
??? const char *JOBZ = jobz[0];
??? delete[] JOBZ;
??? delete[] Cap;
This could work okay, depending on how the rest of the package is
written, but in general, it is considered a bad idea for linear algebra
routines to deallocate memory they didn't allocate. ("Pointer
ownership is usually retained by the calling code.")
May I suggest once again the idea of writing a Fortran 2003 wrapper
zhpevxC instead of C++? Subroutines defined using iso_c_binding are
guaranteed to follow the C calling convention, and, this being Fortran,
call zhpevx(...) is guaranteed to match the Fortran calling convention,
bringing you the best of both worlds:
https://stat.ethz.ch/pipermail/r-package-devel/2020q3/005710.html
No need to allocate or deallocate memory or provide different
definitions depending on the availability of FC_LEN_T, just make sure
that both prototypes mean the same thing. By the way,
std::complex<double> is guaranteed to match the memory layout of C type
double _Complex and Fortran type complex(kind = c_double_complex) by
the respective standards.
--
Best regards,
Ivan