This bugzilla service is closed. All entries have been migrated to https://gitlab.com/libeigen/eigen
Bug 877 - Warnings on implicit conversion loses integer precision
Summary: Warnings on implicit conversion loses integer precision
Status: RESOLVED FIXED
Alias: None
Product: Eigen
Classification: Unclassified
Component: General (show other bugs)
Version: 3.3 (current stable)
Hardware: All All
: Normal Compilation Problem
Assignee: Gael Guennebaud
URL:
Whiteboard:
Keywords:
Depends on: 572
Blocks: 3.3
  Show dependency treegraph
 
Reported: 2014-09-12 19:57 UTC by Georg Drenkhahn
Modified: 2019-12-04 13:44 UTC (History)
3 users (show)



Attachments

Description Georg Drenkhahn 2014-09-12 19:57:31 UTC
Clang 3.4 reports a lot of "implicit conversion" warnings when compiling the tests (e.g. http://manao.inria.fr/CDash/viewBuildError.php?type=1&buildid=14193).  Many of those are due to conversions of index types mapping to int or long.

I have fixed these type issues by changing index types to match the index types of the accessed objects wherever possible.  If this was not possible I converted the implicit conversion into an explicit one and adding internal asserts to be able to catch real overflows on conversion.

Here are short descriptions of the problems and my solution proposals:

eigen/unsupported/test/../../Eigen/src/SparseCore/SparseDenseProduct.h:141:58: warning: implicit conversion loses integer precision: 'Index' (aka 'long') to
      'typename TransposeImpl<Block<DynamicSparseMatrix<float, 0, int>, -1, -1, false>, Sparse>::Index' (aka 'int') [-Wshorten-64-to-32]
      typename Traits::_RhsNested::InnerIterator it(rhs, outer);
                                                 ~~      ^~~~~

In SparseDenseOuterProduct if Index type of the Lhs is narrower than Index of Rhs, then coeff access on Rhs shows this warning.  I saw no possibility to promote the Index type.  outer must be converted explicitly in SparseDenseProduct.h.

--

eigen/unsupported/test/NonLinearOptimization.cpp:249:23: warning: implicit conversion loses integer precision: 'Index' (aka 'long') to 'const int'
      [-Wshorten-64-to-32]
        const int n = x.size();

Fixed by using VectorXd::Index instead of int in NonLinearOptimization.cpp.

--

eigen/Eigen/src/Core/DenseCoeffsBase.h:499:60: warning: implicit conversion loses integer precision: 'Index' (aka 'long') to 'Index' (aka 'int')
      [-Wshorten-64-to-32]
      derived().coeffRef(row, col) = other.derived().coeff(row, col);
                                     ~~~~~                 ^~~

In class DenseCoeffsBase the methods copyPacket(), copyCoeff() might access the other objects with an wider Index type.  Fixed this with explicit conversions.

--

eigen/Eigen/src/SparseCore/SparseMatrix.h:1076:23: warning: implicit conversion loses integer precision: 'Index' (aka 'long') to 'Index' (aka 'int')
      [-Wshorten-64-to-32]
    SparseMatrix dest(other.rows(),other.cols());
                 ~~~~ ^~~~~~~~~~~~

In operator= of SparseMatrix the Index type of the source may be narrower than those of the destination matrix.  Solved by explicit conversion.

--

eigen/Eigen/src/Core/AssignEvaluator.h:426:112: warning: implicit conversion loses integer precision: 'Index' (aka 'long') to 'int' [-Wshorten-64-to-32]
      copy_using_evaluator_innervec_InnerUnrolling<Kernel, 0, DstXprType::InnerSizeAtCompileTime>::run(kernel, outer);
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~                                                             ^~~~~

The run method takes an hard coded int as index.  Solved by using Index type of Kernel for the function argument.

--

eigen/Eigen/src/Core/CoreEvaluators.h:580:19: warning: implicit conversion loses integer precision: 'Index' (aka 'long') to 'int' [-Wshorten-64-to-32]
      m_rowStride(map.rowStride()),
                 ~^~~~~~~~~~~~~~~

Class members where hard coded ints.  Solved by converting to members of type Index.

--

eigen/Eigen/src/Core/Diagonal.h:161:14: warning: implicit conversion loses integer precision: 'long' to 'int' [-Wshorten-64-to-32]
      return m_index.value();
      ~~~~~~ ^~~~~~~~~~~~~~~

Diagonal::index() returned hard coded int type.  Solved by converting return type to Index.
Also added obviously forgotten inline keyword to member function.

--

eigen/Eigen/src/IterativeLinearSolvers/IncompleteLUT.h:34:13: warning: implicit conversion loses integer precision: 'Index' (aka 'long') to 'int'
      [-Wshorten-64-to-32]
  Index n = row.size(); /* length of the vector */
        ~   ^~~~~~~~~~

Function template QuickSplit receives Index typename from function argument.  If this is an int it may not match to the Index type of the Vector function argument.  Solved by removing the independent Index template parameter and using Index of vector type instead.

--

eigen/Eigen/src/SparseLU/SparseLU_SupernodalMatrix.h:236:15: warning: implicit conversion loses integer precision: 'Index' (aka 'long') to 'Index' (aka 'int')
      [-Wshorten-64-to-32]
    Index n = X.rows(); 
          ~   ^~~~~~~~

In MappedSuperNodalMatrix<Scalar,Index>::solveInPlace() the Index of the destination matrix may be wider than the class Index type.  Solved by explicit conversion.

--

eigen/Eigen/src/SparseLU/SparseLU.h:690:15: warning: implicit conversion loses integer precision: 'Index' (aka 'long') to 'Index' (aka 'int')
      [-Wshorten-64-to-32]
    Index n = X.rows();
          ~   ^~~~~~~~

Same as for MappedSuperNodalMatrix.

--

eigen/Eigen/src/SparseLU/SparseLU_panel_bmod.h:209:54: warning: implicit conversion loses integer precision: 'long' to 'int' [-Wshorten-64-to-32]
              if(segsize==1)  LU_kernel_bmod<1>::run(segsize, dense_col, tempv, glu.lusup, luptr, lda, nrow, glu.lsub, lptr, no_zeros);
                              ~~~~~~~~~~~~~~         ^~~~~~~

Hard coded int types used in method arguments.  Solved by using Index type arguments.

--

eigen/Eigen/src/misc/SparseSolve.h:39:38: warning: implicit conversion loses integer precision: 'Index' (aka 'long') to 'Index' (aka 'int') [-Wshorten-64-to-32]
  inline Index rows() const { return m_dec.cols(); }
                              ~~~~~~ ^~~~~~~~~~~~

Class sparse_solve_retval_base defined Index as Index of Lhs.  Fixed by using promoted Index of Lhs and Rhs.

--

eigen/Eigen/src/SparseQR/SparseQR.h:595:38: warning: implicit conversion loses integer precision: 'Index' (aka 'long') to 'Index' (aka 'int')
      [-Wshorten-64-to-32]
  inline Index cols() const { return m_other.cols(); }
                              ~~~~~~ ^~~~~~~~~~~~~~

The Index type of class SparseQR_QProduct may not correspond to those of member m_other.  Fixed by explicit conversion.

--

eigen/unsupported/test/../../Eigen/src/Core/ReturnByValue.h:63:58: warning: implicit conversion loses integer precision: 'Index' (aka 'long') to 'Index'
      (aka 'int') [-Wshorten-64-to-32]
    EIGEN_DEVICE_FUNC inline Index rows() const { return static_cast<const Derived*>(this)->rows(); }
                                                  ~~~~~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The ReturnType of KroneckerProductSparse was defined with Index=int.  Fixed by defining it to be equal to promoted index of Rhs::Index and Lhs::Index.

--

eigen/unsupported/Eigen/src/KroneckerProduct/KroneckerTensorProduct.h:154:28: warning: implicit conversion loses integer precision: 'Index' (aka 'long') to
      'Index' (aka 'int') [-Wshorten-64-to-32]
  dst.resize(this->rows(), this->cols());
  ~~~                      ^~~~~~~~~~~~

Mixing of Rhs::Index and Lhs:Index in various loop variables.

--

eigen/unsupported/Eigen/src/LevenbergMarquardt/LMqrsolv.h:35:15: warning: implicit conversion loses integer precision: 'Index' (aka 'long') to 'int'
      [-Wshorten-64-to-32]
    Index n = s.cols();
          ~   ^~~~~~~~

Mixing of types of permutation matrix index and other matrix index in template function void lmqrsolv().

Fixed compiler warning on implicit integer conversion by separating index type for matrix and permutation matric which may not be equal.

--

I have prepared pull request #83 with the fixes for the default branch:
 https://bitbucket.org/eigen/eigen/pull-request/83

All changesets are independent from each other as they fix different warnings. Please have a look at it.
Comment 1 Gael Guennebaud 2014-09-15 07:53:08 UTC
Thank you for looking at them and fixing them individually. I won't apply them to the default branch but in the evaluator branch because 1) some of them won't apply anymore (dead code), and 2) some are already fixed by deeper refactoring. This might take some time because the merge is not straightforward.
Comment 2 Georg Drenkhahn 2014-09-20 10:53:26 UTC
With Rev 6414 no more "implicit conversion" warnings are issued (clang 3.5 compiler).
This bug can be closed and the pull request should be rejected.
Comment 3 Georg Drenkhahn 2014-09-20 11:16:13 UTC
Forget about my last comment.  I saw too late that the warnings were not enabled.
I will have a look at them and will propose some new fixes.
Comment 4 Georg Drenkhahn 2014-09-22 22:47:55 UTC
I prepared a new pull request which fixes of the easy to fix implicit integer conversion warnings.
https://bitbucket.org/eigen/eigen/pull-request/85

There are still issues for which I do not have a solution proposal.
Comment 5 Gael Guennebaud 2014-09-29 10:35:39 UTC
I applied all changes, and re-enabled the respective warning.
Comment 6 Christoph Hertzberg 2014-09-29 12:40:32 UTC
A small suggestion: It might be a good idea to hide Index conversions from bigger to smaller types into a function which also handles the assertions. At the moment we have a mixture of eigen_assert and eigen_internal_assert.

  template<typename IndexDest, typename IndexSrc>
  inline IndexDest convertIndex(const IndexSrc& idx) {
    eigen_assert(idx <= NumTraits<IndexDest>::highest() && "Index to big ...");
    return IndexDest(idx);
  }

Usage:
  StorageIndex idx = convertIndex<StorageIndex>(idx_);

At the moment, we don't really have a lot of conversions, so it is not too important.
Comment 7 Gael Guennebaud 2014-09-29 14:45:12 UTC
Very good idea. This will also permit to easily find then for future enhancements, debugging....
Comment 8 Georg Drenkhahn 2014-09-30 21:24:09 UTC
Another suggestion for the index conversion: The runtime asserts are only needed for conversions to narrower type.  The following templates take care of this:

template<typename IndexDest, typename IndexSrc, bool WithAssert = (sizeof(IndexDest)<sizeof(IndexSrc))>
struct IndexConvert {
  static inline IndexDest run(const IndexSrc& idx) {
    return IndexDest(idx);
  }
};

