C++11 Feature Proposal: In-class Member Initialization

[Jeremy Roman]

What:
Allow and encourage use of in-class member initializers.

Why:
Improves code readability and avoids uninitialized members. It's very
easy to tell that all members will be initialized, and is particularly
clear in the case of a bunch of values that should all be null, zero,
etc.

For an example of where I think this would be useful in Chromium, I'll
steal danakj's link from the delegated constructor thread:
https://code.google.com/p/chromium/codesearch#chromium/src/cc/resources/resource_provider.cc&l=209

Quoth the Google C++ style guide:
"In-class member initialization ensures that a member variable will be
initialized appropriately without having to duplicate the
initialization code in multiple constructors. This can reduce bugs
where you add a new member variable, initialize it in one constructor,
and forget to put that initialization code in another constructor."

For example:

class Foo {
public:
Foo() {
}
Foo(Delegate* delegate)
: delegate_(delegate) {
}

private:
Delegate* delegate = nullptr;
bool active_ = false;
int id_ = 0;
};

without the feature, this looks like:

class Foo {
public:
Foo()
: delegate_(nullptr)
, active_(false)
, id_(0) {
}
Foo(Delegate* delegate)
: delegate_(delegate)
, active_(false)
, id_(0) {
}

private:
Delegate* delegate_;
bool active_;
int id_;
};

Proposal:

Adopt the same recommendation as the style guide: "Use in-class member
initialization for simple initializations, especially when a member
variable must be initialized the same way in more than one
constructor."

[Alex Vakulenko]

I generally support this. There are drawback, like non-static member initialization is effectively creating an inline constructor (or pieces of it) and the members are initialized at at call sites instead of once .cc translation unit where the class in implemented. For this, one still need to provide a default constructor and implement it in a .cc file to eliminate code duplication at call sites. But there are definite benefits in my mind, especially if we are talking about simple/trivial classes.

There interesting use cases for this that people might not be aware of. One thing involving weak factories which require initializing the factory object with "this". You still can do this without writing a custom constructor.

Compare:

class Foo {

 public:

  Foo();

  std::string str_;

  int val_;

  base::WeakPtrFactory<Foo> weak_ptr_factory_;

};

Foo::Foo() : str_("text"), val_(15), weak_ptr_factory_(this) {}

vs:

class Foo {

 public:

  std::string str_{"text"};

  int val_{15};

  base::WeakPtrFactory<Foo> weak_ptr_factory_{this};

};

You still can provide custom constructors on top to override the "default defaults":

class Foo {

 public:

  Foo() = default;

  Foo(int new_val);

  std::string str_{"text"};

  int val_{15};

  base::WeakPtrFactory<Foo> weak_ptr_factory_{this};

};

Foo::Foo(int new_val) : val_{new_val} {}

The custom constructor will still initialize str_ to "text" and weak_ptr_factory_ to this.

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

[Jeremy Roman]

On Thu, Sep 25, 2014 at 11:53 AM, Alex Vakulenko
<avaku...@chromium.org> wrote:
> I generally support this. There are drawback, like non-static member
> initialization is effectively creating an inline constructor (or pieces of
> it) and the members are initialized at at call sites instead of once .cc
> translation unit where the class in implemented.

I believe that this is already the case when any member has a
non-trivial default constructor.

Despite my example, I still think the constructors should be declared
in the .cc file in the same cases that we do today. They should just
have shorter initialization lists.

[Hendrik]

Your example with the custom constructor is one of the reasons I usually prefer to not use them, some values are initialized in the header, others in the source file (but only sometimes), it leads to more confusing code.

I'm not against using them, but for this case, I would prefer to see all of the variables initialized in one place.

For me something like:

struct point {

 double x = 0.0;

 double y = 0.0;

}; // cool

Minor point - changing defaults trigger larger compiles, which is really only an small annoyance.

[Jeremy Roman]

I suppose the counterpoint I would make is that I prefer to see what
the constructor is doing that's not obvious (zeroing out all the
primitive fields, creating a WeakPtrFactory, etc., are).
I think that the Google guide strikes a good balance, but I can
appreciate your concerns. I'm not sure what you're proposing, though;
do you want to restrict it to small structs?

I should add, though, that caveat I missed in my initial email was
that this is sadly not supported for bit-fields in C++11.
As a result, here's what the cc::ResourceProvider::Resource looks like
using in-class member initialization.
https://codereview.chromium.org/606733003/

