On Fri, Nov 20, 2009 at 4:23 AM, rahul <[email protected]> wrote: > I was trying to compile chromium today on Linux and I got this error. > This error occurred because of -Werror CFLAG set in the Makefile. I > remedied by writing a one-liner to delete all occurrences of -Werror > from CFLAGS in all Makefiles. > > However, this error intrigued me. I saw through the code and the > classes in question have declared virtual methods but haven't declared > a destructor. In that case, the compiler generates the destructor. > > > Base *b = new Sub(); > delete b; > > To the best of my knowledge, if Base is the class which defines > virtual methods but non-virtual destructor, delete can cause a memory > leak if Sub happens to be taking more memory than Base. I think that > the destructor for a class with virtual methods has either to be > "public virtual" or "protected non-virtual" for the proper freeing of > memory. > > > Would someone help me clarify this? > > I know there might be some scenarios where this can be done > intentionally but looks like there are more arguments for adding a > virtual destructor than not adding one. I see that this is done for > the Delegates. Also, if this is one of the intended special cases, can > someone explain how to compile chromium then? Deleting -Werror from > CFLAGS is an ugly hack at best.
Your analysis is correct. However, a virtual destructor is not needed in the case where you never delete through the Base*. 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! :) The correct fix would be to disable this particular warning in build/common.gypi. 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.? -- Chromium Developers mailing list: [email protected] View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev
