Has the C++17 extension to aggregate initialization made brace initialization dangerous?
Asked Answered
P

2

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 ???
}
Plasticizer answered 9/5, 2018 at 13:20 Comment(14)
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.Rapper
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.Tisman
@Max, what is new is that prior to C++17 the compiler would have caught these mistakes.Plasticizer
@Barnett: Who decides that it is a "mistake"? How would the compiler know that you didn't mean it?Kristiankristiansand
@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.Plasticizer
@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.Kristiankristiansand
@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.Plasticizer
@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.Kristiankristiansand
@Barnett: Please read about the 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.Kristiankristiansand
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.Plasticizer
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 cpp.sh/367lw.Piwowar
@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;.Kristiankristiansand
@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.Plasticizer
@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.Kristiankristiansand
K
38

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.

Kristiankristiansand answered 9/5, 2018 at 13:33 Comment(10)
@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.Kristiankristiansand
@Plasticizer 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.Solan
@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.Plasticizer
@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.Kristiankristiansand
@Plasticizer 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.Corrinnecorrival
@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".Plasticizer
@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.Kristiankristiansand
@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.Plasticizer
@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.Kristiankristiansand
@Plasticizer 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?Corrinnecorrival
F
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.

Fairlead answered 9/5, 2018 at 13:30 Comment(5)
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)?Plasticizer
@Plasticizer I don't understand the concern. Given void f(D); void f(E);, f({d}) calls the former, f({b}) is ambiguous.Fairlead
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).Plasticizer
@Plasticizer 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 ...Fairlead
Unfortunately Visual C++ gives no such warning, not even with /Wall.Plasticizer

© 2022 - 2024 — McMap. All rights reserved.