// template specialization for conversion to narrower type w/ runtime overflow assert check
template<typename IndexDest, typename IndexSrc>
struct IndexConvert<IndexDest, IndexSrc, true> {
  static inline IndexDest run(const IndexSrc& idx) {
    eigen_assert(idx <= NumTraits<IndexDest>::highest() && "Index value to big for target type");
    return IndexDest(idx);
  }
};

template<typename IndexDest, typename IndexSrc>
inline IndexDest convertIndex(const IndexSrc& idx) {
  return IndexConvert<IndexDest, IndexSrc, (sizeof(IndexDest)<sizeof(IndexSrc))>::run(idx);
}
Comment 9 Christoph Hertzberg 2014-10-02 18:02:39 UTC
I started fixing this, but got stuck with some decisions:

1) At the moment when operations involve different Index types (most common when mixing SparseMatrix<..., int> with DenseMatrix) the generated expression has promote_index_type<Idx1, Idx2>::type as Index, which is the bigger of the two. That results in lots of conversion warnings, mostly when Iterators are initialized.
Wouldn't it make sense here, to use the _smaller_ of the index types as common index type and assert only at initialization that both sizes fit into that Index type?

2) It would simplify a lot of things, if PermutationMatrix would typedef its StorageIndexType as Index -- most importantly, when calculating products between PermutationMatrix and SparseMatrix. Maybe, for convenience, we could add a constructor which accepts arbitrary integer types and asserts that the value is smaller that NumTraits<Index>::highest().
An alternative could be to specialize the product-traits for PermutationMatrices, replacing PermutationType::Index by PermutationType::StorageIndexType.

3) Some pivoting decompositions use internal::plain_{col,row}_type<MatrixType, Index>::type instead of TranspositionsType to store transpositions, I guess that has just not been ported yet? Nevertheless, the transpositions should use the same type as the Permutation, and the probably the Index typedef of the decomposition should be the smallest of MatrixType::Index and PermutationType::Index. Or at least that should be used as the "working Index".
Comment 10 Christoph Hertzberg 2014-10-02 18:06:30 UTC
(In reply to Georg Drenkhahn from comment #8)
> Another suggestion for the index conversion: The runtime asserts are only
> needed for conversions to narrower type. [...]

I guess any decent compiler is capable of optimizing this assertion away. So I would save the template construct, unless someone observes a regression without it.

We might actually distinguish between assert and internal_asserts at some points, because some conversions should never fail, as their input data had been checked before -- on the other hand, in most cases this should better be caught by choosing the Index type properly.
Comment 11 Gael Guennebaud 2014-10-02 22:34:48 UTC
(In reply to Christoph Hertzberg from comment #9)
> 1) At the moment when operations involve different Index types (most common
> when mixing SparseMatrix<..., int> with DenseMatrix) the generated
> expression has promote_index_type<Idx1, Idx2>::type as Index, which is the
> bigger of the two. That results in lots of conversion warnings, mostly when
> Iterators are initialized.
> Wouldn't it make sense here, to use the _smaller_ of the index types as
> common index type and assert only at initialization that both sizes fit into
> that Index type?

For coefficient-wise operations mixing sparse and dense objects you are probably right, but when two sparse objects are involved, the sparse outcome is more likely to require more non-zeros than the inputs, and so so we should use the largest one.

> 2) It would simplify a lot of things, if PermutationMatrix would typedef its
> StorageIndexType as Index -- most importantly, when calculating products
> between PermutationMatrix and SparseMatrix. Maybe, for convenience, we could
> add a constructor which accepts arbitrary integer types and asserts that the
> value is smaller that NumTraits<Index>::highest().
> An alternative could be to specialize the product-traits for
> PermutationMatrices, replacing PermutationType::Index by
> PermutationType::StorageIndexType.

Then you'll get the same issues when mixing permutation and dense matrices. Actually, I'd rather change SparseMatrix to mimic PermutationMatrix wrt indices, that is, distinguish between the index type used for storage, and the "Index" type as used in dense Matrix (e.g., as iteration index in for loops, for rows(), cols(), row(Index), coeff(Index,Index), etc.).

> 3) Some pivoting decompositions use
> internal::plain_{col,row}_type<MatrixType, Index>::type instead of
> TranspositionsType to store transpositions, I guess that has just not been
> ported yet? Nevertheless, the transpositions should use the same type as the
> Permutation, and the probably the Index typedef of the decomposition should
> be the smallest of MatrixType::Index and PermutationType::Index. Or at least
> that should be used as the "working Index".

Indeed, they should rather use a Transpositions<int>, but DenseIndex should still be used for everything else as we observed a significant speedup when moving from int to std::ptrdiff_t.
Comment 12 Christoph Hertzberg 2014-10-03 00:19:35 UTC
(In reply to Gael Guennebaud from comment #11)
> For coefficient-wise operations mixing sparse and dense objects you are
> probably right, but when two sparse objects are involved, the sparse outcome
> is more likely to require more non-zeros than the inputs, and so so we
> should use the largest one.

How about always using ptrdiff_t/size_t for nnz, independent of IndexType?
But see 2).

> > 2) It would simplify a lot of things, if PermutationMatrix would typedef its
> > StorageIndexType as Index [...]
> 
> Then you'll get the same issues when mixing permutation and dense matrices.
> Actually, I'd rather change SparseMatrix to mimic PermutationMatrix wrt
> indices, that is, distinguish between the index type used for storage, and
> the "Index" type as used in dense Matrix (e.g., as iteration index in for
> loops, for rows(), cols(), row(Index), coeff(Index,Index), etc.).

You're absolutely right. It would actually be much simpler, if every function accepting Indexes would accept the same (ptrdiff_t) Index. We could save a lot of work determining the index type when accessing an element. And inside the accessed element, we only need to [internal_]assert (row<rows() && col<cols()) if required.
Actually, then I don't see a purpose of determining the Index type of an expression anymore. Maybe except CwiseBinary<Sparse, Sparse>, where I still believe that, e.g.
   (SparseMatrix<..., int>() + SparseMatrix<..., long>()).eval()
