This bugzilla service is closed. All entries have been migrated to https://gitlab.com/libeigen/eigen
Bug 1736 - Column access of some IndexedView won't compile
Summary: Column access of some IndexedView won't compile
Status: RESOLVED FIXED
Alias: None
Product: Eigen
Classification: Unclassified
Component: Core - expression templates (show other bugs)
Version: 3.4 (development)
Hardware: All All
: Normal Compilation Problem
Assignee: Nobody
URL:
Whiteboard:
Keywords: test-needed
Depends on:
Blocks: 3.4
  Show dependency treegraph
 
Reported: 2019-08-07 11:34 UTC by Antoine Bussy
Modified: 2019-12-04 18:44 UTC (History)
4 users (show)



Attachments

Description Antoine Bussy 2019-08-07 11:34:28 UTC
Hi,

The following code won't compile on clang-7 (I also tried gcc-9 and clang-8 on https://godbolt.org/):

#include <Eigen/Core>
#include <array>

auto indexed_view_col()
{
    auto const m = Eigen::Matrix4d::Random().eval();
    auto ok  = m(Eigen::all, Eigen::seq(1, Eigen::last)).col(0).eval();
    auto ok2 = m(Eigen::all, Eigen::seq(0, 1)).col(0).eval();
    auto err = m(Eigen::all, std::array<Eigen::Index,2>{0,1}).col(0).eval();
    auto err2= m(Eigen::all, Eigen::Vector<Eigen::Index,2>(0, 1)).col(0).eval();
}

The compilers don't seem to find the unary version of coeff(), and indeed, it isn't in IndexedView.h (whereas it is present in Block.h). I implemented it similarly to Block's:

  EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE
  CoeffReturnType coeff(Index index) const
  {
    return coeff((m_xpr.rows() == 1 ? 0 : index),
                 (m_xpr.rows() == 1 ? index : 0));
  }

  EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE
  Scalar& coeffRef(Index index)
  {
    return coeffRef((m_xpr.rows() == 1 ? 0 : index),
                 (m_xpr.rows() == 1 ? index : 0));
  }

and it seems to solve my problem. but I'm not familiar with Eigen's internals at all, so the root cause might be elsewhere?
Comment 1 Christoph Hertzberg 2019-08-07 17:18:03 UTC
The actual problem here is that `.coeff(Index)` is called -- even though only in a false-branch, i.e., never executed.
With C++17 this would be trivial to workaround using

   if constexpr (ForwardLinearAccess) {...}

Your patch should work of course (since it will never actually be called). But perhaps we should add assertions that the expression is a vector at compile time (actually, also to the corresponding Block::coeff methods)


Ideally, m(X, Y).col(i) could be simplified to m(X,Y[i]) -- similar for any other block-operations.
Comment 2 Antoine Bussy 2019-08-08 09:23:00 UTC
Thanks for the answer, adding constexpr in CoreEvaluator.h is indeed much cleaner :)

Concerning m(X,Y[i]), I can't really use this simplification in my code since m(X,Y) is the input of a function that works on matrices.

For a non-C++17 workaround, it is possible to emulate the if constexpr with a small amount of boilerplate:

EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE
CoeffReturnType coeff_impl(std::true_type, Index index) const
{ 
	return m_argImpl.coeff(m_linear_offset.value() + index); 
}

EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE
CoeffReturnType coeff_impl(std::false_type, Index index) const
{ 
	return coeff(RowsAtCompileTime == 1 ? 0 : index, RowsAtCompileTime == 1 ? index : 0);
}

EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE
CoeffReturnType coeff(Index index) const
{
	return coeff_impl(std::bool_constant<ForwardLinearAccess>{}, index);
}

Of course, it would be necessary to reimplement std::bool_constant, but this is trivial.
Comment 3 Gael Guennebaud 2019-09-11 13:41:52 UTC
Thank you for reporting this issue and suggesting a fix.

https://bitbucket.org/eigen/eigen/commits/f4920c83f9c4/
Comment 4 Nobody 2019-12-04 18:44:04 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/1736.

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