Proposal to initialize static const int data members in class definitions

[Wan-Teh Chang]

In C++98, the ability to initialize static const integral data members
in class definitions was added. For example,

class TCPClientSocket {
...
private:
static const int kBufferSize = 32 * 1024;
...
};

Several well-known C++ books published before year 2000, such as The
C++ Programming Language, 2nd Ed.; Effective C++, 2nd Ed.; and More
Effective C++, mentioned this new feature.

Unfortunately Visual C++ did not allow this until recently. So the
Chromium source tree has a lot of code that did it the old-fashioned
way:

class TCPClientSocket {
...
private:
static const int kBufferSize;
...
};

// static
const int TCPClientSocket::kBufferSize = 32 * 1024;

But the Chromium source tree also has code that uses the new feature.
This implies all the compilers we use support this feature.

Therefore, I'd like to propose that in new code, we initialize static
const integral data members in class definitions because that makes
the code easier to understand. I don't think we need to go back and
update the old code.

Two notes:

1. Only static const members of integral types (such as int, char,
size_t) can be initialized in class definitions. For example, a
static const data member of type double still needs to be initialized
outside the class definition.

2. The C++ books also showed examples of defining static const
integral data members, without initializers. Our example above would
look like:

class TCPClientSocket {
...
private:
static const int kBufferSize = 32 * 1024;
...
};

// static
const int TCPClientSocket::kBufferSize;

I am not sure if this definition is necessary if we never takes a
reference or the address of the constant.

Wan-Teh Chang

[Antoine Labour]

In my experience that's where Visual Studio vs gcc is painful.

IIUC the spec says you have to add exactly one definition into a translation unit. But if you do so, Visual Studio complains about a duplicate definition. If you take the address/reference, gcc complains about a missing symbol if you don't do so.

So in practice you have to ifdef the definition if you do take a reference (even const) or the address of that constant.

Antoine

Wan-Teh Chang

--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
   http://groups.google.com/a/chromium.org/group/chromium-dev

[William Chan (陈智昌)]

Is this still true in VS2010? I'd hope they'd have fixed this.

[Antoine Labour]

On Wed, Aug 31, 2011 at 4:19 PM, William Chan (陈智昌) <will...@chromium.org> wrote:

Is this still true in VS2010? I'd hope they'd have fixed this.

I have no idea.

Have we dropped support for VS2008 ?

Antoine

[William Chan (陈智昌)]

On Wed, Aug 31, 2011 at 4:34 PM, Antoine Labour <pi...@chromium.org> wrote:
>
>
> On Wed, Aug 31, 2011 at 4:19 PM, William Chan (陈智昌) <will...@chromium.org>
> wrote:
>>
>> Is this still true in VS2010? I'd hope they'd have fixed this.
>
> I have no idea.
> Have we dropped support for VS2008 ?

I think that's the plan for
not-too-far-in-the-future-but-not-right-now but hasn't happened yet. I
always forget. I'm sure it's the plan for *some* point in the future.

[Scott Graham]

I guess we'd be exposing slightly more physical dependency, assuming
the class body is in a header.

IANALL, but if there's trouble between compilers, is there some reason
we don't just use enum for plain constants? Taking the address of a
constant can't be especially common. And, as a side effect we'd ensure
there's no init-time code running.

(ps, shed -> Fuchsia)

On Wed, Aug 31, 2011 at 3:56 PM, Wan-Teh Chang <w...@google.com> wrote:

[Ami Fischman]

I am not sure if this definition is necessary if we never takes a
reference or the address of the constant.

C++98 9.4.2 says:

-5- There shall be exactly one definition of a static data member that is used in a program;

I agree w/ scottmg@ that if there's no need to take the address of the constant, using an enum for static const integral quantities is nicer since it allows in-line initialization without requiring dropping an out-of-line definition in another file.

Cheers,

-a

[Peter Kasting]

On Wed, Aug 31, 2011 at 4:08 PM, Antoine Labour <pi...@chromium.org> wrote:

To elaborate slightly, since I've been trying to use wtc's proposed pattern quite a bit lately:

