Sorry about the late reply(I am in a different timezone and when my
comment didn't appear after 12 hours of submission, I thought it had
been pruned).
@Eric Roman
>What class do you get this error on?
I actually got fed up of the compilation errors and grabbed a binary
build for Ubuntu. Other people here do seem to know about this
possible issue. However, I would be happy to reproduce the issue if
you would wish so.
@Evan Martin
>It turns out
>for our codebase that is very common (due to lots of "observer"-like
>patterns), so we decided to not rely on this compiler warning. It's
>only caused horrible bugs once or twice! :)
My generated Makefile had -Werror which made the build fail. IMHO
"horrible bugs once ore twice" are worth the effort to fix them for
the long term. However, I see that the dev team is divided on the
issue. From an end-user perspective, it is annoying to have the build
fail for something which the dev team knows about; The end-user
doesn't and he is left with the task of investigation:-)
>The correct fix would be to disable this particular warning in
>build/common.gypi.
Oh, I didn't know about this. I took the brute force approach of
finding all files with -Werror in them and deleting them.
$> awk -F: '{print $1}' <(grep -R '\-Werror' *) | xargs perl -i.bak -
pe 's/-Werror//g' (Not pretty but works)
I would prefer this to be part of documentation somewhere.
>However, I'm puzzled why nobody else has encountered the problem
>you're mentioning.
>Can you provide some details about your compiler, system configuration, etc.?
I ran into this issue with both my work and home system. I downloaded
the source tarball and did a "gclient sync --force" after setting
"GYP_GENERATORS=make".
Work - RHEL 5, gcc 4.4, python 2.6.4
Home - Ubuntu 9.10, gcc 4.4, python 2.6.4
@Jacob Mandelson
> (So, add on -Wno-non-virtual-dtor to get a build.)
Didn't know about this and didn't bother to read through gcc doc.
Thanks for the tip.
@Marc Mentoval
>I happen to find this warning very useful, just as I find our policy
>to make warnings hard errors in our own code helpful.
Yes, of course. That's a good practice. But what about the end-users
who have to deal with warnings as hard errors.
>Strictly speaking, the problem isn't precisely that a leak will occur
>if Sub is larger than Base. The real problem is that Sub::~Sub won't
>run unless Base declares a virtual destructor.
My bad. Didn't word it properly.
@Peter Kasting
> However, the edit doesn't create or own pointers to its
>controller, and never deletes its controller, so this abstract class doesn't
>have a virtual destructor. The pattern here is "implements interface X" as
>opposed to the "is a specialized type of an X" pattern of parent and child
>classes.
No two opinions about elegance of your design patterns. But it looks
like sort of black magic to count on "doesn't create or own pointers
to its
controller, and never deletes its controller". Of course, it isn't a
problem in a homogeneous environment where people know about the
established conventions.
@Mark Mentoval
>The style guide recommends always providing an empty virtual
>destructor in interface classes.
>http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showon...
>"To make sure all implementations of the interface can be destroyed
>correctly, they must also declare a virtual destructor (in an
>exception to the first rule, this should not be pure)."
Pardon my naive self, but I still don't get it. Enforcing it won't be
breaking anything other than:
1) Purity of design patter. C++ doesn't have interfaces as first class
citizens, so using an abstract class with a virtual d'tor as an
interface doesn't amount to violation(your design guide says so as
well).
2) Some extra overhead which isn't really large - one extra entry in
vtbl, one extra indirection, possible loss of compiler inlining.
>but again, there are valid uses where this is not strictly required.
But those usage aren't being hurt by enforcing this except for what
you guys already agree on.
--
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