Note that the delegating-constructor equivalent also has a drawback,
in that delegating constructors cannot have any other initializers.

[Hendrik]

I'm not proposing that we restrict anything, only that I prefer all intializations to be in one place.  I trust other developers to make decisions that make sense, I'd rather allow all C++11 features + a bit of education on the pitfalls.

Not being able to initialize bit-fields is annoying.  In this case, I'm wondering why these are even bit-fields, this class does not get created thousands of times, and doesn't need to be space compact.

Ignoring the bit-fields, was there a reason you didn't also initialize all of the other variables (e.g. |type| enum?), when I see that change I look at the header and wonder how many initialized variables we have.  I know this doesn't stop us from missing some (I fixed one today), but it's difficult to look at two sets of variables and see if the union of the set captures initialization for every variable.

On Thursday, September 25, 2014 11:03:43 AM UTC-7, Jeremy Roman wrote:

I suppose the counterpoint I would make is that I prefer to see what
the constructor is doing that's not obvious (zeroing out all the
primitive fields, creating a WeakPtrFactory, etc., are).
I think that the Google guide strikes a good balance, but I can
appreciate your concerns. I'm not sure what you're proposing, though;
do you want to restrict it to small structs?

I should add, though, that caveat I missed in my initial email was
that this is sadly not supported for bit-fields in C++11.
As a result, here's what the cc::ResourceProvider::Resource looks like
using in-class member initialization.
https://codereview.chromium.org/606733003/

Note that the delegating-constructor equivalent also has a drawback,
in that delegating constructors cannot have any other initializers.

[Alex Vakulenko]

I think the general recommendation should be:

If you use in-class member initialization, then all members that can be value or default initialized must be. Custom constructors can provide different values for some members as well as initialize members that don't have default constructors (e.g. references). This would at least guarantee that no uninitialized members are left behind.

--

[Jeremy Roman]

On Thu, Sep 25, 2014 at 2:22 PM, Hendrik <hend...@chromium.org> wrote:
> I'm not proposing that we restrict anything, only that I prefer all
> intializations to be in one place. I trust other developers to make
> decisions that make sense, I'd rather allow all C++11 features + a bit of
> education on the pitfalls.
>
> Not being able to initialize bit-fields is annoying. In this case, I'm
> wondering why these are even bit-fields, this class does not get created
> thousands of times, and doesn't need to be space compact.
>
> Ignoring the bit-fields, was there a reason you didn't also initialize all
> of the other variables (e.g. |type| enum?), when I see that change I look at
> the header and wonder how many initialized variables we have. I know this
> doesn't stop us from missing some (I fixed one today), but it's difficult to
> look at two sets of variables and see if the union of the set captures
> initialization for every variable.

tl;dr: I don't feel strongly about this, and the Google style guide
doesn't seem to comment on it.

In the case of the |type| enum, I wasn't convinced there one that was
a natural "zero".

My ultimate preference would be to have the *compiler* validate that
there are no uninitialized members (except where whitelisted), and
then to specify as defaults only things which make are logically
default/zero. Then the compiler would force you to supply a reasonable
value for each constructor.

But in the absence of that I don't care which approach is taken. A
rule like Alex's SGTM.

[James Robinson]

This sounds like a reasonable rule to me too.  Let's sit on this one for a bit, though, to let the other proposals through and let folks digest things.

- James

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.

[Christian Biesinger]

Hmm, this is a feature I hate in Java and I didn't realize that C++11 allowed it. I find it so confusing to have the initialization of a class distributed over the constructor and this kind of inline initialization. "initialized the same way in more than one constructor" can easily be handled by delegated constructors. Basically, without this feature, you only need to read the constructor to know how the variable is initialized, whereas with the feature, you need to look at declaration and constructors.

-christian

[Dana Jansens]

On Thu, Sep 25, 2014 at 3:24 PM, Christian Biesinger <cbies...@chromium.org> wrote:

Hmm, this is a feature I hate in Java and I didn't realize that C++11 allowed it. I find it so confusing to have the initialization of a class distributed over the constructor and this kind of inline initialization. "initialized the same way in more than one constructor" can easily be handled by delegated constructors. Basically, without this feature, you only need to read the constructor to know how the variable is initialized, whereas with the feature, you need to look at declaration and constructors.

