Code::Blocks Forums

Developer forums (C::B DEVELOPMENT STRICTLY!) => Development => Topic started by: ollydbg on December 27, 2011, 02:47:58 am

Title: static variables in cbplugin.h
Post by: ollydbg on December 27, 2011, 02:47:58 am
I just found a potential bug:
Here, in cbplugin.h, there are code like:
Code
// Define basic groups for plugins' configuration.
static const int cgCompiler         = 0x01; ///< Compiler related.
static const int cgDebugger         = 0x02; ///< Debugger related.
static const int cgEditor           = 0x04; ///< Editor related.
static const int cgCorePlugin       = 0x08; ///< One of the core plugins.
static const int cgContribPlugin    = 0x10; ///< One of the contrib plugins (or any third-party plugin for that matter).
static const int cgUnknown          = 0x20; ///< Unknown. This will be probably grouped with cgContribPlugin.

This will let all the translation unit which include the "cbplugin.h" have a own copy of those variables. Is that a bug?

Why not just use extern modifier, and define them in the cpp files???
Or just use enum?
Title: Re: static variables in cbplugin.h
Post by: danselmi on December 27, 2011, 12:03:06 pm
They are defined as const. So a "copy" is the same constant value.
Title: Re: static variables in cbplugin.h
Post by: Folco on December 27, 2011, 01:24:59 pm
This is a linker optimisation, no ? "Constant merging". Is it guaranteed by the standard ?
Title: Re: static variables in cbplugin.h
Post by: ollydbg on December 27, 2011, 02:49:21 pm
They are defined as const. So a "copy" is the same constant value.
Yes. But...Anyway, I do believe variable definitions in header file is not a good idea.
Title: Re: static variables in cbplugin.h
Post by: MortenMacFly on December 28, 2011, 04:20:17 pm
Yes. But...Anyway, I do believe variable definitions in header file is not a good idea.
static const can be seen as the c++ style of a #define. In the case you refer to it's also similar to an enum, so this is OK.
Title: Re: static variables in cbplugin.h
Post by: oBFusCATed on December 29, 2011, 01:35:41 pm
Morten: do you know the reason why this is not an enum, but a number of static variables?
Title: Re: static variables in cbplugin.h
Post by: thomas on December 29, 2011, 04:21:52 pm
ISO 14882 3.5.3 states that names having namespace scope automatically have internal linkage if they are either const or static and not at the same time explicitly extern. Which means that the linkage is irrespective of the presence of static because of const. The rationale for that is given in Appendix C. The standard makes explicit mention of inclusion of headers in many compilation units.

As to why these values are not an enum, the likely reason is simply that the person who wrote that code (Yiannis) wrote it that way. There is no real advantage or disadvantage to either optimization-wise.
Title: Re: static variables in cbplugin.h
Post by: MortenMacFly on December 29, 2011, 04:43:50 pm
Morten: do you know the reason why this is not an enum, but a number of static variables?
Usually this is because if you compare the values against another one which is an INT, too you don't need to take care to cast the values to avoid compiler warnings. At least that's when I personally prefer static const instead of an enum.
Title: Re: static variables in cbplugin.h
Post by: ollydbg on December 30, 2011, 01:07:56 am
Usually this is because if you compare the values against another one which is an INT, too you don't need to take care to cast the values to avoid compiler warnings.
I guess that if it was defined as a enum, then the user/client will define same enum type variables too, some kind of type consistent. :)

"Int type" basically does not have much specific information like enum type.
Title: Re: static variables in cbplugin.h
Post by: MortenMacFly on December 30, 2011, 07:42:57 am
I guess that if it was defined as a enum, then the user/client will define same enum type variables too, some kind of type consistent. :)
I thought more in a direction to compare it against a value of a control or alike. There, you have no choice other than INT or SIZE_T or alike.