0

I know this question.

However I have a compiler warning which mentions this which makes a bit different from the question above.

As the above question's answer clearly states that the attribute in the warning is defined with

attribute((nonnull));

Consider this class:

class ReferenceCounter {
    public:
        ReferenceCounter(void) : count_(0) {
        }

        ReferenceCounter& operator=(const ReferenceCounter& other_) {
            if (this != &other_) {
                count_ = other_.count_;
            }
            return *this;
        }

        void increment(void) {
            ++count_;
        }

        int decrement(void) {
            return this == NULL ? 0 : --count_;
        }

    private:
        int count_;
    };

Compiler warns for the return this == NULL ? 0 : --count_; line with

warning: nonnull argument ‘this’ compared to NULL

Where is "this" decided to be nonnull here?

Note: ReferenceCounter is used only in one place as a pointer:

ReferenceCounter* r;

Which makes possible to be NULL so I don't understand why the compiler complains about this particular check and want to

  1. understand
  2. correct

Please don't shoot me about this code, it's not mine, just I'm forced to use it.

Daniel
  • 2,318
  • 2
  • 22
  • 53
  • 4
    If `this` is `NULL`, your program has big, big problems. – user4581301 Apr 26 '22 at 19:23
  • 1
    if `this` is `NULL` how would be possible to call `decrement`? As I stated in the question I would like to **understand**. – Daniel Apr 26 '22 at 19:24
  • `ReferenceCounter* r = NULL; r->decrement();` is undefined behavior, but usually crashs on the first member variable access. Probably the writer thought he could outsmart the compiler or so. – mch Apr 26 '22 at 19:26
  • 4
    If `r` is `nullptr` (C++'s version of `NULL`) then `r->decrement();` is already Undefined Behavior and can crash, unpredictably corrupt your program or silently ignore the error. Once execution might reach the function's body it is already too late. – François Andrieux Apr 26 '22 at 19:28
  • 1
    It's possible, unfortunately, but the program is broken. The compiler cannot pick off the more complicated versions of `ReferenceCounter* r = NULL; r->decrement();`, so a flawed program can be produced. As François points out, the test inside the function is too late, the program is already broken. The correction is to remove the test as it is meaningless. – user4581301 Apr 26 '22 at 19:36
  • 3
    It is interesting that the test occurs in `decrement` and not in `increment`, so this was likely some hack to "fix" a bug elsewhere that never was tripped over when calling `increment`. The program has a lurking bug that may be causing other problems that no one has noticed yet. Valgrind or Address Sanitizer may be able to help you track it down and kill it. – user4581301 Apr 26 '22 at 19:37
  • GCC and Clang have the flag `-fno-delete-null-pointer-checks` that enables this type of code to work. Yes, I know, it remains a bad thing to do. – prapin Apr 26 '22 at 20:05

1 Answers1

3

Compiler warns for the return this == NULL ? 0 : --count_; line with

warning: nonnull argument ‘this’ compared to NULL

Where is "this" decided to be nonnull here?

The compiler will assume that this is always non-null, because it is undefined behavior when a member function is invoked on the null pointer value, i.e. when ptr in ptr->decrement() is a null pointer.

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
j6t
  • 9,150
  • 1
  • 15
  • 35
  • So it's "safe" to change `return this == NULL ? 0 : --count_;` to `return --count_;`? – Daniel Apr 26 '22 at 19:35
  • @Daniel The two are functionally identical. `this == NULL` can reliably be assumed to be `false`, which mean the code is equivalent to `false ? 0 : --count_;` which is equivalent to `--count_;`. The difference is the latter is clearer in intent. – François Andrieux Apr 26 '22 at 19:38
  • Well, yes. If you don't write the code in this "safe" way, the compiler will pretend you have written it this way ;) – j6t Apr 26 '22 at 19:38