Using references or pointers for output parameters is an endless debate, back in 2008 we decided to go with pointers because of the '&' sign on the call site. That's how it is, and adding reference overload will just clutter the API.
The real solution is adding: auto [value, row, col] = mat.maxCoeffIdx(); This is also generalizable to rowwise/colwise, and we could also offer variants returning indices only: auto [row, col] = mat.maxIdx(); Index i = vec.maxIdx(); This would need some refinement to make the following ok (this situation might occur in generic code): auto [row, col] = vec.maxIdx(); gael On Thu, Mar 28, 2019 at 2:50 AM Meng Zhu <[email protected]> wrote: > just my two cents. > > On Mon, Mar 25, 2019 at 7:18 PM Yuanchen Zhu <[email protected]> > wrote: > >> Just to chime in a bit, I much prefer the pure-function style of >> returning both the min value and the associated index in a compound struct >> which is then unpacked with `auto [v, i] = ...`. I don't see why the >> function returning only the min value and the function returning both the >> min value and the index have to have the same name. >> > they don't have to, though having identical name seems more elegant, and > friendly to tooling discovery such as ide intellisense. see glRotated* > function series for a counter example. no overloading there results in an > abundance of related yet different names just for pure syntactic need. > > >> Now if people really have to pass a output parameter in order to return >> the index, my suggestion is to use `gsl::not_null`, i.e., a pointer that's >> guaranteed to be non-null. This way the semantics is clear. At usage cite, >> the "&" sign unequivocally indicates an output parameter. And you cannot >> pass a null without incurring assertion failure (for debug build at least). >> > in this particular instance, gsl::not_null is only a bandit, not a > solution. there are other bad pointers, nullptr is only one kind. wild > pointers, for example, do more harms. with nullptr, at least you get an > assertion or exception in runtime and notice the bug right away. with wild > pointers, the UB incurred may lurk for years and randomly manifest from > time to time and take huge effort to debug. > > I don't see how being able to have an & sign on the call site justifies > all the weakness pointers inherently carry. moreover, there is std::swap, > so c++ programmers are expected and kind of forced to work with lvalue > reference out param anyway. I think eigen should at least provide reference > overload to open up such usage. over time, it also provides real data about > whether pointer or reference out param is used more often in reality, > allowing to make data-driven decisions. or maybe that discussion has been > done and the current status quo is already an informed consensus? if so, > someone knows eigen better please enlighten. thanks. > > >> On Mon, Mar 25, 2019 at 12:51 AM Michael Hofmann <[email protected]> >> 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 <[email protected]> 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 < >>> [email protected]> 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 < >>> [email protected]> 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 <[email protected]> >>> 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: [email protected] >>> >> > >>> >> > 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 >>> >> > ------------------------------------------------------------- >>> >> > >>> >> > >>> >> > >>> >> >>> >> >>> >> >>> >>> >>>
