?ggsv? followup to #434 & #409#437
Conversation
…re mismatches
… 'const' in signatures Based on how the surrounding functions in lapack.h are handling the parameters, particularly the ?ggsv?3-variants of the affected functions
h-vetinari
left a comment
h-vetinari
left a comment
There was a problem hiding this comment.
References for the signature inconsistencies
| double* alpha, double* beta, double* u, | ||
| lapack_int* ldu, double* v, lapack_int* ldv, double* q, | ||
| lapack_int* ldq, float* work, lapack_int* iwork, lapack_int* info ); | ||
| lapack_int* ldq, double* work, lapack_int* iwork, lapack_int* info ); |
There was a problem hiding this comment.
| lapack_int* ldu, lapack_complex_float* v, | ||
| lapack_int* ldv, lapack_complex_float* q, | ||
| lapack_int* ldq, float* work, lapack_int* rwork, lapack_int* iwork, lapack_int *info ); | ||
| lapack_int* ldq, lapack_complex_float* work, float* rwork, lapack_int* iwork, lapack_int* info ); |
There was a problem hiding this comment.
| lapack_complex_double* q, lapack_int* ldq, | ||
| float* work, lapack_int* rwork, lapack_int* iwork, lapack_int* info ); | ||
| lapack_complex_double* work, double* rwork, lapack_int* iwork, lapack_int* info ); | ||
|
|
There was a problem hiding this comment.
| lapack_complex_float* v, lapack_int* ldv, | ||
| lapack_complex_float* q, lapack_int* ldq, | ||
| lapack_int* iwork, lapack_int* rwork, | ||
| lapack_int* iwork, float* rwork, |
There was a problem hiding this comment.
| lapack_int* ldu, lapack_complex_double* v, | ||
| lapack_int* ldv, lapack_complex_double* q, | ||
| lapack_int* ldq, lapack_int* iwork, lapack_int* rwork, | ||
| lapack_int* ldq, lapack_int* iwork, double* rwork, |
There was a problem hiding this comment.
|
@langou @martin-frbg PTAL |
Codecov Report
@@ Coverage Diff @@
## master #437 +/- ##
=======================================
Coverage 83.24% 83.24%
=======================================
Files 1808 1808
Lines 170197 170197
=======================================
Hits 141681 141681
Misses 28516 28516 Continue to review full report at Codecov.
|
|
Indeed you are right, not sure how i managed to keep the parameter list messed up (and why it did not get caught by the user who built with -Werror and reported all was fine with my version of the patch). No strong opinion on the |
|
Thanks! |
|
Thanks! |
3 participants
After #434 got merged, I retried building lapack for conda-forge in conda-forge/lapack-feedstock#32 based on
tags/v3.9.0+ #367, #370, #390, #408, #427, #431, #434, #436, but still got build warnings for the ?ggsv?-functions affected by #367, #409 & #434:Details
Based on this, I compared what I had done in #409 with the implementation from #434, and ended up splitting the differences into three separate commits, for ease of reviewing/choosing:
constqualifiers that I had had in Fix some build warnings due to signature inconsistencies #409, based on what I had seen from the surrounding functions, and particularly the ?ggsv?3-variants of the deprecated functions - this compiles without warnings as well. This should be double-checked for correctness, as I'm not 100% sure why which parameters should be const or not. I tracked the original addition of theconstmodifiers back to Split lapack.h out of lapacke.h. Add const for inputs. Body of lapack… #294, but there's not much reasoning to go on for me.lapack.hI think the first commit should be included, the second and third are matters of choice & style, but here for completeness.