Re: [eigen] Feature proposal - add error string to help debug const/non-const mapping errors

[Rohit Garg <rpg.314@xxxxxxxxx>]

On Mon, Mar 4, 2013 at 3:22 AM, Gael Guennebaud
<gael.guennebaud@xxxxxxxxx> wrote:
> Is the standard error message that bad? Here is with gcc:
>
> ll.cpp: In function 'int main()':
> ll.cpp:58:32: error: invalid conversion from 'const double*' to
> 'Eigen::Map<Eigen::Matrix<double, -1, -1> >::PointerArgType {aka
> double*}' [-fpermissive]
> In file included from .../Eigen/Core:299:0,
>                  from .../Eigen/SparseCore:4,
>                  from .../Eigen/Sparse:17,
>                  from ll.cpp:3:
> .../Eigen/src/Core/Map.h:164:12: error:   initializing argument 1 of
> 'Eigen::Map<MatrixType, MapOptions,
> StrideType>::Map(Eigen::Map<MatrixType, MapOptions,
> StrideType>::PointerArgType, Eigen::Map<MatrixType, MapOptions,
> StrideType>::Index, Eigen::Map<MatrixType, MapOptions,
> StrideType>::Index, const StrideType&) [with PlainObjectType =
> Eigen::Matrix<double, -1, -1>; int MapOptions = 0; StrideType =
> Eigen::Stride<0, 0>; Eigen::Map<MatrixType, MapOptions,
> StrideType>::PointerArgType = double*; Eigen::Map<MatrixType,
> MapOptions, StrideType>::Index = long int]' [-fpermissive]
>
> The first line tell me exactly what's wrong (conversion from const
> double* to double*).

I was able to debug it ultimately using the error that came on my
machine. It's not hopeless, but it would be better if there was a
string in there. Clang's errors are better though.

>
>
>
> With clang:
>
> ll.cpp:58:19: error: no matching constructor for initialization of
> 'Map<MatrixXd>'
>     Map<MatrixXd> m(ptr, 10, 10);
>                   ^ ~~~~~~~~~~~
> .../Eigen/src/Core/Map.h:164:12: note: candidate constructor not
> viable: 1st argument ('const double *') would lose const qualifier
>     inline Map(PointerArgType dataPtr, Index nbRows, Index nbCols,
> const StrideType& a_stride = StrideType())
>            ^
> .../Eigen/src/Core/Map.h:151:12: note: candidate constructor not
> viable: 1st argument ('const double *') would lose const qualifier
>     inline Map(PointerArgType dataPtr, Index a_size, const StrideType&
> a_stride = StrideType())
>            ^
> .../Eigen/src/Core/Map.h:139:12: note: candidate constructor not
> viable: requires at most 2 arguments, but 3 were provided
>     inline Map(PointerArgType dataPtr, const StrideType& a_stride =
> StrideType())
>            ^
> .../Eigen/src/Core/Map.h:104:79: note: candidate constructor (the
> implicit copy constructor) not viable: requires 1 argument, but 3 were
> provided
> template<typename PlainObjectType, int MapOptions, typename
> StrideType> class Map
>
>
> and more generally, when someone write a class with a member like this:
>
> struct Bar {
>   void foo(double* ptr);
> };
>
> nobody add a member: void foo(const double*) {  static_assert(false,
> "blabla"); }
>
> So it seems to me that such change would go a bit too far, and I'm not
> really sure it will ease understanding the problem.
>
> Finally, if you use MatrixXd::Map(ptr, rows, cols), then no
> const/non-const issue anymore!

Yes, that's right, I wanted to put const where I could to aid in
optimizations and stricter errors/warnings.


>
> gael
>
> On Mon, Mar 4, 2013 at 5:54 AM, Rohit Garg <rpg.314@xxxxxxxxx> wrote:
>> On Sun, Mar 3, 2013 at 7:06 AM, Christoph Hertzberg
>> <chtz@xxxxxxxxxxxxxxxxxxxxxxxx> wrote:
>>> On 03.03.2013 00:58, Rohit Garg wrote:
>>>>
>>>> Hi all,
>>>>
>>>> I have been using the Map objects a lot lately, and I have seen that
>>>> often, a simple mistake of mapping a const pointer as, say, MatrixXd.
>>>> Obviously, this leads to a big compiler error, which is often
>>>> difficult to decipher. I propose that a small error string be added to
>>>> Map object, which will aid debugging. Something similar to
>>>> "you_mixed_different_types_use_cast_function".
>>>>
>>>> This could prove very helpful. I know 3.2 beta is being planned, just
>>>> how big a change is this?
>>>
>>>
>>> I guess that's a rather small change (basically provide a const Scalar*
>>> constructor which has a static assertion).
>>> But I would vote against suggesting to "use_cast" in the error string.
>>> Maybe something like expecting_a_non_const_pointer or
>>> check_your_const_correctness, though this might be also a bit confusing to
>>> the novice (but it would be easier to provide a FAQ entry for that).
>>
>> It's good to know that it's a small change. I mentioned the cast
>> string just to make a better reference to what I was talking about. I
>> should have phrased it better. Your suggestion for the error string is
>> a good one.
>>
>>
>>>
>>> Christoph
>>>
>>>
>>>
>>>
>>> --
>>> ----------------------------------------------
>>> Dipl.-Inf., Dipl.-Math. Christoph Hertzberg
>>> Cartesium 0.049
>>> Universität Bremen
>>> Enrique-Schmidt-Straße 5
>>> 28359 Bremen
>>>
>>> Tel: +49 (421) 218-64252
>>> ----------------------------------------------
>>>
>>>
>>
>>
>>
>> --
>> Rohit Garg
>>
>> http://rpg-314.blogspot.com/
>>
>> Graduate Student
>> Applied and Engineering Physics
>> Cornell University
>>
>>
>
>



-- 
Rohit Garg

http://rpg-314.blogspot.com/

Graduate Student
Applied and Engineering Physics
Cornell University