[R-pkg-devel] [Re] warning: type of ‘zhpevx_’ does not match original declaration [-Wlto-type-mismatch]

[Tomas Kalibera]

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 using gmail.com>
> *Sent:* Friday, 18 December 2020 19:14
> *To:* Pierre Lafaye de Micheaux <lafaye using unsw.edu.au>; Ivan Krylov 
> <krylov.r00t using gmail.com>
> *Cc:* R Package Devel <r-package-devel using 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 using gmail.com> 
>> <mailto:krylov.r00t using gmail.com>
>> *Sent:* Friday, 18 December 2020 18:16
>> *To:* Pierre Lafaye de Micheaux <lafaye using unsw.edu.au> 
>> <mailto:lafaye using unsw.edu.au>
>> *Cc:* Tomas Kalibera <tomas.kalibera using gmail.com> 
>> <mailto:tomas.kalibera using gmail.com>; Prof Brian Ripley 
>> <ripley using stats.ox.ac.uk> <mailto:ripley using stats.ox.ac.uk>; R Package 
>> Devel <r-package-devel using r-project.org> 
>> <mailto:r-package-devel using 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
>
>


	[[alternative HTML version deleted]]