Re: [eigen] Can we prevent that this compiles?

[Benoit Jacob <jacob.benoit.1@xxxxxxxxx>]

Hi,

Thanks for investigating this.

I think that we can do simpler while staying with our usual
static_assert's which allow better error messages (especially for
users who enable the static_assert keyword... just saying that this is
the future).

The basic problem was that the Replicate constructor was taking an
argument of type const MatrixType&, hence by the time it received this
argument, it had already been casted to that type, and the information
of the original type of that data was lost.

So instead, why not make this constructor templated in
"OriginalMatrixType" and then do a static assertion that this is the
same type as MatrixType.

Attached patch.

Compiler output (gcc 4.4, default options):

$ g++ a.cpp -o a -I eigen2 && ./a
In file included from eigen2/Eigen/Array:38,
                 from a.cpp:1:
eigen2/Eigen/src/Array/Replicate.h: In constructor
‘Eigen::Replicate<MatrixType, RowFactor, ColFactor>::Replicate(const
OriginalMatrixType&) [with OriginalMatrixType =
Eigen::Block<Eigen::Matrix<double, 33331, 33331, 0, 33331, 33331>, 1,
33331, 1, 32>, MatrixType = Eigen::Matrix<double, 33331, 33331, 0,
33331, 33331>, int RowFactor = 2, int ColFactor = 1]’:
a.cpp:8:   instantiated from here
eigen2/Eigen/src/Array/Replicate.h:76: error:
‘THE_MATRIX_OR_EXPRESSION_THAT_YOU_PASSED_DOES_NOT_HAVE_THE_EXPECTED_TYPE’
is not a member of ‘Eigen::ei_static_assert<false>’


of course it looks even much nicer with --std=c++0x:

$ g++ a.cpp -o a -I eigen2 --std=c++0x && ./a
In file included from eigen2/Eigen/Array:38,
                 from a.cpp:1:
eigen2/Eigen/src/Array/Replicate.h: In constructor
‘Eigen::Replicate<MatrixType, RowFactor, ColFactor>::Replicate(const
OriginalMatrixType&) [with OriginalMatrixType =
Eigen::Block<Eigen::Matrix<double, 33331, 33331, 0, 33331, 33331>, 1,
33331, 1, 32>, MatrixType = Eigen::Matrix<double, 33331, 33331, 0,
33331, 33331>, int RowFactor = 2, int ColFactor = 1]’:
a.cpp:8:   instantiated from here
eigen2/Eigen/src/Array/Replicate.h:76: error: static assertion failed:
"THE_MATRIX_OR_EXPRESSION_THAT_YOU_PASSED_DOES_NOT_HAVE_THE_EXPECTED_TYPE"

Makes me think that in that case we should make natural strings.

Benoit





2009/9/30 Hauke Heibel <hauke.heibel@xxxxxxxxxxxxxx>:
> Here, this patch is working. When Gael and/or Benoit agree, I could submit
> it. Though the error message on VC is not very nice...
>
> Hauke
>
> ---------- error message - begin ----------
> Error    1    error C2664:
> 'Eigen::Replicate<MatrixType,RowFactor,ColFactor>::Replicate(const
> Eigen::Replicate<MatrixType,RowFactor,ColFactor>::MustBeInstantiatedWith<MatrixType>
> &)' : cannot convert parameter 1 from
> 'Eigen::Block<MatrixType,BlockRows,BlockCols>' to 'const
> Eigen::Replicate<MatrixType,RowFactor,ColFactor>::MustBeInstantiatedWith<MatrixType>
> &'    main.cpp    9
> ---------- error message - end ----------
>
>
> On Wed, Sep 30, 2009 at 5:18 PM, Markus Moll <markus.moll@xxxxxxxxxxxxxxxx>
> wrote:
>>
>> Hi
>>
>> On Wednesday 30 September 2009 17:05:28 Hauke Heibel wrote:
>> > With static asserts we can make the compile error self explanatory. I
>> > preferred the ctor approach since it does not require implementing a
>> > ConstRef class.
>>
>> Hm, but that can be a _very_ thin wrapper even declared inside Replicate
>> (basically the code I gave is sufficient). It doesn't need to be a
>> full-blown
>> ConstRef class with all its problems. The more I think about it, the less
>> I
>> like the template solution. Even with static asserts, the error message
>> will
>> look radically different from what a user might expect. And the type
>> ConstRef
>> could be named "MustBeInstantiatedFrom" or "ExactType", so that the
>> candidate
>> list would read:
>>
>> candidates are:
>>  Replicate::Replicate(ExactType<const T&>)
>>  Replicate::Replicate(const Replicate&);
>>  [... whatever else ...]
>>
>> Markus
>>
>> --
>> PGP key on www.esat.kuleuven.be/~mmoll/public_key.pgp
>> Fingerprint is
>> 90C4 B47D 1A00 5AC1 9147  3197 EDA7 1E0E 99E4 9EDB
>>
>>
>
>

# HG changeset patch
# User Benoit Jacob <jacob.benoit.1@xxxxxxxxx>
# Date 1254340031 14400
# Node ID 273d1d0c613a2e31958d962e68a68cb3e68c6576
# Parent  eb9b807278ec5bc264cfa562d900fdcfd486fa75
make Replicate ctor require the exact expected type

diff -r eb9b807278ec -r 273d1d0c613a Eigen/src/Array/Replicate.h
--- a/Eigen/src/Array/Replicate.h	Wed Sep 30 16:48:02 2009 +0200
+++ b/Eigen/src/Array/Replicate.h	Wed Sep 30 15:47:11 2009 -0400
@@ -69,15 +69,22 @@
 
     EIGEN_GENERIC_PUBLIC_INTERFACE(Replicate)
 
-    inline Replicate(const MatrixType& matrix)
+    template<typename OriginalMatrixType>
+    inline Replicate(const OriginalMatrixType& matrix)
       : m_matrix(matrix), m_rowFactor(RowFactor), m_colFactor(ColFactor)
     {
+      EIGEN_STATIC_ASSERT((ei_is_same_type<MatrixType,OriginalMatrixType>::ret),
+                          THE_MATRIX_OR_EXPRESSION_THAT_YOU_PASSED_DOES_NOT_HAVE_THE_EXPECTED_TYPE)
       ei_assert(RowFactor!=Dynamic && ColFactor!=Dynamic);
     }
 
-    inline Replicate(const MatrixType& matrix, int rowFactor, int colFactor)
+    template<typename OriginalMatrixType>
+    inline Replicate(const OriginalMatrixType& matrix, int rowFactor, int colFactor)
       : m_matrix(matrix), m_rowFactor(rowFactor), m_colFactor(colFactor)
-    {}
+    {
+      EIGEN_STATIC_ASSERT((ei_is_same_type<MatrixType,OriginalMatrixType>::ret),
+                          THE_MATRIX_OR_EXPRESSION_THAT_YOU_PASSED_DOES_NOT_HAVE_THE_EXPECTED_TYPE)
+    }
 
     inline int rows() const { return m_matrix.rows() * m_rowFactor.value(); }
     inline int cols() const { return m_matrix.cols() * m_colFactor.value(); }
diff -r eb9b807278ec -r 273d1d0c613a Eigen/src/Core/util/StaticAssert.h
--- a/Eigen/src/Core/util/StaticAssert.h	Wed Sep 30 16:48:02 2009 +0200
+++ b/Eigen/src/Core/util/StaticAssert.h	Wed Sep 30 15:47:11 2009 -0400
@@ -77,7 +77,8 @@
         THIS_METHOD_IS_ONLY_FOR_ROW_MAJOR_MATRICES,
         INVALID_MATRIX_TEMPLATE_PARAMETERS,
         BOTH_MATRICES_MUST_HAVE_THE_SAME_STORAGE_ORDER,
-        THIS_METHOD_IS_ONLY_FOR_DIAGONAL_MATRIX
+        THIS_METHOD_IS_ONLY_FOR_DIAGONAL_MATRIX,
+        THE_MATRIX_OR_EXPRESSION_THAT_YOU_PASSED_DOES_NOT_HAVE_THE_EXPECTED_TYPE
       };
     };