should always fit into a SparseMatrix<..., int>.


> > 3) [...] probably the Index typedef of the decomposition should
> > be the smallest of MatrixType::Index and PermutationType::Index. Or at least
> > that should be used as the "working Index".
> 
> Indeed, they should rather use a Transpositions<int>, but DenseIndex should
> still be used for everything else as we observed a significant speedup when
> moving from int to std::ptrdiff_t.

Yes, true again. We only need to assert here once that
  max(rows(), cols()) < NumTraits<PermIndex>::highest()
and for the rest, simply directly cast Index to PermIndex before inserting it into the transposition or permutation type.
Comment 13 Gael Guennebaud 2014-10-03 23:33:46 UTC
(In reply to Christoph Hertzberg from comment #12)
> > > 2) It would simplify a lot of things, if PermutationMatrix would typedef its
> > > StorageIndexType as Index [...]
> > 
> > Then you'll get the same issues when mixing permutation and dense matrices.
> > Actually, I'd rather change SparseMatrix to mimic PermutationMatrix wrt
> > indices, that is, distinguish between the index type used for storage, and
> > the "Index" type as used in dense Matrix (e.g., as iteration index in for
> > loops, for rows(), cols(), row(Index), coeff(Index,Index), etc.).
> 
> You're absolutely right. It would actually be much simpler, if every
> function accepting Indexes would accept the same (ptrdiff_t) Index. We could
> save a lot of work determining the index type when accessing an element. And
> inside the accessed element, we only need to [internal_]assert (row<rows()
> && col<cols()) if required.
> Actually, then I don't see a purpose of determining the Index type of an
> expression anymore. Maybe except CwiseBinary<Sparse, Sparse>, where I still
> believe that, e.g.
>    (SparseMatrix<..., int>() + SparseMatrix<..., long>()).eval()
> should always fit into a SparseMatrix<..., int>.

Recall that the index type passed to SparseMatrix is used to store the "inner indices" of the non zeros, but also the position of the first non-zero of each "inner vector". Consequently, we must ensure that highest(IndexType) is greater than nnz.
Comment 14 Christoph Hertzberg 2014-10-04 00:47:37 UTC
(In reply to Gael Guennebaud from comment #13)
> Recall that the index type passed to SparseMatrix is used to store the
> "inner indices" of the non zeros, but also the position of the first
> non-zero of each "inner vector". Consequently, we must ensure that
> highest(IndexType) is greater than nnz.

Oh yes, true again. So for sparse compressed format we should indeed use the bigger type. Theoretically, it might make sense to distinguish between inner and outer index type, but that would require breaking the API. It is quite unfortunate how much memory is wasted, if nnz>2^31. Basically, all the higher bits in the row-indices are wasted, as well as most higher bits of the column-indices.
Comment 15 Christoph Hertzberg 2014-10-14 17:18:00 UTC
I think we generally agreed that the easiest/cleanest solution is to make all Index types in the interface equal ptrdiff_t (resp. EIGEN_DEFAULT_DENSE_INDEX_TYPE).

The question is how to realize that (this concerns mostly the sparse API):
 A) Remove the local Index typedefs and only have a global Eigen::Index typedef. This breaks code for users who are using the current typedefs.
 B) Use Eigen::Index explicitly in the API. This requires a lot of search&replace
 C) Always typedef Eigen::ClassName::Index as Eigen::Index, for sparse objects add a typedef StorageIndex which equals the current Index (similar to, e.g., PermutationMatrix). This (silently) changes behavior for users who are using the current typedef.
 D) Other suggestions?

C) would be easiest to implement, but I'm not too happy with that, e.g., in cases where someone uses something like SparseMatrix<?, ?, typename OtherSparse::Index>.
I'm slightly tending to B) since it would have the lowest impact on user code.
Comment 16 Gael Guennebaud 2014-10-14 23:28:29 UTC
Ideally A seems to be the cleanest and simplest option, but unfortunately this would severely break API compatibility. So I cannot think about any better option than B because it would be too weird that SparseMatrix<?,?,Foo>::Index be different than Foo (even though SparseMatrixBase<>::Index is not documented). My biggest concern about this option is the discrepancy it introduces wrt PermutationMatrix/Transpositions. Perhaps, PermutationMatrix should be changed to match the implementation strategy we'll take for SparseMatrix. It's not too late.
Comment 17 Christoph Hertzberg 2014-10-31 20:56:19 UTC
I made a pull request:
https://bitbucket.org/eigen/eigen/pull-request/89/