Various functions like DCHECK_EQ() can result in taking the address of their arguments (Nico had a guess at why this is, it's too complicated for me to fully understand, but it involves template resolution).  This means if you define a constant in a header that you use in one of these sorts of calls, you may run into "missing symbol" link errors on Mac and Linux.

If you then try to resolve this by adding storage for the variable by declaring it in a .cc file, then as Antoine mentioned, Visual Studio (at least VS2008, haven't tried 2010) may complain about duplicate symbols at link time.

There are workarounds.  The obvious one is to change DCHECK_EQ(a, b) to DCHECK(b == a) and similar.  I use this in some of my patches.

My overall feeling with the pattern that wtc suggests is that it's useful in some places but too problematic to recommend in general.  I like the readability of it, so where it's possible to easily use I'm not opposed, but I don't think we should recommend it as the default style.  I think the "declare in header, define in .cc file" pattern we use today is safer and easier even if it does require looking in the .cc file when you need to know a constant's value.

On Wed, Aug 31, 2011 at 4:39 PM, William Chan (陈智昌) <will...@chromium.org> wrote:

On Wed, Aug 31, 2011 at 4:34 PM, Antoine Labour <pi...@chromium.org> wrote:
> On Wed, Aug 31, 2011 at 4:19 PM, William Chan (陈智昌) <will...@chromium.org>
> wrote:
>> Is this still true in VS2010? I'd hope they'd have fixed this.
>
> I have no idea.
> Have we dropped support for VS2008 ?

I think that's the plan for
not-too-far-in-the-future-but-not-right-now but hasn't happened yet. I
always forget. I'm sure it's the plan for *some* point in the future.

I don't think it's the plan any time soon.  I wanted to do it until I was told that the GUI and debugger in VS2010 is even worse (buggier) than 2008.

On Wed, Aug 31, 2011 at 4:47 PM, Ami Fischman <fisc...@chromium.org> wrote:

I agree w/ scottmg@ that if there's no need to take the address of the constant, using an enum for static const integral quantities is nicer since it allows in-line initialization without requiring dropping an out-of-line definition in another file.

I am opposed to this pattern.  In fact I've been trying to eliminate usage of it lately, for a couple of reasons.

* When using enums to declare constants, the constants' type is effectively implementation-defined.  If the enum is anonymous, then this is worse than having a constant with an explicit type for various reasons, including requiring annoying casts on some compilers but not others, and not necessarily type-checking (or objecting on questionable type conversions) in code that uses the constant.  I would argue that using an anonymous enum for this is somewhat similar to using a #define, and would hope that people prefer "static const" to #define.  If the enum is named, life is somewhat better since while the underlying type is still implementation-dependent, at least code can hopefully use the enum name as the type name in most places.

* When defining bitfields, using an enum is particularly troublesome, since code attempting to use the bit values cannot actually name the enum and use the named type unless the enum declares all possible bit combinations.  As a result we have code that declares the bits in an enum and then uses "int" for the actual bitfield type, which causes the above problems and can't be easily fixed by naming the enum.  I now consider this an anti-pattern compared to:

typedef uint32 MyBitfieldType;

static const MyBitfieldType VAL_A = 1 << 0;

static const MyBitfieldType VAL_B = 1 << 1;

...which allows all places that want to refer to a bitfield value to use the named type.  Converting existing code to this pattern has already caught one bug and I find the results to be much clearer than "int".  Readability-wise, this case is sort of analogous to why we suggest using an enumerated type instead of "bool" in some cases.

* In C++03, passing a value of anonymous type to a template arg -- like what happens when you use one of these constants with DCHECK_EQ() -- is illegal.  This was mostly relaxed in C++11, but we don't compile with C++11 yet.  While we disable this warning, it is preferable not to write illegal code if we can avoid it.

PK

[Scott Hess]

I recently hit this problem with a patch which used std::min() on such
a value. It was fun, because on Mac I got a link error like:

Undefined symbols:
"base::debug::StackTrace::kMaxTraces", referenced from:
__ZN4base5debug10StackTrace10kMaxTracesE$non_lazy_ptr in
libbase.a(stack_trace.o)
(maybe you meant: __ZN4base5debug10StackTrace10kMaxTracesE$non_lazy_ptr)
ld: symbol(s) not found

Pore over that for a few minutes and tell me what's different about
the target reference and the "maybe you meant" reference!

[Give up yet? They're identical.]

-scott

http://en.wikipedia.org/wiki/One_Definition_Rule#Definitions_of_static_const_data_members
was as close as I got to why. You can also manage it by casting to
force a constant context.

[Peter Kasting]

Oh, and finally, since I think C++11 is supposed to fix a lot of the cases that make wtc's proposal not work well, then I suspect we'll want to make that the standard pattern once we switch to C++11 mode.  /cue argument as to when and how that should happen.

PK

[Ami Fischman]

* When using enums to declare constants, the constants' type is effectively implementation-defined.  If the enum is anonymous, then this is worse than having a constant with an explicit type for various reasons, including requiring annoying casts on some compilers but not others, and not necessarily type-checking (or objecting on questionable type conversions) in code that uses the constant.  I would argue that using an anonymous enum for this is somewhat similar to using a #define, and would hope that people prefer "static const" to #define.

Hah.  My take on it is that using an enum is similar to using the numeric literal, which is what people think in, but with the up-side of abstraction (compared to the literal) and of no syntactic confusion (compared to the #define).

 

* When defining bitfields, using an enum is particularly troublesome, since code attempting to use the bit values cannot actually name the enum and use the named type unless the enum declares all possible bit combinations.

Why not?  The underlying type can hold all combinations (7.2-6-).

* In C++03, passing a value of anonymous type to a template arg -- like what happens when you use one of these constants with DCHECK_EQ() -- is illegal.  This was mostly relaxed in C++11, but we don't compile with C++11 yet.  While we disable this warning, it is preferable not to write illegal code if we can avoid it.

Would you say r99087 is an example of the tradeoff you're making?

Before: we have a disabled warning that only triggers on this (fixed in (not-yet-adopted by chromium) newer spec) spec bug and we get: in-line readability, no need for out-of-line definition, and usability with DCHECK_EQ.

After: we can remove the warning disablement, we have in-line readability, no need for out-of-line definition, but can't use with DCHECK_EQ.

I don't understand how this is an argument against the use of anonymous enums over static consts.

Once you give up DCHECK_EQ (which you must in the After scenario) the anonymous enum is perfectly fine, even without disabling the warning.

Cheers,

-a

[Carlos Pizano]

Same here. All other things being equal you should try to use a typed
constant for numeric values, kBufferSize would be a good example.

-cpu

ps:In my experience the VS2010 debugger is way faster that in VS2008.

[Peter Kasting]

On Wed, Aug 31, 2011 at 6:18 PM, Ami Fischman <fisc...@chromium.org> wrote:

* When using enums to declare constants, the constants' type is effectively implementation-defined.  If the enum is anonymous, then this is worse than having a constant with an explicit type for various reasons, including requiring annoying casts on some compilers but not others, and not necessarily type-checking (or objecting on questionable type conversions) in code that uses the constant.  I would argue that using an anonymous enum for this is somewhat similar to using a #define, and would hope that people prefer "static const" to #define.

Hah.  My take on it is that using an enum is similar to using the numeric literal, which is what people think in, but with the up-side of abstraction (compared to the literal) and of no syntactic confusion (compared to the #define).

I think part of my bias here is that I think of enum as being "properly" used for truly "enumerated" types, and consider the use of it to define individual constants to be a hack compared with the "semantically correct" use of "const <type>".  This is of course about as nitpicky as web authors preferring <strong> to <b> so perhaps we're off in the weeds here.  That said, at least a few other people have in the past been approving of changes that move constants defined as anonymous enums to being static consts, so I'm not sure I'm alone.

* When defining bitfields, using an enum is particularly troublesome, since code attempting to use the bit values cannot actually name the enum and use the named type unless the enum declares all possible bit combinations.

Why not?  The underlying type can hold all combinations (7.2-6-).

Thanks for the spec section link.  That was an interesting read.  I downgrade my objection here to:

Code actually trying to use a named enum type will keep running into problems because any attempt to perform bit operations on the enum values will end up converting to the underlying type (or some promotion thereof), necessitating a cast.  So if you do:

enum X { VAL_A = ..., VAL_B = ..., ... };

X x = VAL_A | VAL_B;

The compiler whines at you that you need to explicitly cast.  Using a typedefed integral type here avoids this issue.

I still think this is a serious enough problem that I would avoid enums at least for bitfield types, even if not avoiding them elsewhere.

* In C++03, passing a value of anonymous type to a template arg -- like what happens when you use one of these constants with DCHECK_EQ() -- is illegal.  This was mostly relaxed in C++11, but we don't compile with C++11 yet.  While we disable this warning, it is preferable not to write illegal code if we can avoid it.

Would you say r99087 is an example of the tradeoff you're making?

Yes.  Interestingly, according to my try runs on some other patches, the DHECK_EQ() (and similar) problem doesn't seem to rear its head all the time, so this (and other patches of mine) may have been too aggressive in preemptively removing it.

You may be right that right now some of this is "six of one, half a dozen of the other" (or even less compelling in some cases); I am curious whether your opinion would change after we move to C++11, in which it seems like we could probably use static consts without the current downsides.

PK

[Ami Fischman]

I think part of my bias here is that I think of enum as being "properly" used for truly "enumerated" types, and consider the use of it to define individual constants to be a hack compared with the "semantically correct" use of "const <type>".

...but of course the <type> here is not more semantically informative than "integer" :)

 

The compiler whines at you that you need to explicitly cast.  Using a typedefed integral type here avoids this issue.

I agree that a typedef'd integral type works better than an enum for this reason.

 

I am curious whether your opinion would change after we move to C++11, in which it seems like we could probably use static consts without the current downsides.

AFAICT c++11 doesn't change things; 9.4.2 -3- & -4- in it seem equivalent to c++98's 9.4.2-5- as far as this discussion goes.

FTR, I agree that "static const int" reads a lot more like "this is a universal integral constant" than does "enum {}" :)  I just like the lack of definition turds in .cc files more.

Certainly I agree this is a tradeoff with pros & cons on both sides.

Cheers,

-a

[Greg Thompson]

Counter argument: "static const int" is something that can be passed around by const ref, can have its address taken, and may very well end up in the .rdata section of binaries.  Why are those properties interesting for members of a bitfield?  I find it pretty to put the bits of a bitfield in an enum, but I wouldn't use that enum type as the type of the bitfield itself because of the excellently annoying casting required.  For example:

class Blibber {

 private:

  enum BlubberBits {

    STICKY = 1 << 0,

    GOOEY = 1 << 1,

    SLIMY = 1 << 2,

  };

  typedef uint8 Blubber; // bitwise inclusive OR of any combination of BlubberBits.

  Blubber blubber_;

};

For that matter, however, why not use facilities of the language that do all this annoying stuff for us?  Do some of our compilers not like this:

class Blibber {

 private:

  struct Blubber {

    bool sticky : 1;

    bool gooey : 1;

    bool slimy : 1;

  };

  Blubber blubber_;

};

And now code to access/modify the bits of a Blubber are insanely simple: no masking off bits to clear them, etc.  Why isn't this more common?

[Fred Akalin]

On Thu, Sep 1, 2011 at 7:57 AM, Greg Thompson <g...@chromium.org> wrote:
> For that matter, however, why not use facilities of the language that do all
> this annoying stuff for us?  Do some of our compilers not like this:
> class Blibber {
>  private:
>   struct Blubber {
>     bool sticky : 1;
>     bool gooey : 1;
>     bool slimy : 1;
>   };
>   Blubber blubber_;
> };
> And now code to access/modify the bits of a Blubber are insanely simple: no
> masking off bits to clear them, etc.  Why isn't this more common?

I believe most compilers do support bitfields, but people don't use
them because supposedly compilers generate suboptimal code. I'm not
sure how true this is today.

[Peter Kasting]

On Thu, Sep 1, 2011 at 7:57 AM, Greg Thompson <g...@chromium.org> wrote:

For example:

class Blibber {

 private:

  enum BlubberBits {

    STICKY = 1 << 0,

    GOOEY = 1 << 1,

    SLIMY = 1 << 2,

  };

  typedef uint8 Blubber; // bitwise inclusive OR of any combination of BlubberBits.

  Blubber blubber_;

};

The problem with this is that the individual bit values do not have the same type as the typedef.  This can become annoying if your compiler decides to give them a different signedness or something from the typedef (e.g. as far as I can tell, VS2008 changes whether the enum's underlying type is signed based on whether the enum is anonymous, which seems insane, but is legal).  This precise problem has meant that in the past I've needed to add static_cast<>()s for code like the above when doing simple things like comparing in a DCHECK_EQ().  Boo to that.

For that matter, however, why not use facilities of the language that do all this annoying stuff for us?  Do some of our compilers not like this:

class Blibber {

 private:

  struct Blubber {

    bool sticky : 1;

    bool gooey : 1;

    bool slimy : 1;

  };

  Blubber blubber_;

};

And now code to access/modify the bits of a Blubber are insanely simple: no masking off bits to clear them, etc.  Why isn't this more common?

One reason is that it can be a bit less clear in this case precisely which physical bits of a value correspond to the logical bits that are named.  This matters if you need to e.g. mask off a group of bits in the type (see e.g. the page transition types).  In fact if you're not careful, you can sometimes write code that packs in an unexpected way that results in the complete type becoming wide enough to kill your perf (because you can no longer hold it in a register, or whatever) -- this is a risk if you start mixing bools with other types in the packed object.

It's also true as Fred mentioned that compilers can generate worse code, though in general the auto-generated code for the above is about the same as the hand-generated masking that we'd do with other methods.

PK

[Greg Thompson]

On Thu, Sep 1, 2011 at 1:36 PM, Peter Kasting <pkas...@google.com> wrote:

On Thu, Sep 1, 2011 at 7:57 AM, Greg Thompson <g...@chromium.org> wrote:

For that matter, however, why not use facilities of the language that do all this annoying stuff for us?  Do some of our compilers not like this:

class Blibber {

 private:

  struct Blubber {

    bool sticky : 1;

    bool gooey : 1;

    bool slimy : 1;

  };

  Blubber blubber_;

};

And now code to access/modify the bits of a Blubber are insanely simple: no masking off bits to clear them, etc.  Why isn't this more common?

One reason is that it can be a bit less clear in this case precisely which physical bits of a value correspond to the logical bits that are named.  This matters if you need to e.g. mask off a group of bits in the type (see e.g. the page transition types).

This sounds like an argument for using explicit bit twiddling if bit positions are part of some kind of contract.  This makes sense.

 

In fact if you're not careful, you can sometimes write code that packs in an unexpected way that results in the complete type becoming wide enough to kill your perf (because you can no longer hold it in a register, or whatever) -- this is a risk if you start mixing bools with other types in the packed object.

Sounds like something clang could find for us, no?  Carefully designing types vs. carefully crafting all call sites seems like a better problem to me.  Who hasn't seen bad use of parens around something like:

if ((spam & foo) !=0 || (spam & bar) != 0) {...}

There are all kinds of ways to do that wrong that the compiler won't tell you about.

 

It's also true as Fred mentioned that compilers can generate worse code, though in general the auto-generated code for the above is about the same as the hand-generated masking that we'd do with other methods.

 

I just checked VS2008, and there's no difference in the generated code.  I would be surprised if modern compilers generate worse code.  (Note: I've been surprised before.)  This argument sounds a bit like "language feature X was horrible back in the Cfront days.  Stay away!"

[Peter Kasting]

On Thu, Sep 1, 2011 at 11:01 AM, Greg Thompson <g...@chromium.org> wrote:

On Thu, Sep 1, 2011 at 1:36 PM, Peter Kasting <pkas...@google.com> wrote:

One reason is that it can be a bit less clear in this case precisely which physical bits of a value correspond to the logical bits that are named.  This matters if you need to e.g. mask off a group of bits in the type (see e.g. the page transition types).

This sounds like an argument for using explicit bit twiddling if bit positions are part of some kind of contract.  This makes sense.

It's more than just "part of a contract".  It's "any time it's convenient for some user to access groups of values within the bitfield".  The page transition types I mentioned before are an example.  There's no need for some kind of contract that particular bits have to be at particular locations, there's merely a need by some callers to be able to strip the type down to only a subset of its components.  Using explicitly-placed bits and mask values is merely the easiest way to accomplish this.

In fact if you're not careful, you can sometimes write code that packs in an unexpected way that results in the complete type becoming wide enough to kill your perf (because you can no longer hold it in a register, or whatever) -- this is a risk if you start mixing bools with other types in the packed object.

Sounds like something clang could find for us, no?

I'm not sure what you're saying.  There's nothing illegal about writing structs whose bitfields pack in ways that expand the overall struct size, so I don't know what you're hoping Clang would catch.

 

It's also true as Fred mentioned that compilers can generate worse code, though in general the auto-generated code for the above is about the same as the hand-generated masking that we'd do with other methods.

 

I just checked VS2008, and there's no difference in the generated code.  I would be surprised if modern compilers generate worse code.  (Note: I've been surprised before.)  This argument sounds a bit like "language feature X was horrible back in the Cfront days.  Stay away!"

No, it was intended as the reverse: "this is a theoretical problem but it doesn't matter in most practical situations so don't worry about it".

What I was trying to say overall was that there are reasons why in some cases the pattern you propose has flaws.  It's a reasonable pattern to use when all you want to do is the simple case of a few flags packed into a field.  I personally don't find the code for "if (val & FLAG1)" to be terribly more onerous than "if (val.flag1)", and since each has minor downsides in some scenarios, it seems OK to me for people to choose different ones at different times.

PK

[Steve VanDeBogart]

On Wed, Aug 31, 2011 at 6:44 PM, Peter Kasting <pkas...@google.com> wrote:

On Wed, Aug 31, 2011 at 6:18 PM, Ami Fischman <fisc...@chromium.org> wrote:

* When using enums to declare constants, the constants' type is effectively implementation-defined.  If the enum is anonymous, then this is worse than having a constant with an explicit type for various reasons, including requiring annoying casts on some compilers but not others, and not necessarily type-checking (or objecting on questionable type conversions) in code that uses the constant.  I would argue that using an anonymous enum for this is somewhat similar to using a #define, and would hope that people prefer "static const" to #define.

Hah.  My take on it is that using an enum is similar to using the numeric literal, which is what people think in, but with the up-side of abstraction (compared to the literal) and of no syntactic confusion (compared to the #define).

I think part of my bias here is that I think of enum as being "properly" used for truly "enumerated" types, and consider the use of it to define individual constants to be a hack compared with the "semantically correct" use of "const <type>".  This is of course about as nitpicky as web authors preferring <strong> to <b> so perhaps we're off in the weeds here.  That said, at least a few other people have in the past been approving of changes that move constants defined as anonymous enums to being static consts, so I'm not sure I'm alone.

* When defining bitfields, using an enum is particularly troublesome, since code attempting to use the bit values cannot actually name the enum and use the named type unless the enum declares all possible bit combinations.

Why not?  The underlying type can hold all combinations (7.2-6-).

Thanks for the spec section link.  That was an interesting read.  I downgrade my objection here to:

Code actually trying to use a named enum type will keep running into problems because any attempt to perform bit operations on the enum values will end up converting to the underlying type (or some promotion thereof), necessitating a cast.  So if you do:

enum X { VAL_A = ..., VAL_B = ..., ... };

X x = VAL_A | VAL_B;

The compiler whines at you that you need to explicitly cast.  Using a typedefed integral type here avoids this issue.

I still think this is a serious enough problem that I would avoid enums at least for bitfield types, even if not avoiding them elsewhere.

Would the following snippet solve the problem well enough for you?

template <typename T>

T bitOr(T a, T b) {

  return static_cast<T>(a | b);

}

--

Steve

 

* In C++03, passing a value of anonymous type to a template arg -- like what happens when you use one of these constants with DCHECK_EQ() -- is illegal.  This was mostly relaxed in C++11, but we don't compile with C++11 yet.  While we disable this warning, it is preferable not to write illegal code if we can avoid it.

Would you say r99087 is an example of the tradeoff you're making?

Yes.  Interestingly, according to my try runs on some other patches, the DHECK_EQ() (and similar) problem doesn't seem to rear its head all the time, so this (and other patches of mine) may have been too aggressive in preemptively removing it.

You may be right that right now some of this is "six of one, half a dozen of the other" (or even less compelling in some cases); I am curious whether your opinion would change after we move to C++11, in which it seems like we could probably use static consts without the current downsides.

PK

--

[Greg Thompson]

On Thu, Sep 1, 2011 at 2:22 PM, Peter Kasting <pkas...@google.com> wrote:

On Thu, Sep 1, 2011 at 11:01 AM, Greg Thompson <g...@chromium.org> wrote:

On Thu, Sep 1, 2011 at 1:36 PM, Peter Kasting <pkas...@google.com> wrote:

One reason is that it can be a bit less clear in this case precisely which physical bits of a value correspond to the logical bits that are named.  This matters if you need to e.g. mask off a group of bits in the type (see e.g. the page transition types).

This sounds like an argument for using explicit bit twiddling if bit positions are part of some kind of contract.  This makes sense.

It's more than just "part of a contract".  It's "any time it's convenient for some user to access groups of values within the bitfield".  The page transition types I mentioned before are an example.  There's no need for some kind of contract that particular bits have to be at particular locations, there's merely a need by some callers to be able to strip the type down to only a subset of its components.  Using explicitly-placed bits and mask values is merely the easiest way to accomplish this.

Now I follow.  Agreed.

 

In fact if you're not careful, you can sometimes write code that packs in an unexpected way that results in the complete type becoming wide enough to kill your perf (because you can no longer hold it in a register, or whatever) -- this is a risk if you start mixing bools with other types in the packed object.

Sounds like something clang could find for us, no?

I'm not sure what you're saying.  There's nothing illegal about writing structs whose bitfields pack in ways that expand the overall struct size, so I don't know what you're hoping Clang would catch.

Your statement about packing applies to all compound types, not just bitfields, no (struct {char c1; int i; char c2;};)?  A compiler could tell you if you have a compound type with a lot of padding (or you can see for  yourself using VC++'s /d1reportSingleClassLayoutFOOBAR, where FOOBAR is a typename).  I swear I've had compilers warn me about such things.  I was suggesting that we could have compiler smarts tell us if a struct's padding is above some threshold (esp. if the struct contains bitfields).

It's also true as Fred mentioned that compilers can generate worse code, though in general the auto-generated code for the above is about the same as the hand-generated masking that we'd do with other methods.

 

I just checked VS2008, and there's no difference in the generated code.  I would be surprised if modern compilers generate worse code.  (Note: I've been surprised before.)  This argument sounds a bit like "language feature X was horrible back in the Cfront days.  Stay away!"

No, it was intended as the reverse: "this is a theoretical problem but it doesn't matter in most practical situations so don't worry about it".

I was refering to Fred's comment, not yours.  Apologies for the mis-attribution.

 

What I was trying to say overall was that there are reasons why in some cases the pattern you propose has flaws.  It's a reasonable pattern to use when all you want to do is the simple case of a few flags packed into a field.  I personally don't find the code for "if (val & FLAG1)" to be terribly more onerous than "if (val.flag1)", and since each has minor downsides in some scenarios, it seems OK to me for people to choose different ones at different times.

 

This sounds altogether reasonable.  Thanks for the discussion.

[Peter Kasting]

On Thu, Sep 1, 2011 at 11:24 AM, Steve VanDeBogart <van...@chromium.org> wrote:

On Wed, Aug 31, 2011 at 6:44 PM, Peter Kasting <pkas...@google.com> wrote:

Code actually trying to use a named enum type will keep running into problems because any attempt to perform bit operations on the enum values will end up converting to the underlying type (or some promotion thereof), necessitating a cast.  So if you do:

enum X { VAL_A = ..., VAL_B = ..., ... };

X x = VAL_A | VAL_B;

The compiler whines at you that you need to explicitly cast.  Using a typedefed integral type here avoids this issue.

Would the following snippet solve the problem well enough for you?

template <typename T>

T bitOr(T a, T b) {

  return static_cast<T>(a | b);

}

You'd also need to define bitAnd().  Then using them is cumbersome.  (And you can't do a |= b, you have to do a = bitOr(a, b).)  Is it possible?  Yes.  Would I prefer to use it?  No.

PK

[Scott Hess]

template <typename T>
T bitOrEqual(T &a, T b) {
  return a = bitOr(a, b);
}

It makes you tear up a little. Of course, we could also posit the
existence of an entire system of templates to enable definition of and
operation on bitwise types.

That said, I did recently find myself thinking that locking down the
bitwise operators wouldn't be a bad thing. The number of cases where
they are used and the author REALLY thought through the precedence
rules is probably pretty low. Abstracting it a little would be
annoying for those of us who are used to thinking about the bits, but
a lot of people don't really know how badly they're missing the
bit-level boat, unfortunately.

-scott

[Peter Kasting]

On Thu, Sep 1, 2011 at 12:38 PM, Scott Hess <sh...@chromium.org> wrote:

That said, I did recently find myself thinking that locking down the

bitwise operators wouldn't be a bad thing.  The number of cases where
they are used and the author REALLY thought through the precedence
rules is probably pretty low.  Abstracting it a little would be
annoying for those of us who are used to thinking about the bits, but
a lot of people don't really know how badly they're missing the
bit-level boat, unfortunately.

:/  Seems easier to adopt the general practice: always place parens around all binary operations within any compound statement, so that precedence is never relevant.  I do this, hopefully some other people do too.

PK 

[Scott Hess]

Yeah, the case which got me thinking was (x & mask == 0). It seems
like it might be reasonable to warn when you have to use precedence to
decide between bitwise operators and equality, and also possibly
between things like bitwise operators and arithmetic operators. For
instance, (x << b | flag) seems somewhat reasonable, since it's all
bitwise, but (x + y & mask) is somewhat questionable, to my mind -
indeed, a certain kind of C programmer embeds an ability to
transparently compose bitwise operations out of 2s-complement
arithmetic (sometimes I don't even notice I've done it), but that
doesn't mean it is appropriate when working on a large project like
this. (x & y && flag) seems right out, though if it were (x & y AND
flag) it wouldn't be so bad (I'm more concerned about the potential
for mis-reading intention that case).

Having (x << shift == mask) work right and (x & mask == 0) work wrong
is really wonderful :-).

