Is this a -Weffc++ bug in gcc?

[DeMarcus]

Hi,

If I compile the following with gcc 4.7.1 and -Weffc++ then I get a
compiler warning on the operator% but not the others. Is this a bug?

struct A
{
A& operator%( int i ) { return *this; }
A& operator++() { return *this; }
A& fnc( int i ) { return *this; }
};

int main()
{
return 0;
}


Thanks,
Daniel

 

[Victor Bazarov]

If I compile the following with gcc 4.7.1 and -Weffc++ then I get a
compiler warning on the operator% but not the others. Is this a bug?

struct A
{
A& operator%( int i ) { return *this; }
A& operator++() { return *this; }
A& fnc( int i ) { return *this; }
};

int main()
{
return 0;
}

And for those of us who don't have [any intention to use] gcc, what
compiler warning do you get? Just out of curiosity, of course...

V

 

[red floyd]

If I compile the following with gcc 4.7.1 and -Weffc++ then I get a
compiler warning on the operator% but not the others. Is this a bug?

struct A
{
A& operator%( int i ) { return *this; }
A& operator++() { return *this; }
A& fnc( int i ) { return *this; }
};

int main()
{
return 0;
}

And for those of us who don't have [any intention to use] gcc, what
compiler warning do you get? Just out of curiosity, of course...

$ g++ -Weffc++ -o /tmp/a.o -c /tmp/a.cc
/tmp/a.cc:3:25: warning: 'A& A:perator%(int)' should return by value [-Weffc++]

Because the semantics of a binary arithmetic operator such as "%"
indicate that the return should be an rvalue. Scott Meyers discusses
this in "Effective C++" (which is what the -Weffc++ looks for).

Consider: Does the following make sense?

int x, y;
(x % y) = 3;

If, however, the following does make sense:

A a;
(a + 3) = 7;

Then feel free to ignore the warning.

 

[red floyd]

On 8/16/2012 4:53 PM, DeMarcus wrote:
If I compile the following with gcc 4.7.1 and -Weffc++ then I get a
compiler warning on the operator% but not the others. Is this a bug?

struct A
{
A& operator%( int i ) { return *this; }
A& operator++() { return *this; }
A& fnc( int i ) { return *this; }
};

int main()
{
return 0;
}

And for those of us who don't have [any intention to use] gcc, what
compiler warning do you get? Just out of curiosity, of course...

$ g++ -Weffc++ -o /tmp/a.o -c /tmp/a.cc
/tmp/a.cc:3:25: warning: 'A& A:perator%(int)' should return by value
[-Weffc++]

Because the semantics of a binary arithmetic operator such as "%"
indicate that the return should be an rvalue. Scott Meyers discusses
this in "Effective C++" (which is what the -Weffc++ looks for).

Consider: Does the following make sense?

int x, y;
(x % y) = 3;

If, however, the following does make sense:

A a;
(a + 3) = 7;

Oops. Typed too fast.

A a, b;
(a % 3) = b;

 

[Rui Maciel]

And for those of us who don't have [any intention to use] gcc, what
compiler warning do you get? Just out of curiosity, of course...

From g++'s manpage, v4.6.3:

<quote>
-Weffc++ (C++ and Objective-C++ only)
Warn about violations of the following style guidelines from
Scott Meyers' Effective C++ book:

· Item 11: Define a copy constructor and an assignment
operator for classes with dynamically allocated memory.

· Item 12: Prefer initialization to assignment in
constructors.

· Item 14: Make destructors virtual in base classes.

· Item 15: Have "operator=" return a reference to *this.

· Item 23: Don't try to return a reference when you must
return an object.

Also warn about violations of the following style guidelines from
Scott Meyers' More Effective C++ book:

· Item 6: Distinguish between prefix and postfix forms of
increment and decrement operators.

· Item 7: Never overload "&&", "||", or ",".

When selecting this option, be aware that the standard library
headers do not obey all of these guidelines; use grep -v to filter out those
warnings.
</quote>


