Bug 29348 - Ambiguous warning with -Weffc++
Summary: Ambiguous warning with -Weffc++
Status: RESOLVED INVALID
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 4.0.1
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-10-04 16:15 UTC by Federico Carminati
Modified: 2006-10-05 05:18 UTC (History)
2 users (show)

See Also:
Host: Darwin 8.8.1 i386
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Federico Carminati 2006-10-04 16:15:44 UTC
The following code produces an ambiguous warning from -Weffc++

class a {
private:
  a operator||(const a&);
};

[/Users/fca] g++ -Weffc++ -c effcpp01.cxx 
effcpp01.cxx:3: warning: user-defined 'a a::operator||(const a&)' always evaluates both arguments

The meaning is "effective c++ discourages you to override the || operator because you most probably will implement it in an unoptimised way, evaluating uselessly both arguments". However the warning as it stands talks about an implementation that, in this case does not exist. A better warning should be produced.
Comment 1 Andrew Pinski 2006-10-05 04:31:03 UTC
effcpp01.cxx:3: warning: user-defined 'a a::operator||(const a&)' always
evaluates both arguments

I don't see why this is ambiguous.  Because normally || is short ciruting in that if the left hand side is true the other side is not evaluated.  And it is has nothing to do with optimization but rather correct behavior.
so with user defined operator||, you have to evaluate both sides which is different than the default behavior which then could cause different behavior.
Comment 2 Federico Carminati 2006-10-05 05:10:23 UTC
Subject: Re:  Ambiguous warning with -Weffc++

First of all a user can implement the correct behaviour, evaluating  
just the first member and returning if true, continuing if false.  
Second, the compiler tells me that I am evaluating the two members  
when it has no way to tell this just by looking at the declaration of  
the operator. So the warning is at best not justified when referring  
only to the declaration, at worst wrong.

Federico Carminati
CERN-PH
1211 Geneva 23
Switzerland
Tel: +41 22 76 74959
Fax: +41 22 76 79480
Mobile: +41 76 487 4843

On 5 Oct 2006, at 06:31, pinskia at gcc dot gnu dot org wrote:

>
>
> ------- Comment #1 from pinskia at gcc dot gnu dot org  2006-10-05  
> 04:31 -------
> effcpp01.cxx:3: warning: user-defined 'a a::operator||(const a&)'  
> always
> evaluates both arguments
>
> I don't see why this is ambiguous.  Because normally || is short  
> ciruting in
> that if the left hand side is true the other side is not  
> evaluated.  And it is
> has nothing to do with optimization but rather correct behavior.
> so with user defined operator||, you have to evaluate both sides  
> which is
> different than the default behavior which then could cause  
> different behavior.
>
>
> -- 
>
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29348
>
> ------- You are receiving this mail because: -------
> You are on the CC list for the bug, or are watching someone who is.
> You reported the bug, or are watching the reporter.

Comment 3 Andrew Pinski 2006-10-05 05:18:49 UTC
(In reply to comment #2)
> Subject: Re:  Ambiguous warning with -Weffc++
> 
> First of all a user can implement the correct behaviour, evaluating  
> just the first member and returning if true, continuing if false.  
> Second, the compiler tells me that I am evaluating the two members  
> when it has no way to tell this just by looking at the declaration of  
> the operator. So the warning is at best not justified when referring  
> only to the declaration, at worst wrong.

You are incorrect thinking the user can implement the old behavior here as the compiler needs to evaluate it to pass to the function.

And example about this is talking about:
struct a {
  bool operator||(const a&);
  //operator bool();
};

a g();

int f(a lhs)
{
  return lhs || g();
}
----
In the non user defined case, lhs is evaulated and if that is true, we don't call g() at all.  In the user defined case, both lhs and g() are evaulated as we need to call lhs.operator|| with the argument of the value of g().

What it means by both arguments, it means both the left hand side (this) and the right hand side, the argument to the member operator.  This is the correct warning and just from the sound of it, you don't understand how the builtin operator|| works or how having an user declared operator|| can work without evaulating both sides.
Comment 4 Federico Carminati 2006-10-05 07:44:24 UTC
Subject: Re:  Ambiguous warning with -Weffc++

Thanks for your answer. However, if I just code in the following way

int f(a lhc) {
	if (lhs) return TRUE;
	else if g() return TRUE;
	return FALSE
}

I am not always evaluating both arguments, but the warning will still  
be there. Or am I missing something? Bes,t

Federico Carminati
CERN-PH
1211 Geneva 23
Switzerland
Tel: +41 22 76 74959
Fax: +41 22 76 79480
Mobile: +41 76 487 4843

On 5 Oct 2006, at 07:18, pinskia at gcc dot gnu dot org wrote:

>
>
> ------- Comment #3 from pinskia at gcc dot gnu dot org  2006-10-05  
> 05:18 -------
> (In reply to comment #2)
>> Subject: Re:  Ambiguous warning with -Weffc++
>>
>> First of all a user can implement the correct behaviour, evaluating
>> just the first member and returning if true, continuing if false.
>> Second, the compiler tells me that I am evaluating the two members
>> when it has no way to tell this just by looking at the declaration of
>> the operator. So the warning is at best not justified when referring
>> only to the declaration, at worst wrong.
>
> You are incorrect thinking the user can implement the old behavior  
> here as the
> compiler needs to evaluate it to pass to the function.
>
> And example about this is talking about:
> struct a {
>   bool operator||(const a&);
>   //operator bool();
> };
>
> a g();
>
> int f(a lhs)
> {
>   return lhs || g();
> }
> ----
> In the non user defined case, lhs is evaulated and if that is true,  
> we don't
> call g() at all.  In the user defined case, both lhs and g() are  
> evaulated as
> we need to call lhs.operator|| with the argument of the value of g().
>
> What it means by both arguments, it means both the left hand side  
> (this) and
> the right hand side, the argument to the member operator.  This is  
> the correct
> warning and just from the sound of it, you don't understand how the  
> builtin
> operator|| works or how having an user declared operator|| can work  
> without
> evaulating both sides.
>
>
> -- 
>
> pinskia at gcc dot gnu dot org changed:
>
>            What    |Removed                     |Added
> ---------------------------------------------------------------------- 
> ------
>              Status|UNCONFIRMED                 |RESOLVED
>          Resolution|                            |INVALID
>
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29348
>
> ------- You are receiving this mail because: -------
> You are on the CC list for the bug, or are watching someone who is.
> You reported the bug, or are watching the reporter.