Topic: std::minmax considered harmful
Author: "Niels Dekker - no reply address" <invalid@this.is.invalid>
Date: Thu, 16 Sep 2010 11:36:31 CST Raw View
[ Repost -- originally posted on Tue, 31 Aug 2010. ]
Sebastian (SG) wrote:
> double f();
> double g();
> ...
> auto mm = minmax(f(),g());
>
> At first sight, this seems harmless, but mm.first and mm.second are in
> fact dangling references.
...
> In my opinion, this should be fixed by using rvalue references and the
> common_type trait so that if at least one argument is an rvalue, the
> resulting pair also contains values instead of references
Does N3106, "Proposed Resolution for US 122" by Nicolai Josuttis take away
your concerns?
http://www.open-std.org/JTC1/sc22/WG21/docs/papers/2010/n3106.pdf It will
allow you to do:
auto mm = minmax( { f(),g() } );
According to N3106, minmax(initializer_list<T> t) will return pair<T,T>,
instead of pair<T&,T&>. So if you don't want a pair of references, just add
those curly braces!
BTW, personally I'd prefer to have all of the minmax overloads return a pair
of references, in order to avoid CopyConstructible requirements and
potential exceptions (while copying). However, to my regret, LWG issue
#1308, "Concerns about initializer_list overloads of min, max, and minmax",
didn't make it:
http://www.open-std.org/JTC1/sc22/WG21/docs/lwg-closed.html#1308
Kind regards, Niels
--
Niels Dekker
http://www.xs4all.nl/~nd/dekkerware
Scientific programmer at LKEB, Leiden University Medical Center
[ comp.std.c++ is moderated. To submit articles, try just posting with ]
[ your news-reader. If that fails, use
mailto:std-c++@netlab.cs.rpi.edu<std-c%2B%2B@netlab.cs.rpi.edu>
]
[ --- Please see the FAQ before posting. --- ]
[ FAQ: http://www.comeaucomputing.com/csc/faq.html ]
Author: "Niels Dekker - no reply address" <invalid@this.is.invalid>
Date: Thu, 16 Sep 2010 11:36:46 CST Raw View
Sebastian (SG) wrote:
> double f();
> double g();
> ...
> auto mm = minmax(f(),g());
>
> At first sight, this seems harmless, but mm.first and mm.second are in
> fact dangling references.
...
> In my opinion, this should be fixed by using rvalue references and the
> common_type trait so that if at least one argument is an rvalue, the
> resulting pair also contains values instead of references
Does N3106, "Proposed Resolution for US 122" by Nicolai Josuttis take away
your concerns?
http://www.open-std.org/JTC1/sc22/WG21/docs/papers/2010/n3106.pdf It will
allow you to do:
auto mm = minmax( { f(),g() } );
According to N3106, minmax(initializer_list<T> t) will return pair<T,T>,
instead of pair<T&,T&>. So if you don't want a pair of references, just add
those curly braces!
BTW, personally I'd prefer to have all of the minmax overloads return a pair
of references, in order to avoid CopyConstructible requirements and
potential exceptions (while copying). However, to my regret, LWG issue
#1308, "Concerns about initializer_list overloads of min, max, and minmax",
didn't make it:
http://www.open-std.org/JTC1/sc22/WG21/docs/lwg-closed.html#1308
Kind regards, Niels
--
Niels Dekker
http://www.xs4all.nl/~nd/dekkerware
Scientific programmer at LKEB, Leiden University Medical Center
[ comp.std.c++ is moderated. To submit articles, try just posting with ]
[ your news-reader. If that fails, use
mailto:std-c++@netlab.cs.rpi.edu<std-c%2B%2B@netlab.cs.rpi.edu>
]
[ --- Please see the FAQ before posting. --- ]
[ FAQ: http://www.comeaucomputing.com/csc/faq.html ]
Author: SG <s.gesemann@gmail.com>
Date: Sun, 19 Sep 2010 16:54:13 CST Raw View
On 16 Sep., 19:36, Niels Dekker wrote:
> Sebastian (SG) wrote:
> > double f();
> > double g();
> > ...
> > auto mm = minmax(f(),g());
>
> > [...] mm.first and mm.second are in fact dangling references.
> ...
> > In my opinion, this should be fixed by using rvalue references and the
> > common_type trait so that if at least one argument is an rvalue, the
> > resulting pair also contains values instead of references
>
> Does N3106, "Proposed Resolution for US 122" by Nicolai Josuttis take awa=
y
> your concerns?http://www.open-std.org/JTC1/sc22/WG21/docs/papers/2010/n31=
06.pdfIt will
> allow you to do:
>
> =A0auto mm = minmax( { f(),g() } );
Sure. But that requires you to be aware of the dangling reference
issue with the harmless looking
auto mm = minmax( f(), g() );
cout << mm.first; // Oops!
> BTW, personally I'd prefer to have all of the minmax overloads return a p=
air
> of references, in order to avoid CopyConstructible requirements and
> potential exceptions (while copying).
My proposal doesn't rule out pairs with reference members. Example:
int i = 23;
int j = 42;
auto mm = minmax(i,j);
i = 99;
cout << mm.first;
would print 99. decltype(mm) is a pair<int&,int&> because i and j are
reference-compatible lvalues. Perhaps, const references are more
appropriate. The proposal just rules out reference members in case at
least one argument was an rvalue. And even if you want to have
references in these cases as well you can still use a
reference_wrapper:
// C++0x doesn't allow std::ref and std::cref to be used on rvalues
because
// that's usually a bad idea (hint hint)
template<class T>
auto anyref(T && x) -> decltype(std::ref(x)) {return std::ref(x);}
void foo(pair<const int&, const int&> const& x);
void bar() {
foo(minmax(anyref(23),anyref(42)));
}
In my opinion the dangling references problem with the current version
of this std::minmax overload is a big issue while exception safety or
type requirements is no issue at all since the proposal still supports
reference members in safe situations and allows you to use
reference_wrappers to override this policy in case the user explicitly
wants the pair to have reference members.
Cheers!
SG
--
[ comp.std.c++ is moderated. To submit articles, try just posting with ]
[ your news-reader. If that fails, use mailto:std-c++@netlab.cs.rpi.edu]
[ --- Please see the FAQ before posting. --- ]
[ FAQ: http://www.comeaucomputing.com/csc/faq.html ]
Author: SG <s.gesemann@gmail.com>
Date: Mon, 30 Aug 2010 20:11:05 CST Raw View
Hello!
I've been skimming the most recent C++0x draft (N3126) and noticed the
function templates std::minmax. What I'm concerned about is that these
function templates -- since they return pair<const T&,const T&> --
interact quite badly with the auto type deduction feature. Since the
function returns basically two things, it is natural to store the
object and access its members later. One might be tempted to use the
new auto feature like this:
double f();
double g();
...
auto mm = minmax(f(),g());
At first sight, this seems harmless, but mm.first and mm.second are in
fact dangling references. With the other min and max functions that
only return 'const T&' this dangling reference problem is not as
severe because the references are not hidden behind a class and 'auto'
is never replaced by a reference type.
In my opinion, this should be fixed by using rvalue references and the
common_type trait so that if at least one argument is an rvalue, the
resulting pair also contains values instead of references:
template<class T, class U>
pair<
typename common_type<T,U>::type,
typename common_type<T,U>::type
>
minmax(T && t, U && u)
{
if (u<t) return {u,t}; else return {t,u};
}
This still allows the function template to return a pair of references
(when the arguments are reference-compatible lvalues) but it also
avoids the dangling reference problem in the example from above which
I guess will be a common trap when using std::minmax.
thanks for your attention,
Sebastian
--
[ comp.std.c++ is moderated. To submit articles, try just posting with ]
[ your news-reader. If that fails, use mailto:std-c++@netlab.cs.rpi.edu]
[ --- Please see the FAQ before posting. --- ]
[ FAQ: http://www.comeaucomputing.com/csc/faq.html ]