Agree, my own preference would be something like "Only allowed when there is no initialization in the constructor" so you only have to look at one place or the other but never both.

[Jeremy Roman]

I don't understand how delegated constructors are any different in this regard.

With a delegated constructor, you still have to look at both the
initializing constructor (for what it initializes) and the called
constructor (for what it assigns).

On Thu, Sep 25, 2014 at 3:24 PM, Christian Biesinger
<cbies...@chromium.org> wrote:

[Dana Jansens]

On Thu, Sep 25, 2014 at 4:18 PM, Jeremy Roman <jbr...@chromium.org> wrote:

I don't understand how delegated constructors are any different in this regard.

With a delegated constructor, you still have to look at both the
initializing constructor (for what it initializes) and the called
constructor (for what it assigns).

Constructors should all be neighbours in the .cc file, and you have an explicit call to another constructor to trace. Whereas member variables appear at the bottom of the .h file and there's nothing in the .cc file to say they exist or not.

[Alex Vakulenko]

To play a devil's advocate here for a change, delegating constructors don't have their own initializer list. You either call another constructor or initialize the members yourself. After a delegated constructor is called, all the class members are fully initialized.

[Jeffrey Yasskin]

On Thu, Sep 25, 2014 at 1:21 PM, Dana Jansens <dan...@chromium.org> wrote:
> On Thu, Sep 25, 2014 at 4:18 PM, Jeremy Roman <jbr...@chromium.org> wrote:
>>
>> I don't understand how delegated constructors are any different in this
>> regard.
>>
>> With a delegated constructor, you still have to look at both the
>> initializing constructor (for what it initializes) and the called
>> constructor (for what it assigns).
>
>
> Constructors should all be neighbours in the .cc file, and you have an
> explicit call to another constructor to trace. Whereas member variables
> appear at the bottom of the .h file and there's nothing in the .cc file to
> say they exist or not.

On the other hand, you already have to check the .h file to know a
member's type, which is important in figuring out what its
constructor-initialization means.

"I ran into this in Java and hated it" is reasonably compelling. One
thing that might be different in C++ is that .h files are usually
shorter than .java files, so it might be easier to find the relevant
initialization in C++.

[Antoine Labour]

On Thu, Sep 25, 2014 at 11:35 AM, Jeremy Roman <jbr...@chromium.org> wrote:

On Thu, Sep 25, 2014 at 2:22 PM, Hendrik <hend...@chromium.org> wrote:
> I'm not proposing that we restrict anything, only that I prefer all
> intializations to be in one place.  I trust other developers to make
> decisions that make sense, I'd rather allow all C++11 features + a bit of
> education on the pitfalls.
>
> Not being able to initialize bit-fields is annoying.  In this case, I'm
> wondering why these are even bit-fields, this class does not get created
> thousands of times, and doesn't need to be space compact.
>
> Ignoring the bit-fields, was there a reason you didn't also initialize all
> of the other variables (e.g. |type| enum?), when I see that change I look at
> the header and wonder how many initialized variables we have.  I know this
> doesn't stop us from missing some (I fixed one today), but it's difficult to
> look at two sets of variables and see if the union of the set captures
> initialization for every variable.

tl;dr: I don't feel strongly about this, and the Google style guide
doesn't seem to comment on it.

In the case of the |type| enum, I wasn't convinced there one that was
a natural "zero".

My ultimate preference would be to have the *compiler* validate that
there are no uninitialized members (except where whitelisted),

Compilers don't do that (really unfortunately).

Antoine

 

and
then to specify as defaults only things which make are logically
default/zero. Then the compiler would force you to supply a reasonable
value for each constructor.

But in the absence of that I don't care which approach is taken. A
rule like Alex's SGTM.

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

[Julien Tinnes]

Could we try again to reach consensus on this?

I didn't see too strong an opposition. Given what the style guide already allows, I feel that delegated constructors are just a way to do things in a better / more concise way. I would love to see them allowed.

Julien

[Bruce]

I'm fixing a bug in pdfium where struct FX_SYSTEMTIME is sometimes used without being initialized (along error handling paths). The most universal fix is to add a constructor or in-class member initialization. There are eight member variables which should all be initialized to zero. Adding '= 0' to each member declaration is simple and obvious and foolproof. Doing the initialization in a constructor seems less obvious and would be more error prone if this was a struct that would ever be modified.

