After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 460969 - Ignore virtual destructor warning in setinterface.h
Ignore virtual destructor warning in setinterface.h
Status: RESOLVED FIXED
Product: gconfmm
Classification: Other
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: gtkmm-forge
gtkmm-forge
Depends on:
Blocks:
 
 
Reported: 2007-07-27 15:30 UTC by Martin Ejdestig
Modified: 2011-01-16 23:35 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Suppress no virtual destructor warning in setinterface.h so -Werror can be used. (464 bytes, patch)
2007-08-14 08:31 UTC, Martin Ejdestig
none Details | Review
Test (687 bytes, application/x-bzip)
2007-08-26 17:54 UTC, Martin Ejdestig
  Details

Description Martin Ejdestig 2007-07-27 15:30:51 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?
Comment 1 Martin Ejdestig 2007-07-27 19:27:22 UTC
Clarification. A pragma that only ignores the "no virtual destructor warning" in setinterface.h.
Comment 2 Murray Cumming 2007-08-13 11:09:11 UTC
Yes, this annoys me too.

A patch would be welcome.
Comment 3 Martin Ejdestig 2007-08-14 08:31:11 UTC
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.)
Comment 4 Murray Cumming 2007-08-14 09:04:41 UTC
Thanks.

> I don't think we can use that particular functionality in this case

Why not?
Comment 5 Martin Ejdestig 2007-08-14 09:44:51 UTC
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)?
Comment 6 Murray Cumming 2007-08-14 10:05:46 UTC
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.
Comment 7 Martin Ejdestig 2007-08-14 10:33:58 UTC
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.)
Comment 8 Murray Cumming 2007-08-26 15:57:00 UTC
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.
Comment 9 Martin Ejdestig 2007-08-26 17:52:12 UTC
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.
Comment 10 Martin Ejdestig 2007-08-26 17:54:39 UTC
Created attachment 94382 [details]
Test

Extract, cd to dir and type "make".
Comment 11 Martin Ejdestig 2007-08-26 17:56:35 UTC
Oh, I forgot. Pretend broken.h is setinterface.h, ok? :)
Comment 12 Hubert Figuiere (:hub) 2007-09-10 02:48:30 UTC
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.
Comment 13 Martin Ejdestig 2007-09-10 06:41:47 UTC
Yeah, see comment #0. :) It's very annoying though.
Comment 14 Murray Cumming 2007-09-10 15:39:45 UTC
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.

Comment 15 Murray Cumming 2007-09-10 15:40:42 UTC
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.