Re: [eigen] maxCoeff taking reference?

On Mon, Mar 25, 2019 at 12:51 AM Michael Hofmann <kmhofmann@xxxxxxxxx> wrote:

I agree. Using pointers as output parameters (as opposed to
references) seems a bit inspired by the Google coding guidelines, but
these have always seemed, uhm, sub-optimal to me.

I find the following much more consistent and obvious:
- const references (or values) designate input parameters,
- const pointers designate purely optional input parameters,
- non-const references designate output parameters, and
- non-const pointers designate purely optional output parameters.

In this scheme, there is no confusion possible; a non-const reference
should never serve as input parameter.
But in most cases, non-optional output parameters should go into the
return type in the first place, and non-const references should be
avoided.

Some may argue that it's harder to see at the call site what a
parameter is, i.e. without looking at the function declaration. In my
opinion this is negligible, but the cost for using pointers as output
variables is incredibly high: the function gets harder to use (have to
add that extra '*', which *should* mean optional!), and much easier to
mis-use (can simply pass nullptr -> boom!).

Best,
Michael

On Mon, Mar 25, 2019 at 5:56 AM Meng Zhu <meng@xxxxxxxxxxxxx> wrote:
>
> trying to understand the justification here. wouldn't pointers as out parameters easily lead to bugs, such as null pointers, or wild pointers? as I read today's [eigen source here](https://github.com/eigenteam/eigen-git-mirror/blob/667b550a1f161ac85bc8f2013939a0adc10af12c/Eigen/src/Core/Visitor.h#L278), `*rowPtr = maxVisitor.row;` looks like a real bug. with references you
> don't get this kind of bugs, so why not?
>
> as for maxCoeff to return coord, does it make sense to use tags to express programmer intention, e.g.,
> auto [value, row, col] = A.maxCoeff(Eigen::return_coord);
>
> basically a form of tag dispatching, but only for overload purpose to live with the original version to cover all usage.
>
> On Fri, Mar 22, 2019 at 9:20 AM David Tellenbach <david.tellenbach@xxxxxxxxxxxxx> wrote:
>>
>> Hello,
>>
>> > We must make sure that this does not introduce overhead if the index(es) is not used. Perhaps with some template magic it is possible to distinguish calling
>> >
>> > auto [value, row,col] = A.maxCoeff();
>> > vs
>> > auto value = A.maxCoeff();
>>
>> I guess it is not possible to realise something like this without (unnecessarily) calculating the indices in the case
>>
>> auto value = A.maxCoeff();
>>
>> If we allow the unnecessary index calculation this is easy to implement. Distinguishing function calls based on return types is usually really hard because return types are not used for template deduction.
>>
>> > If that is not possible, we could of course just give one method a different name.
>>
>> Another alternative would be a static function maxCoeff(A) that returns a struct (or a tuple) but this might introduce more confusion.
>>
>> Cheers,
>> David
>>
>> > On 22. Mar 2019, at 11:54, Christoph Hertzberg <chtz@xxxxxxxxxxxxxxxxxxxxxxxx> wrote:
>> >
>> > We must make sure that this does not introduce overhead if the index(es) is not used. Perhaps with some template magic it is possible to distinguish calling
>> >
>> > auto [value, row,col] = A.maxCoeff();
>> > vs
>> > auto value = A.maxCoeff();
>> >
>> > This would actually be very close to how Matlab "overloads" many functions. If that is not possible, we could of course just give one method a different name.
>> >
>> > Btw: I agree on not overloading for references, as it should be obvious that these are out-parameters. O.t.o.h., with pointers there is always the ambiguity if null-pointers are allowed or not.
>> >
>> > Christoph
>> >
>> > On 22/03/2019 09.45, Gael Guennebaud wrote:
>> >> We chose pointers to emphasise they are out parameters. I would not add
>> >> overloads taking references but maybe a version returning everything within
>> >> a struct to be used with C++17 structure bindings?
>> >> On Thu, Mar 21, 2019 at 3:27 AM Meng Zhu <meng@xxxxxxxxxxxxx> wrote:
>> >>> hi, I noticed eigen maxCoeff function (
>> >>> https://eigen.tuxfamily.org/dox/classEigen_1_1DenseBase.html#a784e23ccbb39e7c57b70af386f94f8b5)
>> >>> takes pointers to return the max entry coord, and there is no overload to
>> >>> take reference type. is there a reason to not provide reference access?
>> >>> thanks.
>> >>>
>> >>> Meng
>> >>>
>> >>>
>> >>>
>> >
>> > --
>> > Dr.-Ing. Christoph Hertzberg
>> >
>> > Besuchsadresse der Nebengeschäftsstelle:
>> > DFKI GmbH
>> > Robotics Innovation Center
>> > Robert-Hooke-Straße 5
>> > 28359 Bremen, Germany
>> >
>> > Postadresse der Hauptgeschäftsstelle Standort Bremen:
>> > DFKI GmbH
>> > Robotics Innovation Center
>> > Robert-Hooke-Straße 1
>> > 28359 Bremen, Germany
>> >
>> > Tel.: +49 421 178 45-4021
>> > Zentrale: +49 421 178 45-0
>> > E-Mail: christoph.hertzberg@xxxxxxx
>> >
>> > Weitere Informationen: http://www.dfki.de/robotik
>> > -------------------------------------------------------------
>> > Deutsches Forschungszentrum für Künstliche Intelligenz GmbH
>> > Trippstadter Strasse 122, D-67663 Kaiserslautern, Germany
>> >
>> > Geschäftsführung:
>> > Prof. Dr. Jana Koehler (Vorsitzende)
>> > Dr. Walter Olthoff
>> >
>> > Vorsitzender des Aufsichtsrats:
>> > Prof. Dr. h.c. Hans A. Aukes
>> > Amtsgericht Kaiserslautern, HRB 2313
>> > -------------------------------------------------------------
>> >
>> >
>> >
>>
>>
>>