[cfe-dev] Warning when calling virtual functions from constructor/desctructor?

[Arnaud Bienner via cfe-dev]

What I like about having this kind of diagnostics being part of clang
itself is that you can turn them into errors using -Werror=, and prevent
bugs to even been written, saving you debugging time.
I'm not sure how static code analysis is done on the projects you are
working on, but from my experience, it is usually done as part of the CI
(so after you commit your changes): this saves you from having bugs
reaching the release stage, but doesn't save you the time spent debugging a
silly error that could have been catch by the compiler.
On the other hand, I understand that you don't want this kind of diagnostic
to significantly slow down the compilation process (I guess that would be
the case here, but I'm not sure).

Arnaud

Le jeu. 13 déc. 2018 à 16:21, Aaron Ballman <aaron at aaronballman.com> a
écrit :

> On Thu, Dec 13, 2018 at 10:08 AM Arnaud Bienner via cfe-dev
> <cfe-dev at lists.llvm.org> wrote:
> >
> > Hi all :)
> >
> > I noticed that clang doesn't emit a warning in this case.
> > It does when calling a *pure* virtual function though (a warning for
> which I added a diagnostic group recently [1]).
> > I think one difference is that calling pure virtual function from
> constructor is undefined, while calling virtual function is defined (it
> calls the base class function since the vtable isn't filled yet with
> inherited virtual functions reimplementations).
> > So implementing a warning like this will likely trigger false positives.
> >
> > However, I think in most cases it denotes a bug, since many IMHO C++
> developers won't get that the function will not behave as they might expect.
> > Also, we can have a fixit to this, by suggesting the user to to specify
> the class of the function e.g. write "B::f()" instead of "f()".
> >
> > I'm fine (trying to) write a patch for this, but first I wanted to get
> opinions about whether it's worth it.
> > I guess it might not be that much difficult to do for simple cases (when
> calling virtual function directly). I expect it might be more difficult to
> catch cases where we call a function that is not virtual, but which calls a
> virtual function behind the scene.
> > Also, if you're worried about having too many false positives, I think
> we could put this new warning behind Wextra.
> >
> > What do you think?
>
> The static analyzer currently has alpha.cplusplus.VirtualCall that
> should catch this sort of issue. Given the concerns about false
> positives, perhaps that check could be improved enough to pull it out
> of alpha status instead of working on a check within clang itself?
>
> ~Aaron
>
> >
> > Best regards,
> > Arnaud
> >
> > [1]:
> https://github.com/llvm-mirror/clang/commit/528a1d17a3ff8361fdd4a1a379adf3f64ec68e00
> >
> > _______________________________________________
> > cfe-dev mailing list
> > cfe-dev at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20181213/e613b149/attachment.html>