Is this a -Weffc++ bug in gcc?

Discussion in 'C++' started by DeMarcus, Aug 16, 2012.

  1. DeMarcus

    DeMarcus Guest

    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
    DeMarcus, Aug 16, 2012
    #1
    1. Advertising

  2. 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...

    V
    --
    I do not respond to top-posted replies, please don't ask
    Victor Bazarov, Aug 16, 2012
    #2
    1. Advertising

  3. DeMarcus

    red floyd Guest

    On 8/16/2012 2:51 PM, Scott Lurndal wrote:
    > Victor Bazarov <> writes:
    >> 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::eek: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, Aug 16, 2012
    #3
  4. DeMarcus

    red floyd Guest

    On 8/16/2012 3:41 PM, red floyd wrote:
    > On 8/16/2012 2:51 PM, Scott Lurndal wrote:
    >> Victor Bazarov <> writes:
    >>> 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::eek: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;

    >
    > Then feel free to ignore the warning.
    red floyd, Aug 16, 2012
    #4
  5. DeMarcus

    Rui Maciel Guest

    Victor Bazarov wrote:

    > 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
    Rui Maciel, Aug 16, 2012
    #5
  6. DeMarcus

    DeMarcus Guest

    On 08/17/2012 12:41 AM, red floyd wrote:
    > On 8/16/2012 2:51 PM, Scott Lurndal wrote:
    >> Victor Bazarov <> writes:
    >>> 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::eek: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
    DeMarcus, Aug 17, 2012
    #6
  7. DeMarcus <> wrote:

    > 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::eek: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
    Tobias Müller, Aug 17, 2012
    #7
  8. DeMarcus

    DeMarcus Guest

    On 2012-08-17 18:23, Tobias Müller wrote:
    > DeMarcus <> wrote:
    >
    >> 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::eek: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
    DeMarcus, Aug 17, 2012
    #8
  9. DeMarcus

    none Guest

    In article <502e7300$0$293$>,
    DeMarcus <> wrote:
    >On 2012-08-17 18:23, Tobias Müller wrote:
    >> DeMarcus <> wrote:
    >>
    >>> 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::eek: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
    none, Aug 17, 2012
    #9
  10. DeMarcus <> wrote:

    > 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
    Tobias Müller, Aug 17, 2012
    #10
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Replies:
    8
    Views:
    428
  2. Kevin P. Fleming

    C99 structure initialization in gcc-2.95.3 vs gcc-3.3.1

    Kevin P. Fleming, Nov 6, 2003, in forum: C Programming
    Replies:
    2
    Views:
    644
    Kevin P. Fleming
    Nov 6, 2003
  3. Replies:
    5
    Views:
    357
    Nathan Addy
    Sep 17, 2005
  4. Replies:
    5
    Views:
    1,058
    red floyd
    Dec 3, 2009
  5. wimalopaan
    Replies:
    2
    Views:
    430
    wimalopaan
    Aug 18, 2011
Loading...

Share This Page