Rui Maciel

 

[DeMarcus]

On 8/16/2012 4:53 PM, DeMarcus wrote:
If I compile the following with gcc 4.7.1 and -Weffc++ then I get a
compiler warning on the operator% but not the others. Is this a bug?

struct A
{
A& operator%( int i ) { return *this; }
A& operator++() { return *this; }
A& fnc( int i ) { return *this; }
};

int main()
{
return 0;
}

And for those of us who don't have [any intention to use] gcc, what
compiler warning do you get? Just out of curiosity, of course...

$ g++ -Weffc++ -o /tmp/a.o -c /tmp/a.cc
/tmp/a.cc:3:25: warning: 'A& A:perator%(int)' should return by value
[-Weffc++]

Because the semantics of a binary arithmetic operator such as "%"
indicate that the return should be an rvalue. Scott Meyers discusses
this in "Effective C++" (which is what the -Weffc++ looks for).

Consider: Does the following make sense?

int x, y;
(x % y) = 3;

If, however, the following does make sense:

A a;
(a + 3) = 7;

Then feel free to ignore the warning.

The problem is that the warning tries to solve two problems:

1. As Meyers says; "Unrelenting in their pursuit of pass-by-reference
purity, they invariably make a fatal mistake: they start to pass
references to objects that don't exist."

2. Prevent returning lvalues where it doesn't make sense.


In number 1, 'return *this;' should be excluded in such warning since it
guarantees that there exists an object (unless we've done 'delete this;'
on a heap object).

As far as I see in Effective C++, Meyers doesn't mention anything about
lvalues, so number 2 is something made up by someone else. It's a smart
thing, but it should be another kind of warning as I see it.

Anyway, Weffc++ is just a guideline, but it contains a good set of
warnings and it doesn't feel good to disable it back and forth to fit
certain features.


Thanks,
Daniel

 

[Tobias Müller]

The problem is that the warning tries to solve two problems:

1. As Meyers says; "Unrelenting in their pursuit of pass-by-reference
purity, they invariably make a fatal mistake: they start to pass
references to objects that don't exist."

2. Prevent returning lvalues where it doesn't make sense.

It's not only about r/lvalueness, but also about constness. operator% is
usually assumed to be const, which precludes returning *this.

In number 1, 'return *this;' should be excluded in such warning since it
guarantees that there exists an object (unless we've done 'delete this;' on a heap object).

But (1) only makes sense with (2).
If it is semantically ok to return an lvalue, returning a reference is
perfectly fine, it's actually the only possible way. It's up to the
programmer to ensure that the object actually exists. This is not
restricted to "*this". Example:
ElementType& ContainerType:perator[](int i);

But if it is semantically not ok, because everyone expects operator% to be
const and return an rvalue, then it's never ever ok to return a reference.

Tobi

 

[DeMarcus]

The problem is that the warning tries to solve two problems:

1. As Meyers says; "Unrelenting in their pursuit of pass-by-reference
purity, they invariably make a fatal mistake: they start to pass
references to objects that don't exist."

2. Prevent returning lvalues where it doesn't make sense.

It's not only about r/lvalueness, but also about constness. operator% is
usually assumed to be const, which precludes returning *this.

In number 1, 'return *this;' should be excluded in such warning since it
guarantees that there exists an object (unless we've done 'delete this;' on a heap object).

But (1) only makes sense with (2).
If it is semantically ok to return an lvalue, returning a reference is
perfectly fine, it's actually the only possible way. It's up to the
programmer to ensure that the object actually exists. This is not
restricted to "*this". Example:
ElementType& ContainerType:perator[](int i);

But if it is semantically not ok, because everyone expects operator% to be
const and return an rvalue, then it's never ever ok to return a reference.

Ok, I agree. The problem is that I want the warning at the same time I
want to use operator% like they use it in boost::format
http://www.boost.org/doc/libs/1_50_0/libs/format/

like so:
std::cout << boost::format("There are %1% apples") % nApples << std::endl;

I guess I have to come up with a nice strategy to silence the warning
for this particular function.

Does anyone have an idea of a clean way to do this, or do I have to get
my hands dirty with some #pragma mess?

Thanks,
Daniel

 

[none]

The problem is that the warning tries to solve two problems:

1. As Meyers says; "Unrelenting in their pursuit of pass-by-reference
purity, they invariably make a fatal mistake: they start to pass
references to objects that don't exist."

2. Prevent returning lvalues where it doesn't make sense.

It's not only about r/lvalueness, but also about constness. operator% is
usually assumed to be const, which precludes returning *this.

In number 1, 'return *this;' should be excluded in such warning since it
guarantees that there exists an object (unless we've done 'delete this;' on a heap object).

But (1) only makes sense with (2).
If it is semantically ok to return an lvalue, returning a reference is
perfectly fine, it's actually the only possible way. It's up to the
programmer to ensure that the object actually exists. This is not
restricted to "*this". Example:
ElementType& ContainerType:perator[](int i);

But if it is semantically not ok, because everyone expects operator% to be
const and return an rvalue, then it's never ever ok to return a reference.

Ok, I agree. The problem is that I want the warning at the same time I
want to use operator% like they use it in boost::format
http://www.boost.org/doc/libs/1_50_0/libs/format/

like so:
std::cout << boost::format("There are %1% apples") % nApples << std::endl;

Hmm,

There was some discussion in here recently from Java lovers claiming that
operator overloading obfuscate the language.

Generally speaking operator%() should be const, return by value and
essentially execute a modulo-like operation. -Weffc++ does a good
job of covering this.

God knows why boost didn't try to keep the semantic of their formatting
library a bit more printf-like but I guess that since you are using the
operator% in a pattern that already has a precedent, then maybe it is
acceptable. However, I'd recommend to consider very carefully if this
usage is likely to obfuscate the code too much for other readers. It
certainly did to most peoples in here until you explained that your
operator% was not the common meaning of %.

Note that you could use operator() or any member function to achieve
the same functionality while creating potentially much clearer code
(assuming foo() is named well)

class A
{
A& foo(int i)
}

A a;
a.foo(1).foo(2).foo(3);

I guess I have to come up with a nice strategy to silence the warning
for this particular function.

Does anyone have an idea of a clean way to do this, or do I have to get
my hands dirty with some #pragma mess?

Not really, -Weffc++ is meant to enforce a set of arbitrary good-practice
guidelines. pragma's allow you to temporarily disable some warnings.
So assuming you persist in your (somewhat obfuscating) usage of operator%,
you have three choices:
- Don't use -Weffc++
- Ignore the warnigns
- Use pragma.


Yannick

 

[Tobias Müller]

Ok, I agree. The problem is that I want the warning at the same time I
want to use operator% like they use it in boost::format
http://www.boost.org/doc/libs/1_50_0/libs/format/

like so:
std::cout << boost::format("There are %1% apples") % nApples << std::endl;

I guess I have to come up with a nice strategy to silence the warning for
this particular function.

Does anyone have an idea of a clean way to do this, or do I have to get
my hands dirty with some #pragma mess?

My suggestion with C++11:
(Disclaimer: I have no practical experience with C++11, so correct me if
I'm wrong)

class MyFormat
{
MyFormat() {/*...*/}
MyFormat(const MyFormat& other) {/*...*/} // Copy ctor
MyFormat(MyFormat&& other) {/*...*/} // Move ctor
MyFormat(const char* formatString) {/*...*/}

MyFormat& operator%=(int i) { /* Do work here */}

// At this point you can already write:
// std::cout << (MyFormat("There are %1% apples") %= nApples) <<
std::endl;

// If you don't like the parenteses and/or operator%= go ahead:

MyFormat operator%(int i) && // rvalue qualified
{
MyFormat format(std::move(*this)); // Not sure if std::move is actually
correct.
return format %= i; // Hope that RVO that chimes in.
}
};

If RVO works, you have almost no performance penalty.

Tobi