I think the velocipede outbuilding should be mauve, BTW.

-scott

[Peter Kasting]

On Thu, Sep 1, 2011 at 12:59 PM, Scott Hess <sh...@chromium.org> wrote:

Yeah, the case which got me thinking was (x & mask == 0)

I just realized that this is an example in support of WebKit's style rule that one must use "!cond" instead of "cond == 0".  I always thought that was rather arbitrary (but more compact so I was OK with it).

 

(x << b | flag)

(x + y & mask)

(x & y && flag) 

(x & y AND flag)

I'm so used to parenthesizing everything that these all look equally wrong to me :)

pK 

[Nico Weber]

On Thu, Sep 1, 2011 at 12:59 PM, Scott Hess <sh...@chromium.org> wrote:
> On Thu, Sep 1, 2011 at 12:41 PM, Peter Kasting <pkas...@google.com> wrote:
>> On Thu, Sep 1, 2011 at 12:38 PM, Scott Hess <sh...@chromium.org> wrote:
>>> That said, I did recently find myself thinking that locking down the
>>> bitwise operators wouldn't be a bad thing.  The number of cases where
>>> they are used and the author REALLY thought through the precedence
>>> rules is probably pretty low.  Abstracting it a little would be
>>> annoying for those of us who are used to thinking about the bits, but
>>> a lot of people don't really know how badly they're missing the
>>> bit-level boat, unfortunately.
>>
>> :/  Seems easier to adopt the general practice: always place parens around
>> all binary operations within any compound statement, so that precedence is
>> never relevant.  I do this, hopefully some other people do too.
>
> Yeah, the case which got me thinking was (x & mask == 0).

