Following http://stackoverflow.com/questions/204983/static-const-member-value-vs-member-enum-which-method-is-better-why, I'm confused when I also see relatively limited usage in Chromium of https://code.google.com/p/chromium/codesearch#chromium/src/base/compiler_specific.h&q=STATIC_CONST_MEMBER_DEFINITION%20compiler&type=cs&sq=package:chromium&l=71.For integer constants associated with a class, do we prefer still using enums, or do we prefer using static const class members now that compilers generally do the right thing? If the latter, should we:
a) always use STATIC_CONST_MEMBER_DEFINITION in the .cc if the static const member is declared (and initialized) in the .h, orb) only define in the .cc if the static const member is declared (and initialized) in the .h *and* the member is used by reference, and when defining use STATIC_CONST_MEMBER_DEFINITION, or
c) (I suspect not) always define the member in the .cc if the static const member is declared (and initialized) in the .h, and only use STATIC_CONST_MEMBER_DEFINITION in that definition if the member is ever used by reference?Maybe this is FAQ, or even in the style guides, but I couldn't find an answer searching.Thanks!Matt
--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
For integer constants associated with a class, do we prefer still using enums, or do we prefer using static const class members now that compilers generally do the right thing? If the latter, should we:
--
On Tue, Aug 11, 2015 at 4:50 PM, Matthew Wolenetz <wole...@chromium.org> wrote:For integer constants associated with a class, do we prefer still using enums, or do we prefer using static const class members now that compilers generally do the right thing? If the latter, should we:If possible, use a const, or a static const member (or similar) in preference to the "enum hack".
STATIC_CONST_MEMBER_DEFINITION is generally not necessary to get things to work well, but in some cases it's still necessary to make all compilers happy. (The symptom of this is that without it you get gcc complaining when you do one thing and MSVC complaining when you do the other.) I've probably needed this macro in two cases total, so it's rare enough that most people don't know about it.PK
--
If you don't need to take the address, use an enum. It's simpler, works better,
On Tue, Aug 11, 2015 at 5:09 PM, Nico Weber <tha...@chromium.org> wrote:If you don't need to take the address, use an enum. It's simpler, works better,I disagree with both "simpler" and "works better".An enum is generally more lines of code. It's not simpler in the common case where you just declare the constant and are done with it.More importantly, an enum is generally implicitly int, but explicitly specifying the type is clearer, and it's better when it turns out you don't want an int. Plenty of webrtc code needed larger-than-I-would-have-liked changes because I needed various constants to be size_ts and they'd been defined as enums. (No, I'm not going to use "enum class" for that.)A constant is semantically clear. It reads like it means. Using "enum" to mean "const" may be a common idiom but it's not an inherently clear one. Much like git syntax, it only makes any sense through familiarity.
As Dana mentioned, "one day we'll get constexpr", and hopefully at that point we can agree that constexpr is better than either of the above, but until then, using actual typed constants is usually simpler, always clearer, generally more maintainable, and hopefully will more trivially convert to constexpr in the future.PK
--
On Tue, Aug 11, 2015 at 5:09 PM, Nico Weber <tha...@chromium.org> wrote:If you don't need to take the address, use an enum. It's simpler, works better,I disagree with both "simpler" and "works better".An enum is generally more lines of code.
It's not simpler in the common case where you just declare the constant and are done with it.More importantly, an enum is generally implicitly int, but explicitly specifying the type is clearer
, and it's better when it turns out you don't want an int. Plenty of webrtc code needed larger-than-I-would-have-liked changes because I needed various constants to be size_ts and they'd been defined as enums. (No, I'm not going to use "enum class" for that.)
A constant is semantically clear. It reads like it means.
Using "enum" to mean "const" may be a common idiom but it's not an inherently clear one. Much like git syntax, it only makes any sense through familiarity.As Dana mentioned, "one day we'll get constexpr", and hopefully at that point we can agree that constexpr is better than either of the above, but until then, using actual typed constants is usually simpler,
always clearer
generally more maintainable
On Tue, Aug 11, 2015 at 5:14 PM, Peter Kasting <pkas...@chromium.org> wrote:On Tue, Aug 11, 2015 at 5:09 PM, Nico Weber <tha...@chromium.org> wrote:If you don't need to take the address, use an enum. It's simpler, works better,I disagree with both "simpler" and "works better".An enum is generally more lines of code.It's 3 lines, one of which is "};". The static const int is 2 (since you need storage).
It's not simpler in the common case where you just declare the constant and are done with it.More importantly, an enum is generally implicitly int, but explicitly specifying the type is clearerenum foo : int { FOO = 1243; };
A constant is semantically clear. It reads like it means.No, the "constant" wants that you define storage somewhere. An enum is how you define a semantically clear enum.
On Tue, Aug 11, 2015 at 5:22 PM, Nico Weber <tha...@chromium.org> wrote:On Tue, Aug 11, 2015 at 5:14 PM, Peter Kasting <pkas...@chromium.org> wrote:On Tue, Aug 11, 2015 at 5:09 PM, Nico Weber <tha...@chromium.org> wrote:If you don't need to take the address, use an enum. It's simpler, works better,I disagree with both "simpler" and "works better".An enum is generally more lines of code.It's 3 lines, one of which is "};". The static const int is 2 (since you need storage).You usually don't, in C++11 and onward.The intent of the language (see various standards committee email threads on this in the C++0x days) was always to allow things like "static const size_t i = 2;" in class declarations and not require storage, but in olden days this often only worked with ints, and it didn't get properly fixed in the language until C++11. Now that we have C++11-conformant compilers this is generally doable without storage.
On Tue, Aug 11, 2015 at 5:22 PM, Nico Weber <tha...@chromium.org> wrote:It's 3 lines, one of which is "};". The static const int is 2 (since you need storage).You usually don't, in C++11 and onward.The intent of the language (see various standards committee email threads on this in the C++0x days) was always to allow things like "static const size_t i = 2;" in class declarations and not require storage, but in olden days this often only worked with ints, and it didn't get properly fixed in the language until C++11. Now that we have C++11-conformant compilers this is generally doable without storage.
On Tue, Aug 11, 2015 at 5:36 PM, Peter Kasting <pkas...@chromium.org> wrote:On Tue, Aug 11, 2015 at 5:22 PM, Nico Weber <tha...@chromium.org> wrote:On Tue, Aug 11, 2015 at 5:14 PM, Peter Kasting <pkas...@chromium.org> wrote:On Tue, Aug 11, 2015 at 5:09 PM, Nico Weber <tha...@chromium.org> wrote:If you don't need to take the address, use an enum. It's simpler, works better,I disagree with both "simpler" and "works better".An enum is generally more lines of code.It's 3 lines, one of which is "};". The static const int is 2 (since you need storage).You usually don't, in C++11 and onward.The intent of the language (see various standards committee email threads on this in the C++0x days) was always to allow things like "static const size_t i = 2;" in class declarations and not require storage, but in olden days this often only worked with ints, and it didn't get properly fixed in the language until C++11. Now that we have C++11-conformant compilers this is generally doable without storage.That's not true. A declaration in a class still requires storage in C++11 if it is ODR-used. The definition of ODR-used is subtle and is modified further in C++14 and there's this open issues on the standard about different cases(example http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#2104) but I don't think your statement is correct in general. I believe that currently a static const in a class declaration still needs storage in the general case. If it's not ODR-used then it doesn't, but knowing whether something is or ever will be ODR-used is incredibly subtle - for instance if you use the value in a log statement you're almost certainly ODR-using it.
On Tue, Aug 11, 2015 at 5:36 PM, Peter Kasting <pkas...@chromium.org> wrote:On Tue, Aug 11, 2015 at 5:22 PM, Nico Weber <tha...@chromium.org> wrote:It's 3 lines, one of which is "};". The static const int is 2 (since you need storage).You usually don't, in C++11 and onward.The intent of the language (see various standards committee email threads on this in the C++0x days) was always to allow things like "static const size_t i = 2;" in class declarations and not require storage, but in olden days this often only worked with ints, and it didn't get properly fixed in the language until C++11. Now that we have C++11-conformant compilers this is generally doable without storage.I don't believe you:$ cat t.cpp#include <algorithm>struct A { static const int x = 42; };
On Tue, Aug 11, 2015 at 5:47 PM, James Robinson <jam...@chromium.org> wrote:On Tue, Aug 11, 2015 at 5:36 PM, Peter Kasting <pkas...@chromium.org> wrote:On Tue, Aug 11, 2015 at 5:22 PM, Nico Weber <tha...@chromium.org> wrote:On Tue, Aug 11, 2015 at 5:14 PM, Peter Kasting <pkas...@chromium.org> wrote:On Tue, Aug 11, 2015 at 5:09 PM, Nico Weber <tha...@chromium.org> wrote:If you don't need to take the address, use an enum. It's simpler, works better,I disagree with both "simpler" and "works better".An enum is generally more lines of code.It's 3 lines, one of which is "};". The static const int is 2 (since you need storage).You usually don't, in C++11 and onward.The intent of the language (see various standards committee email threads on this in the C++0x days) was always to allow things like "static const size_t i = 2;" in class declarations and not require storage, but in olden days this often only worked with ints, and it didn't get properly fixed in the language until C++11. Now that we have C++11-conformant compilers this is generally doable without storage.That's not true. A declaration in a class still requires storage in C++11 if it is ODR-used. The definition of ODR-used is subtle and is modified further in C++14 and there's this open issues on the standard about different cases(example http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#2104) but I don't think your statement is correct in general. I believe that currently a static const in a class declaration still needs storage in the general case. If it's not ODR-used then it doesn't, but knowing whether something is or ever will be ODR-used is incredibly subtle - for instance if you use the value in a log statement you're almost certainly ODR-using it.I'm speaking about non-ODR-used constants. Most such constants aren't used in LOG statements and similar. I always default to using consts instead of enums and I very rarely need to go back and add storage, because the usage of such constants is usually very simple.
On Tue, Aug 11, 2015 at 5:58 PM, Peter Kasting <pkas...@chromium.org> wrote:I'm speaking about non-ODR-used constants. Most such constants aren't used in LOG statements and similar. I always default to using consts instead of enums and I very rarely need to go back and add storage, because the usage of such constants is usually very simple.That only works if you assume every developer who is going to use this constant knows when it is being ODR-used and when it isn't.
To sum up: Just use an enum if you don't need to take the address. No special cases to keep in mind, and it's a simple rule.
Unless I am mistaken about the consequences of not allocating storage, this is pretty easy: declare your const, then if it compiles on all our targets you're good, and if it does happen to be ODR-used it won't compile and you can either add storage or switch to an enum. Is there a case where this is undefined behavior rather than a compiler error? Because if so, then I will concede that that's a bad enough consequence to change the recommendation here.
Yes. See https://groups.google.com/a/chromium.org/forum/m/#!msg/blink-dev/dFwjUNDGI3k/caPm1A0hCacJ for an example.
Daniel
--
This is undefined in the arbitrarily bad and not always compiler detectable way. If you use a const static, *always* define storage. If you don't want to do that, *always* use an enum. static const without storage is playing with fire.
The aspect of the ODR ([basic.def.odr]) that Peter wants to risk
violating is "Every program shall contain exactly one definition of
every non-inline function or variable that is odr-used in that
program; no diagnostic required." I'm pretty sure all compiler/linker
systems actually diagnose this one. Not certain, but I don't remember
a missed diagnostic here. It's true, however, that "no diagnostic
required" is a red flag.
Linkers don't diagnose the rule 2 paragraphs later: "There can be more
than one definition of [lots of things] ... provided the definitions
satisfy the following requirements." That's the part the
operator<<(WTF::String) example violates.
However, I think Peter's guideline lays traps for future maintainers
and should be avoided anyway, even though the traps aren't generally
made of undefined behavior. Leaving out the storage for a const static
means someone's going to have to add it when they make some other
innocuous change, which will slow them down and make them get extra
LGTMs. The original author should be conscientious enough to add the
storage instead.
--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.
Thanks everyone for the detailed discussion. It sounds like the risk of undefined ODR-use-related behavior when using static const missing storage definition is severe enough to warrant clarification of preference for enums in the style guide. WDYT?
Wow, I leaned a lot from this thread.What do we do about existing class level static consts? I think in many ("most" I guess) cases we are not adding a storage.
Understood. But potentially we could have hidden undefined behavior in the existing code which isn't causing compiling errors, right?
I too had missed the import of Jeff Yaskin's message, and probably myself caused Xiaohan's confusion. My apologies.
A question of style came up in regards to this thread in a recent code review.Currently we have code such asstatic const int kMyConstant = 5;We'd be changing that based on guidance to an enum. My first expectation was that it would look likeenum : int { kMyConstant = 5 };But it was pointed out that technically the chromium styleguide overrides the google one on this point, and would expect the name of the constant to change to MY_CONSTANT.Personally, I don't think that makes a lot of sense in this case, it would require changing all uses of the constant, and misrepresent it's intentions (I'd expect MY_CONSTANT to be one of a set/bitset of other things). So I would prefer kMyConstant.My proposal is that enums-as-constants (ie. enums with a single member with an explicit value) be written kMyConstant as they would for non-enum constants. I hope this won't displease folks.
--On Mon, Aug 17, 2015 at 2:49 PM, Matthew Wolenetz <wole...@chromium.org> wrote:I too had missed the import of Jeff Yaskin's message, and probably myself caused Xiaohan's confusion. My apologies.
On Aug 17, 2015 2:34 PM, "Xiaohan Wang (王消寒)" <xhw...@chromium.org> wrote:--I see. I misunderstood the main reason of preferring enum. Thanks for the clarification!On Mon, Aug 17, 2015 at 2:29 PM, Peter Kasting <pkas...@chromium.org> wrote:On Mon, Aug 17, 2015 at 2:26 PM, Xiaohan Wang (王消寒) <xhw...@chromium.org> wrote:Understood. But potentially we could have hidden undefined behavior in the existing code which isn't causing compiling errors, right?See Jeff Yaskin's message; it seems unlikely that there's undefined behavior that's currently not being diagnosed by any compiler. The risks of these are more in terms of possibly causing compile failures if future maintainers change things.PK
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.
On Mon, Nov 2, 2015 at 3:04 PM, Dana Jansens <dan...@chromium.org> wrote:A question of style came up in regards to this thread in a recent code review.Currently we have code such asstatic const int kMyConstant = 5;We'd be changing that based on guidance to an enum. My first expectation was that it would look likeenum : int { kMyConstant = 5 };But it was pointed out that technically the chromium styleguide overrides the google one on this point, and would expect the name of the constant to change to MY_CONSTANT.Personally, I don't think that makes a lot of sense in this case, it would require changing all uses of the constant, and misrepresent it's intentions (I'd expect MY_CONSTANT to be one of a set/bitset of other things). So I would prefer kMyConstant.My proposal is that enums-as-constants (ie. enums with a single member with an explicit value) be written kMyConstant as they would for non-enum constants. I hope this won't displease folks.I would support this in cases where there is exactly one name with a specified value, since the intent of the code is to declare a constant. Would this be a change to the style guide regarding ENUM_NAMING (ie an exception to the rule)?
I encouraged Dana to write this email because I think the proposal here makes sense but the wording of the Chromium style guide needs to be tweaked to allow this flexibility.
--
Hey,Did this style guide change ever happen? I wanted to refer to it on a codereview I'm doing right now. I've checked both https://www.chromium.org/developers/coding-style and https://www.chromium.org/developers/coding-style/cpp-dos-and-donts.
--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
Yes, I saw that. I meant, was there a summary of the pro/con discussion about when to use the enum hack?In particular, I want to define a constexpr int in a header file included by a bunch of other files. This is not in a class, but in a namespace. I don't want the compiler to feel it needs to allocate storage and/or complicate things with export/linker foo across components.
Yes, I saw that. I meant, was there a summary of the pro/con discussion about when to use the enum hack?
In particular, I want to define a constexpr int in a header file included by a bunch of other files. This is not in a class, but in a namespace. I don't want the compiler to feel it needs to allocate storage and/or complicate things with export/linker foo across components.
I'd just like to clarify something (several months later). I was reading http://stackoverflow.com/questions/34445336/constexpr-global-constants-in-a-header-file-and-odr and as far as I understand it constexpr doesn't help this whole issue, does it? That is, you can have a global constexpr variable and take its address which is an odr use. Am I missing something?