I guess I'm just saying that I'm in favor of allowing in-class member initialization. I'm going to fix this particular bug with initialization in a constructor just to avoid any delays but it would be nice to have in-class initialization as an option in the future:

This is the struct:

typedef unsigned short FX_WORD;

typedef struct _FX_SYSTEMTIME 

{

    FX_WORD wYear;

    FX_WORD wMonth;

    FX_WORD wDayOfWeek;

    FX_WORD wDay;

    FX_WORD wHour;

    FX_WORD wMinute;

    FX_WORD wSecond;

    FX_WORD wMilliseconds;

}FX_SYSTEMTIME;

I checked the code-gen of adding = 0 to each member and declaring a local. VC++ does four 32-bit writes with a zeroed EAX (5 instructions, 14 code bytes). g++ does two movq with immediate values (2 instructions, 16 code bytes). The cost is low enough that I don't think putting a default constructor in a .cc file is justified in this particular case.

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev+unsubscribe@chromium.org.

[Reid Kleckner]

Disclaimer: I work on Clang, not so much Chromium.

I would recommend allowing in-class initializers when the initializer is an integer constant expression.

Things get out of hand quickly if you allow other stuff in there. I expect compiler bugs and inconsistent behavior across platforms. I'm working on one such bug in Clang right now. =(

If you do choose to add a constructor later, the stuff in the initializer list will override whatever you put in the class body, and that seems pretty logical. This code does the obvious thing:

struct A {

  int a_ = 0;

  int b_ = 0;

  A() = default; // a_ is 0, b_ is 0

  explicit A(int a) : a_(a) {} // a_ is a, b_ is 0
};

Class types should have default constructors, so they aren't usually good candidates for in-class initialization anyway.

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.

[Alex Vakulenko]

I would stick with the official wording for this from the style guide: 

Use in-class member initialization for simple initializations, especially when a member variable must be initialized the same way in more than one constructor.

This makes perfect sense to me. I wouldn't limit this to "integer constant expression" either since I should be able to initialize bools to true/false and pointers to nullptr as long as the initialization is trivial.

Here is an example I would use this on:

class Method {
 public:
  enum class Kind { kSimple, kNormal, kAsync, kRaw };
  Method(const std::string& name_in,
         const std::vector<Argument>& input_arguments_in,
         const std::vector<Argument>& output_arguments_in)
      : name(name_in),
        input_arguments(input_arguments_in),
        output_arguments(output_arguments_in) {}
  Method(const std::string& name_in,
         const std::vector<Argument>& input_arguments_in)
      : name(name_in),
        input_arguments(input_arguments_in) {}
  explicit Method(const std::string& name_in) : name(name_in) {}

  std::string name;
  std::vector<Argument> input_arguments;
  std::vector<Argument> output_arguments;

  Kind kind = Kind::kNormal;
  bool is_const = false;
};

Here we have a Method class that has a few constructors dealing with initialization of name, in/out arguments of a method. But none of them actually touch properties like kind and is_const since they are rarely used. To me it makes perfect sense to split the members that can be initialized differently via constructors and the rest which can just be set to their defaults right in the class body...

Alex

[Hendrik]

Hi,

It sounds like everyone is mostly in agreement that this feature is useful in limited cases.

Perhaps with the some guidance, e.g.:

* Prefer not to use them when you need to initialize other members in the constructor (i.e. try to keep your initialization together)

* Prefer not to use them when the type being initialized is complex

Or something like that?

Any chance we can add this soon?  I'd like to use it :)

Thanks!

[Peter Kasting]

On Fri, Nov 7, 2014 at 12:12 PM, Hendrik <hend...@chromium.org> wrote:

Perhaps with the some guidance, e.g.:

* Prefer not to use them when you need to initialize other members in the constructor (i.e. try to keep your initialization together)

* Prefer not to use them when the type being initialized is complex

