48

There seems to be a general consensus that brace initialization should be preferred over other forms of initialization, however since the introduction of the C++17 extension to aggregate initialization there seems to be a risk of unintended conversions. Consider the following code:

struct B { int i; };
struct D : B { char j; };
struct E : B { float k; };

void f( const D& d )
{
  E e1 = d;   // error C2440: 'initializing': cannot convert from 'D' to 'E'
  E e2( d );  // error C2440: 'initializing': cannot convert from 'D' to 'E'
  E e3{ d };  // OK in C++17 ???
}

struct F
{
  F( D d ) : e{ d } {}  // OK in C++17 ???
  E e;
};

In the code above struct D and struct E represent two completely unrelated types. So it is a surprise to me that as of C++17 you can "convert" from one type to another type without any warning if you use brace (aggregate) initialization.

What would you recommend to avoid these types of accidental conversions? Or am I missing something?

PS: The code above was tested in Clang, GCC and the latest VC++ - they are all the same.

Update: In response to the answer from Nicol. Consider a more practical example:

struct point { int x; int y; };
struct circle : point { int r; };
struct rectangle : point { int sx; int sy; };

void move( point& p );

void f( circle c )
{
  move( c ); // OK, makes sense
  rectangle r1( c );  // Error, as it should be
  rectangle r2{ c };  // OK ???
}

I can understand that you can view a circle as a point, because circle has point as base class, but the idea that you can silently convert from a circle to a rectangle, that to me is a problem.

Update 2: Because my poor choice of class name seems to be clouding the issue for some.

struct shape { int x; int y; };
struct circle : shape { int r; };
struct rectangle : shape { int sx; int sy; };

void move( shape& p );

