TRSV forward declarations

[Marcin Copik]

Blaze puts forward declarations of few variants of TRSV functions into the global namespace. Unfortunately, these declarations can create conflicts when building with Eigen and Blaze since Blaze declarations have non-constant pointer types:

blaze/math/lapack/trsv.h:59:148: error: conflicting declaration of C function ‘void dtrsv_(char*, char*, char*, int*, double*, int*, double*, int*)’

eigen3/Eigen/src/Core/util/../../misc/blas.h:188:5: note: previous declaration ‘int dtrsv_(const char*, const char*, const char*, const int*, const double*, const int*, double*, const int*)’
 int BLASFUNC(dtrsv) (const char *, const char *, const char *, const int *, const double *, const int *, double *, const int *);

I think that Eigen is partially correct here: CBLAS header declares these functions with const pointers as parameters. On the other hand, the return type should be void. Is there any specific reason why Blaze is not using const in these params? If not, then I'd be happy to create a PR changing that.

By the way, isn't trsv a level-2 BLAS operation? The Blaze header is located in clapack directory.

[Klaus Iglberger]

Hi Marcin!

Thanks for pointing out this compilation issue. Also thanks for point to the CBLAS header file, which clearly uses const. However, the conflicting declarations don't affect forward declarations of CBLAS functions (e.g. cblas_dtrsv()), but forward declarations for C bindings to Fortran functions. Unfortunately there is no "standard" how to forward declare a C binding for a Fortran function. The common way to do this is to ignore const altogether (see for instance this article), but as stated, there is no standard. Therefore I believe there is no right or wrong, i.e. neither Eigen nor Blaze is "correct".

We are currently considering how to proceed. From our point of view this is not a bug, but merely an inconvenience for users that include both frameworks. Fixing the Blaze forward declarations for trsv would very likely not be enough, we would have to adapt all forward declarations for Fortran functions in the same way. This might however cause problems with other frameworks that have similar forward declarations (the problem of a lacking standard, sigh). We will try to find a reasonable compromise as quickly as possible.

You are correct, though, that trsv is a level-2 BLAS operation. Therefore the Blaze header would correctly be located in the blas directory, not the lapack directory. However, since all other solver algorithms (gesv, sysv, hesv, and posv) are Lapack algorithms, we made the choice to put group all solvers together in the lapack directory.

Thanks again for raising this issue,

Best regards,

Klaus!

[Klaus Iglberger]

Hi Marcin!

After analyzing the issue we have come to the conclusion that there is indeed nothing wrong with the way Blaze provides the C bindings for the LAPACK Fortran functions. Still, the conflicting forward declarations are unfortunate and the issue should be resolved. Whereas we would be willing to add const to the parameters of the forward declarations, we are not willing to adapt the return type. In this case we believe Blaze is doing the right thing. Therefore we have decided to merely hide the Blaze forward declarations in case the Eigen forward declarations are already visible. This solution unfortunately only works if the <Eigen/src/misc/blas.h> header is included before any Blaze header, but the problem can be solved without modifying any source code within Eigen or Blaze.

We should point out that the <Eigen/src/misc/blas.h> header has considerably changed from Eigen 3.2 to 3.4. The number of conflicting forward declarations has been much higher in Eigen 3.2. Therefore there is hope that the remaining conflict might also disappear in future releases of Eigen.

Thanks again for pointing out this compilation issue,

Best regards,

Klaus!

[Klaus Iglberger]

Commit ae8c563 implements a fix to mitigate the compilation issue. The fix is immediately available via cloning the Blaze repository and will be officially released in Blaze 3.4.