That guidance sounds vague enough that, as a reader, I would feel unsure of precisely what the rules of thumb are.  Ideally, we can make definitive statements (do x, don't do y), or not at all.

If someone is convinced they can state some useful rules in such a way, go for it; otherwise I'd propose we basically just let the Google style guide rules be the rules and rely on reviewer discretion.

PK

[Alex Vakulenko]

+1 on using Google style guide stance on this.

--

[Nico Weber]

I think we have enough for this week, so we should at least wait a bit.

I think tooling hasn't quite caught up with this language feature yet: If all constructors set a variable that also has a direct initializer, compilers don't seem to diagnose that the in-class initializer is ignored. I think we should implement a warning for this, but that doesn't block using the feature.

I checked that a POD with in-class initializers doesn't cause static initializers, so that's good.

[Chris Masone]

We just allowed delegated constructors, right? It seems like those can similarly be used to avoid the problem of creating a new member and only remembering to initialize it in one of multiple constructors, without scattering initialization across class declarations and class definitions. This also sidesteps the question of which initializations are "simple" and which aren't. Allowing this seems like optimizing for the writer of the code and not the reader of the code, especially since we just greenlighted a different mechanism for addressing the same concern.

[Peter Kasting]

On Fri, Nov 7, 2014 at 3:28 PM, Chris Masone <cma...@chromium.org> wrote:

We just allowed delegated constructors, right? It seems like those can similarly be used to avoid the problem of creating a new member and only remembering to initialize it in one of multiple constructors, without scattering initialization across class declarations and class definitions. This also sidesteps the question of which initializations are "simple" and which aren't. Allowing this seems like optimizing for the writer of the code and not the reader of the code, especially since we just greenlighted a different mechanism for addressing the same concern.

Can you restate this in the form of an argument about why Chromium code differs from Google code in a way that should lead us to differ from the Google style guide?

PK 

[Nico Weber]

On Fri, Nov 7, 2014 at 3:13 PM, Nico Weber <tha...@chromium.org> wrote:

I think we have enough for this week, so we should at least wait a bit.

I think tooling hasn't quite caught up with this language feature yet: If all constructors set a variable that also has a direct initializer, compilers don't seem to diagnose that the in-class initializer is ignored. I think we should implement a warning for this, but that doesn't block using the feature.

I checked that a POD with in-class initializers doesn't cause static initializers, so that's good.

(a global instance of such a POD, that is.)

[Ryo Hashimoto]

Replying to an old thread.

Can we revisit this now?

Is there anything preventing us from allowing this to let our style guide align with Google C++ style guide?

--
--
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.

[Vitaly Buka]

Is any blocking issue here?

[Daniel Cheng]

What sorts of bugs remain in various C++11 implementations wrt in-class member initiailization? Reid, do you mind expanding on what sorts of bugs you've seen in this area?

Daniel

On Tue Jan 20 2015 at 11:04:14 AM Vitaly Buka <vital...@chromium.org> wrote:

Is any blocking issue here?

On Mon Jan 12 2015 at 8:41:05 PM Ryo Hashimoto <hash...@chromium.org> wrote:

Replying to an old thread.

Can we revisit this now?

Is there anything preventing us from allowing this to let our style guide align with Google C++ style guide?

On Sat, Nov 8, 2014 at 8:30 AM, Nico Weber <tha...@chromium.org> wrote:

On Fri, Nov 7, 2014 at 3:13 PM, Nico Weber <tha...@chromium.org> wrote:

I think we have enough for this week, so we should at least wait a bit.

I think tooling hasn't quite caught up with this language feature yet: If all constructors set a variable that also has a direct initializer, compilers don't seem to diagnose that the in-class initializer is ignored. I think we should implement a warning for this, but that doesn't block using the feature.

I checked that a POD with in-class initializers doesn't cause static initializers, so that's good.

(a global instance of such a POD, that is.)

--
--
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+unsubscribe@chromium.org.

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

[Reid Kleckner]

Given the guidelines that Chromium is proposing to use, I don't think there are any bugs worth worrying about.

[Nico Weber]

Sorry for the slow response. While I'm personally not a fan of this feature for reasons others mentioned above (one more place to look for initialization; the same for static variables is very confusing to people and remains so in c++11 since storage must be declared somewhere and it can cause static initializers if one isn't careful; fairly low demand for the feature), I don't think there's any chromium-specific reason we shouldn't allow this. http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Initialization now has a rule similar to what was suggested upthread, so I think we should just allow this with a link to the google style guide for guidance.

hashimoto, can you send me a CL that moves this to the allowed section and that adds a use of the feature somewhere it makes sense, to check that all compilers are in fact happy with it?

[Ryo Hashimoto]

This is now allowed https://chromium-cpp.appspot.com/.