C++ Logo

std-proposals

Advanced search

Re: Allow block statement before each member in intializer list

From: Joshua Cannon <joshdcannon_at_[hidden]>
Date: Wed, 24 Jul 2019 08:56:54 -0500
(Forgot to reply-all)

The syntax will involve some bikeshedding, but I'm sure it can be made to
not conflict with the trailing-comma proposal (which I'm in the crowd that
is strongly for). I have a preference for curly braces, but I'd be happy to
concede to parenthesis if the committee steered it that way. I'm honestly
wondering if it can be made to "just work" grammatically where the main
body of the constructor can be logically thought of as a trailing
block-statement (which by position has access to all members).

I'll come back to the S2 considerations below, but I think the important
thing to note is that we can do these things today, but in horrible ways
that force assumptions we'd rather not make.

Good point on S3. My initial thought would be to treat each block-statement
as the "pre-initializer" for the member it precedes. This would mean it
would get compiled as if it were x(1), { puts("hello"); }, y(2), {
puts("world"); } z(3) (I guess the programmer got lucky that the
re-ordering kept the logical order of the 2 puts).

For S4, I think there would need to be some discussion on what other people
feel strongly about. I personally see the return as returning (possibly
early if we're in a branch) from the block-statement and continuing on with
initialization. If others dont' see it that way, we could disallow exiting
the scope due to a jump-statement (return/continue), but I think that makes
exiting branches more difficult.

Now on to the (poor) example I gave. I apologize for giving such a
lackluster example, especially given the weight of the proposed solution.
Why don't we beef it up a bit :)

*Example 2.0:*
(This is a lot closer to the real-world instance that made me think of this
proposal) Let's say we have a class Foo with 2 members.

   1. An immovable map-like type (ImmovableMapper) which takes some stuff
   in for construction, and needs to be populated with some data.
   2. A Bar which takes in iterators to the map to populate it's own
   internals.

So Foo looks like class Foo { ImmovableMapper myMapper; Bar bar; public:
Foo(...); };

The first non-ideal solution is to use a base class to "wrap" myMapper and
populate it in the base classes constructor.

namespace internal {
    struct FooBase {
        ImmovableMapper myMapper;
        FooBase(...): myMapper(...) {
            myMapper.add(1, 1);
            myMapper.add(2, 4);
            // ...
        }
    }
}
class Foo: private internal::FooBase {
    Bar bar;
public:
    Foo(...):
        FooBase(...),
        bar(this->myMapper.begin(), this->myMapper.end())
    {}
};

This is some pretty smelly code, since we introduce a new type for the sole
purpose of using it because we know temporally it'll be constructed first.
In a more complicated example (perhaps myMapper also has a dependency on
some initialized other memberm, etc..) this might not work. Additionally,
I'll point out some developers might not like the member in the base class,
and instead keep it in Foo, and then have the Base take in a non-const
reference to the map to initialize it. They will find however, that the
base will get initialized first, and then have a bad time.

The second non-ideal solution gets us closer to what we want, but may
introduce a subtle bug that I guarantee new-to-moderately-experienced
programmers won't be able to find. (Note that using the comma operator
shouldn't be well-known by those programmers, but StackOverflow
<https://stackoverflow.com/a/13314512> is happy to teach them how to aim
the gun at their foot).

class Foo {
    ImmovableMapper myMapper;
    Bar bar;

    void populateMyMapper(){
        // NOTE: DO NOT access any member that comes after myMapper
        // it won't be initialized
        myMapper.add(1, 1);
        myMapper.add(2, 4);
        // ...
    }

public:
    Foo(...):
        myMapper(...),
        bar((populateMyMapper(), myMapper.begin()), myMapper.end())

        // or is it
        // bar(myMapper.begin(), (populateMyMapper(), myMapper.end()))
    {}
};

The comments are pretty self-highlighting as to why this approach is
frightening. But at the end of the day, it *is* a lot closer to what we
want to express.

Lastly, the closest approach we have, but is still non-ideal.

class Foo {
    ImmovableMapper myMapper;
    [[no_unique_address]] bool justToPopulateMyMapper;
    Bar bar;

    void populateMyMapper(){
        // NOTE: DO NOT access any member that comes after myMapper
        // it won't be initialized
        myMapper.add(1, 1);
        myMapper.add(2, 4);
        // ...
    }

public:
    Foo(...):
        myMapper(...),
        justToPopulateMyMapper(populateMyMapper(), true),
        bar(myMapper.begin(), myMapper.end())
    {}
};

This is what we do today, which avoids the problem of argument
evaluation order, but still has a pit just waiting for someone to step in
it.

Now back to S2. Anything example of bad code we can put in the new
block-statement-in-the-initializer-list could just as well be put in a
method that gets invoked in between member initializers. The difference is
that bad code is much easier to spot as it is sitting in between the
members-that-are-usable and the members-that-arent-yet-usable. When we
shove this code in a method, *at best* we put a comment and hope and pray
future refactors won't mess it up (perhaps even a static analyzer might
help us, if we're lucky enough to run one). Alternatively we can very
clearly see what we're doing is wrong, (and in this case, I bet the
compiler has a much better chance of warning as well since code locality
has improved).

Hope that helps explain the use-case (and the crappy alternatives) a lot
better.

>

Received on 2019-07-24 08:59:02