Is there a GCC option to warn about writing `this-field` instead of `this->field`?

Question

This following code (containing a vicious bug) compiles with GCC without any warning. But, of course, it doesn't work as expected by the developer (me).

#include <iostream>

struct A
{
    bool b;
    void set(bool b_) { this->b = b_; }
    bool get() const { return this-b; } // The bug is here: '-' instead of '->'
};

int main()
{
    A a;
    a.set(true);
    std::cout << a.get() << std::endl; // Print 1
    a.set(false);
    std::cout << a.get() << std::endl; // Print 1 too...
    return 0;
}

Which warning can I add for the compiler (GCC 4.8) to avoid this kind of typo?

Linked question: Is there any option to force (or warn) the access to member variables/functions with this->?

Answer 1

This particular issue is detected by cppcheck:

$ cppcheck --enable=all this-minus-bool.cxx 
Checking this-minus-bool.cxx...
[this-minus-bool.cxx:7]: (warning) Suspicious pointer subtraction. Did you intend to write '->'?
(information) Cppcheck cannot find all the include files (use --check-config for details)

This was with no include path given. If I add -I /usr/include/c++/4.8/, the issue is still detected:

Checking this-minus-bool.cxx...
[this-minus-bool.cxx]: (information) Too many #ifdef configurations - cppcheck only checks 12 of 45 configurations. Use --force to check all configurations.
[this-minus-bool.cxx:7]: (warning) Suspicious pointer subtraction. Did you intend to write '->'?
[/usr/include/c++/4.8/bits/ostream.tcc:335]: (style) Struct '__ptr_guard' has a constructor with 1 argument that is not explicit.
[/usr/include/c++/4.8/bits/locale_classes.tcc:248]: (error) Deallocating a deallocated pointer: __c

and then cppcheck slowly works through the aforementioned #ifdef configurations.

(As a side note, the error in local_classes.tcc is a false positive but this is very hard to tell for an automated tool, as it would need to be aware that the catch block at this site should not be entered when the macro __EXCEPTIONS is unset.)

Disclaimer: I have no other experience with cppcheck.

Answer 2

No, this - b is performing pointer arithmetic on the pointer this, despite b being a bool type (b is implicitly converted to int).

(Interestingly, you can always set this + b to a pointer where b is a bool type, since you can set a pointer to one past the end of a scalar! So even your favourite undefined behaviour spotter would permit that one.)

Array bounds checking has always been the job of a C++ programmer.

Note also that in your cases the use of this is superfluous: so curtailing this excessive use is one way of making the problem go away.

Answer 3

I would like to suggest another tool (apart from cppcheck proposed by @arne-vogel), giving a better visual aid instead of the warning asked for:

Use clang-format to automatically format your code. The result might look like this (depending on the settings), making the bug more visible by the spaces added around operator-:

struct A {
  bool b;
  void set(bool b_) { this->b = b_; }
  bool get() const { return this - b; }
};

Answer 4

No, there is no way to get a warning. The implicit conversions, although perverse, are allowed by the language.

However, in this specific use case we can do better - by wrapping the bool in a wrapper class which has explicit conversions and no arithmetic operations defined.

This results in a compiler error when logically misused, which is normally seen as preferable to a warning if logical correctness is the goal.

It is interesting to note that c++17 deprecates bool::operator++ as this arithmetic is seen as evil.

example:

struct Bool
{
    explicit Bool(bool b) : value_(b) {}
    explicit operator bool() const { return value_; }
private:
    bool value_;

    // define only the operators you actually want
    friend std::ostream& operator<<(std::ostream& os, const Bool& b) {
        return os << b;
    }
};

struct X
{
    bool foo() {
        // compilation failure - no arithemetic operators defined.
        // return bool(this-b);

        // explicit conversion is fine
        return bool(b);
    }

    Bool b { true }; // explicit initialisation fine
};