This is caught by clang (i.e. is a compile time error since we build
with -Werror):

hummer:src thakis$ third_party/llvm-build/Release+Asserts/bin/clang
-Wall test.cc
test.cc:3:9: warning: & has lower precedence than ==; == will be
evaluated first [-Wparentheses]
if (a & 1 == 0)
^~~~~~~~
test.cc:3:9: note: place parentheses around the == expression to
silence this warning
if (a & 1 == 0)
^
( )
test.cc:3:9: note: place parentheses around the & expression to
evaluate it first
if (a & 1 == 0)
^
( )
1 warning generated.

It doesn't warn on any of the other things you mention below.

>  It seems
> like it might be reasonable to warn when you have to use precedence to
> decide between bitwise operators and equality, and also possibly
> between things like bitwise operators and arithmetic operators.  For
> instance, (x << b | flag) seems somewhat reasonable, since it's all
> bitwise, but (x + y & mask) is somewhat questionable, to my mind -
> indeed, a certain kind of C programmer embeds an ability to
> transparently compose bitwise operations out of 2s-complement
> arithmetic (sometimes I don't even notice I've done it), but that
> doesn't mean it is appropriate when working on a large project like
> this.  (x & y && flag) seems right out, though if it were (x & y AND
> flag) it wouldn't be so bad (I'm more concerned about the potential
> for mis-reading intention that case).
>
> Having (x << shift == mask) work right and (x & mask == 0) work wrong
> is really wonderful :-).
>
> I think the velocipede outbuilding should be mauve, BTW.
>
> -scott
>