GNOME Bugzilla – Bug 460969
Ignore virtual destructor warning in setinterface.h
Last modified: 2011-01-16 23:35:11 UTC
"/usr/include/gconfmm-2.6/gconfmm/setinterface.h:42: warning: ‘class Gnome::Conf::SetInterface’ has virtual functions but non-virtual destructor" and "//TODO: Add a virtual destructor, to avoid warnings, when we can break ABI." Makes it hard for projects who want to compile with -Werror etc. Has to be worked around by doing something like: #ifndef MYPROJECT_GCONFMM_H #define MYPROJECT_GCONFMM_H #pragma GCC system_header #include <gconfmm.h> #endif // MYPROJECT_GCONFMM_H Since it will probably be a while until gconfmm can break ABI, can something like this be done in upstream gconfmm so projects using it can avoid kludges like the above? Of course, the best thing would be to only ignore this warning in setinterface.h. A while ago (3-4 years) I looked around for a pragma that did this in GCC and couldn't find any. Maybe there is one now though?
Clarification. A pragma that only ignores the "no virtual destructor warning" in setinterface.h.
Yes, this annoys me too. A patch would be welcome.
Created attachment 93630 [details] [review] Suppress no virtual destructor warning in setinterface.h so -Werror can be used. Looks like disabling specific warnings with pragmas has been added to GCC since I last looked. But I don't think we can use that particular functionality in this case. See: http://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Pragmas.html System headers and #pragma GCC system_header is documented here: http://gcc.gnu.org/onlinedocs/cpp/System-Headers.html (FWIW, #pragma GCC system_header is already used in glibmm/value_basictypes.h so I guess it's ok.)
Thanks. > I don't think we can use that particular functionality in this case Why not?
Because of: "Also, while it is syntactically valid to put these pragmas anywhere in your sources, the only supported location for them is before any data or functions are defined." But reading it again, I'm not sure. Before any data or functions after the preprocessor has been run (then I guess it's not ok since we don't know where the header might be included)? Or is it before any data or functions per cpp/h-file (I don't think so)?
I'm more concerned that this would turn off this warning for all other code too, so that application developers would not see the warning for their own code.
Are there any cases that you think the documentation for system headers doesn't cover or any documented case that might prove to be a problem? (It works for simple projects at least. I just tested adding a virtual method to a class but no virtual destructor and compiled with -Wall -Werror. The class header file included a header similar to the one in comment #0.)
I'm not sure that you understand. If we disabled a warning in a header then we disable the warning for all application code that uses our header. I don't think that application developers would like us to disable warnings about their own classes.
No, I don't think you understand. :) (Or maybe we just don't understand each other. ;) The GCC system header pragma is ok to use in this case. (Maybe you already know that and are talking about the diagnostic pragmas?) (Which, btw, I couldn't get to work. So even if the documentation didn't say we must put them before any other code and they only are file local (which I doubt, but who knows... the preprocessor generates markers so there could be some sort of warning stack), it doesn't really matter.) I'll attach an example to demonstrate how the system header pragma works.
Created attachment 94382 [details] Test Extract, cd to dir and type "make".
Oh, I forgot. Pretend broken.h is setinterface.h, ok? :)
As a workaround I just create a "proxy" header for gconfmm that contains: #if __GNUC__ #pragma GCC system_header #endif #include <gconfmm.h> And only include said header instead of gconfmm.
Yeah, see comment #0. :) It's very annoying though.
OK, committed. I worry that only gcc system headers should use this, but let's see. I'm certainly glad to be able to build glom with -Wall -Werror. Please remember to patch the ChangeLog next time. Thanks.
Actually, I haven't committed that yet, because GNOME svn's MAINTAINERS file check is stopping me from committing to any part of gnomemm. I will release a new tarball version anyway.