Check C++ Bad Override

Hi Clang,

The attached patch adds a C++ Checker to clang, it checks whether
there are mistakes in override.

The mistake mean things like this:

class base {
  virtual void foo() const {/*...*/}
};

class child: public base {
  /* child::foo is probably meant to override base::foo but type
signatures don't match
  perfectly */
  void foo() {}
};

and this:

class base {
  void foo() const {}
};

class child: public base {
  /* child::foo is probably meant to override base::foo but
that function is not virtual. */
  void foo() const {}
};

I implement this as a static analysis checker, because i think it's
not too visible and not too fast.

I will appreciate it if there are any advice about this patch.

OverrideChecker.patch (6 KB)

Hi,

Hi Clang,

The attached patch adds a C++ Checker to clang, it checks whether
there are mistakes in override.

The mistake mean things like this:

class base {
virtual void foo() const {/*...*/}
};

class child: public base {
/* child::foo is probably meant to override base::foo but type
signatures don't match
perfectly */
void foo() {}
};

Isn't this caught by -Woverloaded-virtual?

and this:

class base {
void foo() const {}
};

class child: public base {
/* child::foo is probably meant to override base::foo but
that function is not virtual. */
void foo() const {}
};

hans prototyped something like this at
http://llvm.org/bugs/show_bug.cgi?id=10234 , but it seemed to noisy to
be useful. (I haven't read how your patch does this though).

Nico

My understanding is that it is not actually necessary to declare
member functions in subclasses as being virtual if the base class
declares them virtual. The compiler just assumes they are.

That's my understanding anyway. I do not own a copy of the C++ Spec
so I can't Quote Scripture.

I always declare them virtual anyway because it's confusing otherwise.
Is that what you're trying to catch?

If you see the signatures of foo in the base class and the derived class, you can see that the derived class version is missing the “const” keyword.

Have you run this on any open source software to see how many results/violations show up? If yes, sharing the results will be useful. If not, then its a good exercise to run it on an open source software. You can even try running it on llvm itself.

Arjun

Hi,

Hi Clang,

The attached patch adds a C++ Checker to clang, it checks whether
there are mistakes in override.

The mistake mean things like this:

class base {
virtual void foo() const {/*...*/}
};

class child: public base {
/* child::foo is probably meant to override base::foo but type
signatures don't match
perfectly */
void foo() {}
};

Isn't this caught by -Woverloaded-virtual?

Yes, it is caught by -Woverloaded-virtual. So it seems no need to
implement it as a static analysis checker. I will remove relevant code

and this:

class base {
void foo() const {}
};

class child: public base {
/* child::foo is probably meant to override base::foo but
that function is not virtual. */
void foo() const {}
};

hans prototyped something like this at
http://llvm.org/bugs/show_bug.cgi?id=10234 , but it seemed to noisy to
be useful. (I haven't read how your patch does this though).

thanks, i will see test my patch on some open source code. And also
read hans's patch to see what i can do in static analysis.

Hi Arjun,

I have not run this patch on some open source software, but i will do
it later. When i got any results, i will let you know asap.

Hi Arjun,

I tried this patch on llvm itself, it’s too noisy.

I reviewed some of them and i think it’s not the right way to implement this.

I think i have to cancle it.

在 2011年10月8日 下午1:06,章磊 <ioripolo@gmail.com>写道: