Draft: explicit template parameter to tag type argument

This MR is a first attempt at looking to simplify part of the interface of Eigen. In particular, the current style, together with the recommendation/necessity to write functions as templates, often requires the use of a template disambiguation. Arguably, this is less familiar syntax (at least I was confused back when I saw this construct the first time).

This MR would reduce the need for such template markers by introducing tag types that can be passed as dummy objects, for now for the selfadjoint and triangular view functions. This changes

matrix.template selfadjointView<Lower>()

into

matrix.selfadjointView(Lower_t{})

Overall, this alone reduces the number of occurrences of the pattern .template in the Eigen codebase from 1800 to 1300.

A few questions:

  1. Is there interest in this kind of change?
  2. Right now the new implementation is still backward compatible with the old way of calling the function. The syntax could probably be made a bit nicer by defining constexpr objects of the tag types, that can be passed as the arguments, i.e. at global scope have constexpr const Lower_t Lower{};. But this would break compatibility.
  3. So far, i've only used the tag types in the outermost layer of the interface. Maybe it would make sense to also use them in the implementation, as they could be used to impose some more structure than an arbitrary bit-pattern stored in an unsigned int (Right now, it i.e. static_asserts that the diag is not claimed to be zero and one at the same time).
  4. Right now some functions use the marker Lower|Upper to denote a full matrix. This is a bit ambiguous to me, because it could be interpreted either as "a matrix that has both upper and lower parts" or "a matrix that is both upper and lower triangular" -- i.e. diagonal.
  5. Is there a reason why SparseSelfAdjointView is its own template instead of a partial specialization of the generic SelfAdjointView?

Any further comments/requests/criticism?

-- The relevant changes are in SelfAdjointView.h, TriangularMatrix.h, MatrixBase.h, SparseSelfAdjointView.h, SparseMatrixBase.h, SparseTriangularView.h and Constants.h, the rest is just updating call sites to the new syntax.

Edited by Erik Schultheis

Merge request reports

Loading