Except for one minor issue, this should solve the problem.
I simply used ::Index instead of Eigen::Index, I'm not sure if that makes a difference somewhere. If it does, it should be easy to change.
Comment 18 Gael Guennebaud 2014-11-07 12:01:11 UTC
Cannot we find a better name for the globally defined Index type so that we reduce the ambiguities between the Index type used for storage and the API index type? The usage of the old Expr::Index and traits<Expr>::Index types would be deprecated as loop/API index types, and would be defined as the storage index type which for dense matrices is irrelevant and will remains an alias for ptrdiff_t. I know that implies a huge search and replace to clean Eigen's own implementation, but that seems to be worth the effort as the current situation is too confusing and error prone.
Comment 19 Christoph Hertzberg 2014-11-07 16:47:07 UTC
The required search&replace is annoying but manageable ...

I would still like to return the actually stored index type (for methods like rows()) in order to not cause conversion warnings when returned types are stored in smaller types -- obviously innerIndexPtr(), etc have to return the stored type anyways. 
But we shall accept the API index type for every method taking an index. 
Basically two decisions have to be made:
* Where shall we make dimension/conversion assertions? (Also internal or 
  normal asserts?) Currently, many (or all?) .coeff() methods blindly 
  dereference the internal arrays without asserting (not even internally)
  that the sizes suffice. If we keep that behavior, it will not make sense
  for conversion assertions (comment 6) here, either.
  Actually, hardly any conversion shall be needed anymore, except when
  really storing an index.
* How shall we name the API index? Suggestions:
 * IndexAPI, APIIndex (a bit long, but better than Eigen::Index)
 * IIndex, IndexI     (I for interface)
 * Idx, Indx          (too short?)
 * Index_             (we used _ only for internal things so far, 
                       also likely will cause name-clashes)
 * ?

And a minor question: Shall we also provide Eigen::Index as an alias, even though it won't be practically usable from inside Eigen most of the time?

NB: The reason why the pull-request happened to work is because there is a
  using namespace Eigen;
inside test/main.h. Maybe we should make some test files which ensure that including each (public) header individually does work (i.e. also no external dependencies are missing)
Comment 20 Christoph Hertzberg 2014-12-02 19:32:10 UTC
After some discussion on IRC, we decided to replace current occurrences of Index by StorageIndex everywhere and to introduce Eigen::Index as the global API-Index.

DenseBase<>::Index will become an alias for Eigen::Index (practically, that is already the case) and may get removed in the future.

At the moment I will not reintroduce SparseBase<>::Index typedefs, because they would interfere with the global Eigen::Index type, which is different from what, e.g., innerIdxPtr() returns. That way people using the (undocumented) SparseBase::Index type will get a (more or less) graceful error message.
Comment 21 Christoph Hertzberg 2014-12-04 22:58:32 UTC
New pull request:
https://bitbucket.org/eigen/eigen/pull-request/92/
Comment 22 Gael Guennebaud 2015-02-16 15:17:05 UTC
The pull request has been updated. In my opinion, it is good enough to be merged. To summarize, the changes are:

 - Introduce a global typedef Eigen::Index making Eigen::DenseIndex and AnyExpr<>::Index deprecated (default is std::ptrdiff_t).

 - Eigen::Index is used throughout the API to represent indices, offsets, and sizes.

 - Classes storing an array of indices uses the type StorageIndex to store them. This is a template parameter of the class. Default is int.

 - Methods that *explicitly* set or return an element of such an array take or return a StorageIndex type. In all other cases, the Index type is used.
Comment 23 Christoph Hertzberg 2015-02-16 15:26:29 UTC
(In reply to Gael Guennebaud from comment #22)
> The pull request has been updated. In my opinion, it is good enough to be
> merged.

I can't spot any flaws, either. Assuming that you ran the unit-tests, you have my go-ahead to merge. If there are defects left, merging is also the best way to find them (and as it is only the devel-branch, that should be ok).
Comment 24 Gael Guennebaud 2015-02-16 15:31:23 UTC
Merge done.
Comment 25 Nobody 2019-12-04 13:44:11 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to gitlab.com's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.com/libeigen/eigen/issues/877.

Note You need to log in before you can comment on or make changes to this bug.