Basic question on operator overloading

[pauldepstein]

The following code was written by a colleague -- to preserve
confidentiality, the name of the class is changed:

HisClass operator+ (const HisClass & h1, const HisClass & h2)
{ // some code here}

HisClass HisClass:perator+ (const HisClass& h2)
{// more code here}


To me, this code appears to be in error because it appears to make
expressions such as h1 + h2 ambiguous.

It does compile though.

I'd like to run it by the newsgroup because it's an established part
of our library and I'd be a bit surprised to see a fundamental error
of this nature.

Am I missing something?

After writing this, I can now guess what happens. Is it the case that
only the 2nd version {//more code here} ever gets called because its
scope is narrower?


Thanks,

Paul Epstein

 

[Kai-Uwe Bux]

The following code was written by a colleague -- to preserve
confidentiality, the name of the class is changed:

HisClass operator+ (const HisClass & h1, const HisClass & h2)
{ // some code here}

HisClass HisClass:perator+ (const HisClass& h2)
{// more code here}


To me, this code appears to be in error because it appears to make
expressions such as h1 + h2 ambiguous.

It does compile though.

I'd like to run it by the newsgroup because it's an established part
of our library and I'd be a bit surprised to see a fundamental error
of this nature.

Am I missing something?

After writing this, I can now guess what happens. Is it the case that
only the 2nd version {//more code here} ever gets called because its
scope is narrower?

Well, it might be a little tricky to get at the non-member operator,
however, I think it is possible through conversion operators:

#include <iostream>
#include <ostream>


struct A {

A operator+ ( A const & other ) {
std::cout << "member\n";
return ( *this );
}

void member ( void ) {
*this + *this;
}

};

A operator+ ( A const & lhs, A const & rhs ) {
std::cout << "free\n";
return ( lhs );
}

struct B : public A {};

struct C {

operator A ( void ) {
return A();
}

};

int main ( void ) {
A a1;
A a2;

a1.member();
a1 + a2;

B b1;
B b2;
b1.member();
b1 + b2;
a1 + b1;
b1 + a1;

C c1;
C c2;

c1 + c2; // free version
a1 + c1;
c1 + a1; // free version

}


Best

Kai-Uwe Bux

 

[pauldepstein]

Well, it might be a little tricky to get at the non-member operator,
however, I think it is possible through conversion operators:

#include <iostream>
#include <ostream>

struct A {

A operator+ ( A const & other ) {
std::cout << "member\n";
return ( *this );
}

void member ( void ) {
*this + *this;
}

};

A operator+ ( A const & lhs, A const & rhs ) {
std::cout << "free\n";
return ( lhs );

}

struct B : public A {};

struct C {

operator A ( void ) {
return A();
}

};

int main ( void ) {
A a1;
A a2;

a1.member();
a1 + a2;

B b1;
B b2;
b1.member();
b1 + b2;
a1 + b1;
b1 + a1;

C c1;
C c2;

c1 + c2; // free version
a1 + c1;
c1 + a1; // free version

}

Best

Kai-Uwe Bux- Hide quoted text -

- Show quoted text -

Thanks Kai-Uwe

As a relative newbie, I don't quite understand your code.

In particular, I don't fully understand struct C.

How can you be sure in your code that the free version is being
called?

Do you have a good (preferably free online) text which covers this
issue?

However, I do think that I spotted a problem that was simply
overlooked by my colleague, and I believe that the non-member version
should just be commented out.

Thanks again,

Paul Epstein

 

[Alf P. Steinbach]

* (e-mail address removed):

The following code was written by a colleague -- to preserve
confidentiality, the name of the class is changed:

HisClass operator+ (const HisClass & h1, const HisClass & h2)
{ // some code here}

Since operator+ can at most have two arguments, the above cannot be a
function definition in the class definition. Hence it's a free-standing
definition of operator+. Not a member function.

HisClass HisClass:perator+ (const HisClass& h2)
{// more code here}

This one is a member function, and duplicates the functionality of the
former. However, it's not exactly the same: the formal argument for the
left hand side of h1+h2 is non-const.

To me, this code appears to be in error because it appears to make
expressions such as h1 + h2 ambiguous.

h1+h2 will select the member function unless h1 is const or temporary,
in which case the free-standing implementation is selected.

Assuming the code is correctly illustrated above, I think that is not
intentional.

Good programming practice dictates that operator+ should always have
const formal arguments.

It does compile though.

Yes, it should.

I'd like to run it by the newsgroup because it's an established part
of our library and I'd be a bit surprised to see a fundamental error
of this nature.

Am I missing something?

Only the exact nature of the bug, I think. ;-)

After writing this, I can now guess what happens. Is it the case that
only the 2nd version {//more code here} ever gets called because its
scope is narrower?

No, see above.

 

[Kai-Uwe Bux]

* (e-mail address removed):

Since operator+ can at most have two arguments, the above cannot be a
function definition in the class definition. Hence it's a free-standing
definition of operator+. Not a member function.




This one is a member function, and duplicates the functionality of the
former. However, it's not exactly the same: the formal argument for the
left hand side of h1+h2 is non-const.

Just a nit:

h1+h2 will select the member function unless h1 is const
agreed

or temporary,

I would expect a temporary to force the member version since temporaries
cannot bind to the const & parameters of the free-standing version.

in which case the free-standing implementation is selected.

[snip]



Best

Kai-Uwe Bux

 

[pauldepstein]

* (e-mail address removed):



Since operator+ can at most have two arguments, the above cannot be a
function definition in the class definition. Hence it's a free-standing
definition of operator+. Not a member function.


This one is a member function, and duplicates the functionality of the
former. However, it's not exactly the same: the formal argument for the
left hand side of h1+h2 is non-const.


h1+h2 will select the member function unless h1 is const or temporary,
in which case the free-standing implementation is selected.

Assuming the code is correctly illustrated above, I think that is not
intentional.

Good programming practice dictates that operator+ should always have
const formal arguments.


Yes, it should.



Only the exact nature of the bug, I think. ;-)


No, see above.

--
A: Because it messes up the order in which people normally read text.
Q: Why is it such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?

Alf,

Thanks for your answer. I'm a bit surprised.

I thought that scoping considerations overrode matching-parameter
considerations.

So I'm surprised that which version gets called can depend on a
constness issue.

Also, I'm a bit confused by "unless h1 is const or temporary".

I would not have thought it matters whether h1 is temporary or not
because I don't see how the compiler can recognise temporary
variables.

Also, are you saying that you would prefer HisClass HisClass:perator
+ (const HisClass& h2) const
{// .... }

Perhaps you haven't seen enough of the code to form an opinion, but I
believe that the member version should be commented out/ deleted.
(Before, I said this about the non-member version.)

Paul Epstein

 

[Kai-Uwe Bux]

Thanks Kai-Uwe

As a relative newbie, I don't quite understand your code.

In particular, I don't fully understand struct C.

How can you be sure in your code that the free version is being
called?

13.3.1/5 covers overload resolution of member functions and tells us:

During overload resolution, the implied object argument is
indistinguishable from other arguments. The implicit object parameter,
however, retains its identity since conversions on the corresponding
argument shall obey these additional rules:
? no temporary object can be introduced to hold the argument for the
implicit object parameter;
? no user-defined conversions can be applied to achieve a type match with
it; and
? even if the implicit object parameter is not const-qualified, an rvalue
temporary can be bound to the parameter as long as in all other respects
the temporary can be converted to the type of the implicit object
parameter.

This implies that in c1 cannot be converted to the implicit object argument
of the member function operator+ in class A. Thus, only the free-standing
operator can be chosen.

Do you have a good (preferably free online) text which covers this
issue?

Sorry, I use the standard and do not really know other resources all that
well.

However, I do think that I spotted a problem that was simply
overlooked by my colleague,
agreed.

and I believe that the non-member version should just be commented out.

I would keep the free-standing version and ditch the member function. It's
more symmetric in its arguments. In particular, as Alf has pointed out, it
takes both parameters by const &. Also, it allows implicit conversion in
both arguments.



Best

Kai-Uwe Bux

 

[Alf P. Steinbach]

* (e-mail address removed):

Alf,

Thanks for your answer. I'm a bit surprised.

I thought that scoping considerations overrode matching-parameter
considerations.

No. First all viable functions are collected in one big bowl. Then the
compiler checks parameter matching for all of them, on equal footing.

So I'm surprised that which version gets called can depend on a
constness issue.

Also, I'm a bit confused by "unless h1 is const or temporary".

A const or temporary cannot be bound to a reference to non-const (except
with Microsoft's Visual C++, language extension).

I would not have thought it matters whether h1 is temporary or not
because I don't see how the compiler can recognise temporary
variables.

This is a temporary as lhs:

HisClass() + h2;

Also, are you saying that you would prefer HisClass HisClass:perator
+ (const HisClass& h2) const
{// .... }

That is the only sensible signature for operator+ as member function, yes.

Perhaps you haven't seen enough of the code to form an opinion, but I
believe that the member version should be commented out/ deleted.
(Before, I said this about the non-member version.)

Lack of code doesn't prevent me from forming an opinion on that, but
Kai-Uwe Bux (IIRC) argued earlier (half a year ago? earlier?) that it
actually made sense to have operator+= as non-member and operator+ as
member. And so, now I'm not sure which to prefer. This choice might be
akin to the choice of some particular style of formatting, in that it
can be argued at length, without any clear conclusion.

 

[Kai-Uwe Bux]

* (e-mail address removed):

[on the issue as to whether operator+ better be a member or free-standing]

Lack of code doesn't prevent me from forming an opinion on that, but
Kai-Uwe Bux (IIRC) argued earlier (half a year ago? earlier?) that it
actually made sense to have operator+= as non-member and operator+ as
member. And so, now I'm not sure which to prefer. This choice might be
akin to the choice of some particular style of formatting, in that it
can be argued at length, without any clear conclusion.

IIRC, the discussion back then was on the issue whether operator+ better be
defined in terms of operator+= or vice versa.

With respect to the issue currently under discussion, I would favor (as of
today) a free-standing version declared outside class scope. It is more
symmetric with regard to conversions (argued elsethread). E.g.:


#include <iostream>
#include <ostream>

struct A {

friend
A operator+ ( A const & lhs, A const & rhs ) {
std::cout << "friend\n";
return ( lhs );
}

};

// the following line pulls the function into global scope:
A operator+ ( A const & lhs, A const & rhs );

struct C {

operator A ( void ) {
return A();
}

};

int main ( void ) {
C c1;
C c2;
c1 + c2; // this depends on global scope
}


BTW, I wonder why the above compiles. To me, it looks as though the
conversion function should create non-const temporaries which should not
bind to the parameters. Apparently, that is an illusion and implicit
conversions are different.



Best

Kai-Uwe Bux

 

[Alf P. Steinbach]

* Kai-Uwe Bux:

I would expect a temporary to force the member version since temporaries
cannot bind to the const & parameters of the free-standing version.

A temporary can be bound to a reference to const (free-standing version).

A temporary cannot be bound to a reference to non-const.

And so I thought you were entirely wrong, but as it turns out, according
to MSVC and g++ compilers your conclusion is correct (and I was wrong),
but your reason is incorrect: remove the member function and all is
fine, because temporaries can indeed bind to reference to const.

The true reason lies in the section of the standard you cited
else-thread (§13.3.1/5, also mentioned elsewhere in the standard),
namely that a non-const member function can be called on a temporary.

So, sorry, I was wrong about that, hasty conclusion, but note that your
reason above is also wrong.

 

[Kai-Uwe Bux]

* Kai-Uwe Bux:

A temporary can be bound to a reference to const (free-standing version).

A temporary cannot be bound to a reference to non-const.

And so I thought you were entirely wrong, but as it turns out, according
to MSVC and g++ compilers your conclusion is correct (and I was wrong),
but your reason is incorrect: remove the member function and all is
fine, because temporaries can indeed bind to reference to const.

The true reason lies in the section of the standard you cited
else-thread (§13.3.1/5, also mentioned elsewhere in the standard),
namely that a non-const member function can be called on a temporary.

So, sorry, I was wrong about that, hasty conclusion, but note that your
reason above is also wrong.

Right, the reason I gave is total garbage. I did run the hypothesis by my
compiler, which chose the free-standing version and my mind made up a
totally bogus explanation. My bad.


Best

Kai-Uwe Bux

 

[James Kanze]

No, because in the second, the "implicit this" operand is
non-const. Const can be used for overload resolution; the
compiler will normally prefer the non-const version if it can be
called.

Not really. I can't think of a case where it would be
appropriate.

Are you sure that the member isn't +=, rather than just +. That
is a more or less standard idiom: += should be a member, and + a
free function.

The way operator lookup works, both will be considered. If both
can be called, the member will be prefered, because of the rule
of preferring the least const-qualified. Because the member is
const, however, there are cases where it cannot be called, in
which case, the free function will be called.

Well, it might be a little tricky to get at the non-member operator,

Not tricky at all:

HisClass
f( HisClass const& a, HisClass const& b )
{
return a + b ;
}

The member cannot be called, since it is non-const.

I suspect, however, that either the original poster misread, and
took a += for a +, or the author mis-copied code using a member
+= and a non member +.

 

[James Kanze]

* (e-mail address removed):

Since operator+ can at most have two arguments, the above cannot be a
function definition in the class definition. Hence it's a free-standing
definition of operator+. Not a member function.

This one is a member function, and duplicates the functionality of the
former. However, it's not exactly the same: the formal argument for the
left hand side of h1+h2 is non-const.

h1+h2 will select the member function unless h1 is const or temporary,
in which case the free-standing implementation is selected.

I think it can be a temporary; something like:

HisClass f() ;
f() + h2 ;

should still select the member. On the other hand, no implicit
conversion (except Derived to Base?) will be considered to make
the types match.

Assuming the code is correctly illustrated above, I think that
is not intentional.

I suspect the intent was that the member be in fact operator+=.
If the member were in fact +=, then we have a very common idiom.

 

[James Kanze]

* (e-mail address removed): [...]

I thought that scoping considerations overrode
matching-parameter considerations.

No. First all viable functions are collected in one big bowl.
Then the compiler checks parameter matching for all of them,
on equal footing.

I think you're one step ahead of him. Name lookup occurs first,
and only functions which are found during name looup will be
considered when constructing the list of viable functions.

For a named function, the issue is rather clear: if the
expression is of the form p->f() or o.f(), name lookup will
occur only in class scope, and free functions aren't considered.
If the expression is of the form f(), and we are not in a member
function, name lookup will not consider members, and only a
free function can be found. If the expression is of the form
f(), and we are in a member function, name lookup will first
look in the class, and if it finds the function there, will not
look further.

For an operator, function overload resolution proceeds more or
less as it does for a function. But given a + b, the compiler
doesn't know whether you meant "operator+( a, b )" or
"a.operator+( b )", so it does both lookups, and merges the
results. (It also adds synthesized declarations for the
built-in operators, systematically, so these cannot be hidden.)

A const or temporary cannot be bound to a reference to non-const (except
with Microsoft's Visual C++, language extension).

Yes, but there are no references to non-const in his example.
You can call a non-const member function on a temporary; for
overload resolution purposes, the function will behave as if it
had an additional, reference argument, this is only for
overloading purposes, and doesn't affect the viability of the
function. (How often have people asked why `ostringstream() <<
"toto"' displays an address, and not the string "toto". The
reason, of course, is that the operator<< for void const* is a
member, and so can be called, but the operator<< for char const*
is a free function, and can't.)

This is a temporary as lhs:

HisClass() + h2;

And should call the member operator+.

 

[Alf P. Steinbach]

* James Kanze:

And should call the member operator+.

Yes, we resolved that else-thread.

Cheers,

- Alf