void f( circle c )
{
  move( c ); // OK, makes sense
  rectangle r1( c );  // Error, as it should be
  rectangle r2{ c };  // OK ???
}
Barnett
  • 1,491
  • 12
  • 18
  • 5
    Looks like it's initializing the base subobject (D derives from B afterall). Clang also emits a warning `missing field 'k' initializer`. If it weren't, [this would be quite blaspheme](https://wandbox.org/permlink/onSSh1f6Hzvt7oOU). – Marco A. May 09 '18 at 13:42
  • 6
    The unintuitive behavior in your second example largely follows from circles and rectangles _being_ points. Fankly, it is an abuse of inheritance because rectangles are not points. If the base class was `ShapeWithCenter` then it would largely (but not fully) be obvious what is happening. Now, I agree that the difference between brace initialization and the ostensibly intended constructor is still dangerous, but that is not new. – Max Langhof May 09 '18 at 14:13
  • @Max, what is new is that prior to C++17 the compiler would have caught these mistakes. – Barnett May 09 '18 at 14:20
  • @Barnett: Who decides that it is a "mistake"? How would the compiler know that you didn't mean it? – Nicol Bolas May 09 '18 at 14:27
  • @Nicol, in my code `D` and `E` adds no new members to `B`. I could have used `B` everywhere, or I could have used `using D = B` and `using E = B`. If I did that then yes, it should be possible to initialize an `E` from a `D`. But I do NOT want this to be possible, so I am using the type system to create separate types. Before C++17 this was a good way of caching mistakes. But now it is not. – Barnett May 09 '18 at 14:41
  • 9
    @Barnett: "*But I do NOT want this to be possible, so I am using the type system to create separate types.*" Then use the type system to create *separate* types. Public inheritance is not a way to create separate types. It never has been. That something is a "mistake" to you doesn't mean that it is a "mistake" in a language sense. – Nicol Bolas May 09 '18 at 14:42
  • 1
    @Nicol, I am not the first person to use public inheritance. In his book "The C++ Programming Language" Bjarne Stroustrup derives `Manager` and `Assistant` from `Employee`. Same thing as my example. So if I am using the language wrong, at least I am in good company. I just don't think it should be possible to silently convert an `Assistant` into a `Manager`. – Barnett May 09 '18 at 15:40
  • 5
    @Barnett: I haven't seen the example you're talking about, but given the names, it makes *far more* sense that `Manager` and `Assistant` have an "is a" relationship to `Employee` than it does for `circle` and `rectangle` with `point`. Circles *have* points but they are not points; you cannot use them interchangeably with points. Whereas a manager cannot be a manager without also being an employee. I would also wager that none of Stroustrup's types are *aggregates*, so that wouldn't matter. – Nicol Bolas May 09 '18 at 15:46
  • 8
    @Barnett: Please read about the [Liskov Substitution Principle.](https://en.wikipedia.org/wiki/Liskov_substitution_principle) Managers and employees have it; circles and points do not. The problem is not that public inheritance exists; it's that you're using it inappropriately. – Nicol Bolas May 09 '18 at 15:48
  • I will have a look. But let's get back to the original question. C++17 introduced new functionality specifically relating to deriving from aggregates. The examples in the motivation section of that proposal is no different from what I have presented here. The problem remains. When you have more than one class deriving from the same aggregate, the one can be silently converted to the other. – Barnett May 09 '18 at 16:03
  • 1
    I think the conversion from D to B is a red-herring here; as @NicolBolas points out, we expect the language to allow this. What we don't expect is to be able to convert B to E. I suggest changing the example to have only two types, something like http://cpp.sh/367lw. – stewbasic May 09 '18 at 21:47
  • 2
    @Barnett: "*When you have more than one class deriving from the same aggregate, the one can be silently converted to the other.*" Conversion is this: `D d = e;`. That's not allowed. What you're doing is *initializing* an object; that's what a braced-init-list is for. `D d = {e};` is not supposed to be the same thing as `D d = e;`. – Nicol Bolas May 09 '18 at 21:56
  • @Nicol, that is exactly the problem. It has been my understanding that as of C++11 brace initialization is the safe option (eg no narrowing conversions). So when I call function `E f();`, I save the return value in a variable using `E e{ f() };` and not `E e = f();`. But as I illustrated, as of C++17, I can now write `D d{ f() }`, and the compiler will silently accept what is almost certainly an error. The same goes for `struct F { F(D d) : e{d} {} E e; };` I think most people will be very surprised that that now compiles. – Barnett May 10 '18 at 09:06
  • @Barnett: "*the compiler will silently accept what is almost certainly an error.*" Which you could have written in C++14 by using a member; all C++17 changed was that a base class was considered a member for purposes of aggregate initialization. The problem you're having is that you want list initialization to be something it isn't, to make promises that it's not trying to make. – Nicol Bolas May 10 '18 at 13:29

2 Answers2

37

struct D and struct E represent two completely unrelated types.

But they're not "completely unrelated" types. They both have the same base class type. This means that every D is implicitly convertible to a B. And therefore every D is a B. So doing E e{d}; is no different from E e{b}; in terms of the invoked operation.

You cannot turn off implicit conversion to base classes.

If this truly bothers you, the only solution is to prevent aggregate initialization by providing an appropriate constructor(s) that forwards the values to the members.

As for whether this makes aggregate initialization more dangerous, I don't think so. You could reproduce the above circumstances with these structs:

struct B { int i; };
struct D { B b; char j; operator B() {return b;} };
struct E { B b; float k; };

So something of this nature was always a possibility. I don't think that using implicit base class conversion makes it that much "worse".

A deeper question is why a user tried to initialize an E with a D to begin with.

the idea that you can silently convert from a circle to a rectangle, that to me is a problem.

You would have the same problem if you did this:

struct rectangle
{
  rectangle(point p);

  int sx; int sy;
  point p;
};

You can not only perform rectangle r{c}; but rectangle r(c).

Your problem is that you're using inheritance incorrectly. You're saying things about the relationship between circle, rectangle and point which you don't mean. And therefore, the compiler lets you do things you didn't mean to do.

If you had used containment instead of inheritance, this wouldn't be a problem:

struct point { int x; int y; };
struct circle { point center; int r; };
struct rectangle { point top_left; int sx; int sy; };

void move( point& p );

void f( circle c )
{
  move( c ); // Error, as it should, since a circle is not a point.
  rectangle r1( c );  // Error, as it should be
  rectangle r2{ c };  // Error, as it should be.
}

Either circle is always a point, or it is never a point. You're trying to make it a point sometimes and not others. That's logically incoherent. If you create logically incoherent types, then you can write logically incoherent code.


the idea that you can silently convert from a circle to a rectangle, that to me is a problem.

This brings up an important point. Conversion, strictly speaking, looks like this:

circle cr = ...
rectangle rect = cr;

That is ill-formed. When you do rectangle rect = {cr};, you're not doing a conversion. You are explicitly invoking list-initialization, which for an aggregate will usually provoke aggregate initialization.

Now, list-initialization certainly can perform a conversion. But given merely D d = {e};, one should not expect that this means you're performing conversion from an e to a D. You're list-initializing an object of type D with an e. That can perform conversion if E is convertible to D, but this initialization can still be valid if non-conversion list-initialization forms can work too.

So it is incorrect to say that this feature makes circle convertible to rectangle.

Nicol Bolas
  • 449,505
  • 63
  • 781
  • 982
  • 20
    @Barnett: Your new example is no better than the old one. Circles *have* points; rectangles *have* points, but they are not points in any real way. So why did you declare an "is a" relationship (ie: inheritance) where that relationship is not reflected in the types? The problem isn't the language; it's your use of it. – Nicol Bolas May 09 '18 at 14:25
  • @Barnett circle can be implicitly converted to point, point can be used to init paint base class of rectangle. The updated question still have no issue with this answer. – wooooooooosh May 09 '18 at 14:27
  • @Nicol, sorry I don't agree. If `circle` and `rectangle` have significant functionality in common, then it is perfectly acceptable (and desirable) to implement that common functionality in a shared base class. Please don't focus too much on these trivial examples. – Barnett May 09 '18 at 14:53
  • 8
    @Barnett: You asked why the language works the way it does. I explained the theoretical idea behind inheritance. You don't have to agree with it, but that's how the language see it, so that's how the language behaves. What you are trying to do is not something the language wants to support; that's why you're having problems making it work. – Nicol Bolas May 09 '18 at 15:05
  • 5
    @Barnett If you use the C++ object model you are constrained by the semantics of it. `struct A:B` doesn't mean "reuse `B`'s implementation", it means "an `A` is a `B` and can be used anywhere a `B` can be used". If you try to use it *without* that semantic meaning you can be burned. You have already stated that a circle is kind of point and a rectangle is kind of point. It is true that C++17 has now added a new aggregate construction method, but it leading to a problem here relies on your semantic error. – Yakk - Adam Nevraumont May 09 '18 at 18:21
  • @Yakk. I understand that `struct A : B` means that "`A` can be used anywhere `B` can be used", and that `struct C : B` means that "`C` can be used anywhere `B` can be used"... but I don't understand how you then get to "`A` can be used to initialize `C`, and vice versa". – Barnett May 09 '18 at 18:31
  • 4
    @Barnett: If "`B` can be used to initialize `C`" and "`A` can be used anywhere `B` can be used", then by the transitive property, "`A` can be used to initialize `C`". The problem is that you're trying to think of initialization from a single object as something distinct from initialization from multiple objects. – Nicol Bolas May 09 '18 at 18:38
  • @Nicol, I have a problem with the idea that "`B` can be used to initialize `C`"... When I see `struct C : B`, in my mind `C`>`B`, so `C` can be used to initialize `B`, but not the other way round. – Barnett May 09 '18 at 19:10
  • @Barnett: An aggregate can be initialized from a sequence of one or more of its subobjects, in declaration order. `B` is the first subobject of `C`, so `C` can be initialized from `B`. – Nicol Bolas May 09 '18 at 19:19
  • 3
    @Barnett To fix this, you need a *constructor*. If `C`>`B` write a constructor that takes something that isn't just a `B`. How exactly did you intent people to construct a `circle` before this change? Create an uninitialized one then set its values one at a time? – Yakk - Adam Nevraumont May 09 '18 at 20:26
29

This isn't new in C++17. Aggregate initialization always allowed you to leave off members (which would be initialized from an empty initializer list, C++11):

struct X {
    int i, j;
};

X x{42}; // ok in C++11

It's just now that there's more kinds of things that could be left off, since there's more kinds of things that can be included.


gcc and clang at least will provide a warning by way of -Wmissing-field-initializers (it's part of -Wextra) that will indicate that something is missing. If this is a huge concern, just compile with that warning enabled (and, possibly, upgraded to an error):

<source>: In function 'void f(const D&)':
<source>:9:11: warning: missing initializer for member 'E::k' [-Wmissing-field-initializers]
   E e3{ d };  // OK in C++17 ???
           ^
<source>: In constructor 'F::F(D)':
<source>:14:19: warning: missing initializer for member 'E::k' [-Wmissing-field-initializers]
   F( D d ) : e{ d } {}  // OK in C++17 ???
                   ^

More direct would be just to add a constructor to these types so that they cease to be aggregates. You don't have to use aggregate initialization, after all.

Barry
  • 286,269
  • 29
  • 621
  • 977
  • 1
    What if `class D` and `class E` does not add any new members, but are used as distinct types (eg for the purposes of overloading)? – Barnett May 09 '18 at 13:33
  • 1
    @Barnett I don't understand the concern. Given `void f(D); void f(E);`, `f({d})` calls the former, `f({b})` is ambiguous. – Barry May 09 '18 at 13:53
  • 1
    In my code I am using classes as a form of strong types. This allows me to overload functions based on type, etc... but more importantly, it protects me from accidentally initializing one unrelated type from another. Now, with the C++17 compiler, that protection is gone (if I continue to use brace initialization). – Barnett May 09 '18 at 14:18
  • 8
    @Barnett That protection is gone... unless you add a constructor. Or compile with warnings. Or not use brace initialization. Or have any member that isn't default-initialization. Or ... – Barry May 09 '18 at 14:22
  • Unfortunately Visual C++ gives no such warning, not even with `/Wall`. – Barnett May 10 '18 at 09:33