std::initializer_list and good modern C++ style

[Matt Newport]

I raised an issue against GSL array_view recently (which is forming the basis for a propsal for an std::array_view) which led to a side discussion about when it is appropriate to use std::initializer_list and braced init lists. In that discussion Gaby Dos Reis said "any use of std::initializer_list in context other than constructing a container, in particular an array_view should be suspicious to a trained eye.". The issue comments weren't deemed the right place to continue discussion of that topic but I'm still left confused about why it should be. It doesn't seem to be true to me but if there's a good reason I'm not seeing I'd like to understand it.

I'm going to be pasting a bunch of code snippets below, a live version of the full example they come from can be found here.

So as of C++11 all the following range based for loops work and as far as I am aware are all perfectly safe:

    constexpr int intArray[] = {1, 2, 3};

    const auto intStdArray = array<int, 3>{4, 5, 6};

    const auto intVector = vector<int>{7, 8, 9};

    const auto intList = list<int>{10, 11, 12};

    const auto intInitList = {13, 14, 15};

    auto makeVector = [] { return vector<int>{16, 17, 18}; };

    for (auto i : intArray) cout << i << ", ";

    cout << '\n';

    for (auto i : intStdArray) cout << i << ", ";

    cout << '\n';

    for (auto i : intVector) cout << i << ", ";

    cout << '\n';

    for (auto i : intInitList) cout << i << ", ";

    cout << '\n';

    for (auto i : makeVector()) cout << i << ", ";

    cout << '\n';

    for (auto i : vector<int>{19, 20, 21}) cout << i << ", ";

    cout << '\n';

    for (auto i : {21, 22, 23}) cout << i << ", ";

    cout << '\n';

The version that constructs an explicit temporary vector<int>{19, 20, 21} is less efficient and more verbose than the version that uses a braced init list {21, 22, 23} directly. In the case that you know the items you want to iterate over but don't have an existing container to hand the direct braced init list seems to me the most desirable version of the code.

Now say I want to factor this code out into a function. Perhaps my first attempt might be:

void printIntsVector(const vector<int>& v) {

    for (auto i : v) cout << i << ", ";

    cout << '\n';

}

    printIntsVector(intVector);

    printIntsVector(intInitList);

    printIntsVector(makeVector());

    printIntsVector(vector<int>{19, 20, 21});

    printIntsVector({21, 22, 23}); // Implicitly creates a temporary vector and allocates memory

    // doesn't compile

    // printIntsVector(intArray);

    // printIntsVector(intStdArray);

    // printIntsVector(intList);

This isn't a great solution since it doesn't handle all the containers I might want to use and in the case that I pass a braced init list directly it implicitly constructs a temporary vector which likely allocates memory.

A second attempt might be:

template <typename Cont = std::initializer_list<int>>

void printIntsContainer1(const Cont& c) {

    for (auto i : c) cout << i << ", ";

    cout << '\n';

}

    printIntsContainer1(intArray);

    printIntsContainer1(intStdArray);

    printIntsContainer1(intVector);

    printIntsContainer1(intList);

    printIntsContainer1(intInitList);

    printIntsContainer1(makeVector());

    printIntsContainer1(vector<int>{19, 20, 21});

    printIntsContainer1({21, 22, 23});

This is better in my eyes: it works with all the containers that worked before factoring the code out into a function and in the case of directly passing a braced init list it doesn't create a temporary vector. If I understand Gaby's position correctly though, this code should be considered suspicious since it uses an std::initializer_list in a context other than constructing a container. If this is actually to be considered bad style I'd like to understand why.

Perhaps we want to generalize this code further though and not restrict it to containers of ints. One possible way to achieve that is:

template <typename Cont>

void printContainer2(const Cont& c) {

    printContainer1(c);

}

template <typename T>

void printContainer2(std::initializer_list<T> il) {

    printContainer1(il);

}

This still works with all the previous cases but now additionally allows code like this to compile:

    printContainer2(vector<const char*>{"one", "two", "three"});

    printContainer2({"one", "two", "three"});

Which is nice to have. Now this version seems to be crying out to take some kind of generic Range argument but hopefully that's something we'll get down the line with Eric Niebler's Range work. Again, this seems to be considered bad style by Gaby for reasons that are not clear to me.

Now we get closer to the original motivating example that caused me to raise the array_view issue: interoperating with legacy APIs that expect contiguous arrays of elements. Say I'd like to be able to call a function like bsearch with any container guaranteeing contiguous storage. It seems to me like it would be good to have an array_view like class that would help with this:

template <typename T>

class ArrayView {

public:

    ArrayView(initializer_list<T> il) : first{std::begin(il)}, last{std::end(il)} {}

    template <typename Cont>

    ArrayView(Cont& c) : first{std::data(c)}, last{first + std::size(c)} {}

    auto begin() const { return first; }

    auto end() const { return last; }

    auto size() const { return last - first; }

private:

    const T* first;

    const T* last;

};

void printIntArrayView(ArrayView<int> av) {

    for (auto e : av) cout << e << ", ";

    cout << '\n';

}

void wrapLegacyApi(ArrayView<int> av, int searchVal) {

    auto comp = [](const void* x, const void* y) {

        auto toInt = [](const void* x) {

            int res;

            memcpy(&res, x, sizeof(res));

            return res;

        };

        return toInt(x) < toInt(y) ? -1 : toInt(x) == toInt(y) ? 0 : 1;

    };

    const auto findIt = reinterpret_cast<const int*>(

        bsearch(&searchVal, begin(av), size(av), sizeof(*begin(av)), comp));

    if (findIt) cout << "Found " << searchVal << " at position " << (findIt - begin(av)) << '\n';

    else cout << "Did not find " << searchVal << '\n';

}

    wrapLegacyApi(intArray, 2);

    wrapLegacyApi(intStdArray, 2);

    wrapLegacyApi(intVector, 7);

    // These are all forbidden by GSL array_view

    wrapLegacyApi(intInitList, 14);

    wrapLegacyApi(makeVector(), 18); 

    wrapLegacyApi(vector<int>{19, 20, 21}, 21);

    wrapLegacyApi({21, 22, 23}, 22);

This is where GSL array_view takes a different view and forbids construction of a GSL array_view from a temporary (by deleting the constructors that take a Container&&) or from an std::initializer_list (even a non-temporary). I understand the reasoning behind that decision since broken code like this would compile:

    auto badAv1 = array_view<const int>{{1, 2, 3}};

    // dangling reference to temporary initializer list

    auto badAv2 = array_view<const int>{makeVector()};

    // dangling reference to temporary vector

But I don't see how that makes it suspicious to use temporary initializer lists in general. I'm interested to know if there are hidden downsides of using std::initializer_list in contexts other than the argument to a container constructor that I'm not seeing, or if there are other reasons any of the above should be considered bad modern C++ style that I'm not aware of.

Thanks,

Matt.

[Nicol Bolas]

On Saturday, October 17, 2015 at 6:42:44 PM UTC-4, Matt Newport wrote:

I raised an issue against GSL array_view recently (which is forming the basis for a propsal for an std::array_view) which led to a side discussion about when it is appropriate to use std::initializer_list and braced init lists. In that discussion Gaby Dos Reis said "any use of std::initializer_list in context other than constructing a container, in particular an array_view should be suspicious to a trained eye.". The issue comments weren't deemed the right place to continue discussion of that topic but I'm still left confused about why it should be. It doesn't seem to be true to me but if there's a good reason I'm not seeing I'd like to understand it.

It seems pretty obvious.

`initializer_list` is a non-modifiable range of elements. The storage for that range is created by the compiler and has a lifetime that is non-obvious to C++ programmers. Or better yet, C++ programmers should not have to concern themselves with the lifetime of an `initializer_list`'s storage.

If you restrict your usage of `initializer_list` to constructors of containers, then you will never have problems with such lifetime issues. This naturally includes functions that forward items to said constructors. So long as the destination within the callstack of the function is some kind of constructor, you'll be fine.

Also, you might want to notice the name of the type: initializer_list. There's a reason it starts with that word: because it's for initializing something. Similarly, the standard term "braced-init-list" is so named because it's for initializing things.

Braced-init-lists were added to the standard to allow initializing objects. They exist to make it possible for non-array-aggregates to use sequential initialization that looks like array aggregate initialization.

They were not added so that you could write temporary arrays inline. And allowing `array_view` to implicitly convert from them would be encouraging that kind of coding.

In short: stop trying to use `initializer_list` like it's a compact way of writing a temporary array. It's for initializing objects, not for making temporary arrays.

I can understand your concern about the current proposal explicitly disallowing creating `array_view` from a temporary (via deleting the `Container&&` constructors). I'd rather that such constructors just be made explicit. But either way, constructing a view of something that is always temporary like `initializer_list`, is not a good thing.

It doesn't matter that you can write code where it is safe. Just because you can doesn't mean that you should.

This is where GSL array_view takes a different view and forbids construction of a GSL array_view from a temporary (by deleting the constructors that take a Container&&) or from an std::initializer_list (even a non-temporary). I understand the reasoning behind that decision since broken code like this would compile:

    auto badAv1 = array_view<const int>{{1, 2, 3}};

    // dangling reference to temporary initializer list

    auto badAv2 = array_view<const int>{makeVector()};

    // dangling reference to temporary vector

But I don't see how that makes it suspicious to use temporary initializer lists in general.

You just explained exactly how it makes it suspicious.

Do you want to encourage the writing of code that works, so long as you follow all of these rules that nobody is going to check for you? Or do you want to encourage the writing of code that, if it compiles, will always work?

`array_view`, as proposed, focuses on the latter. As does the GSL in general.

C++ isn't a safe language, that is true. But that doesn't justify us intentionally punching holes in it, just to make the syntax a bit more convenient.

[Sam Kellett]

On 18 October 2015 at 15:48, Nicol Bolas <jmck...@gmail.com> wrote:

In short: stop trying to use `initializer_list` like it's a compact way of writing a temporary array. It's for initializing objects, not for making temporary arrays.

does this include using initializer_list in for-range loops?

for (auto i : {1, 3, 5, 7}) {

  std::cout << i;

}

am i right in thinking that this is using initializer_list's std::begin/end and not constructing a vector and using it's std::begin/end overloads?

I can understand your concern about the current proposal explicitly disallowing creating `array_view` from a temporary (via deleting the `Container&&` constructors). I'd rather that such constructors just be made explicit. But either way, constructing a view of something that is always temporary like `initializer_list`, is not a good thing.

doesn't string_view have a std::string&& constructor? pretty sure i saw a thread about that the other day but can't find it now...

[Nicol Bolas]

On Sunday, October 18, 2015 at 2:24:46 PM UTC-4, Sam Kellett wrote:

On 18 October 2015 at 15:48, Nicol Bolas <jmck...@gmail.com> wrote:

In short: stop trying to use `initializer_list` like it's a compact way of writing a temporary array. It's for initializing objects, not for making temporary arrays.

does this include using initializer_list in for-range loops?

for (auto i : {1, 3, 5, 7}) {

  std::cout << i;

}

am i right in thinking that this is using initializer_list's std::begin/end and not constructing a vector and using it's std::begin/end overloads?

Yes, that's true.

The issue is not that the feature cannot possibly ever work. It's that promoting such use will inevitably lead unwary users to use it incorrectly.

I can understand your concern about the current proposal explicitly disallowing creating `array_view` from a temporary (via deleting the `Container&&` constructors). I'd rather that such constructors just be made explicit. But either way, constructing a view of something that is always temporary like `initializer_list`, is not a good thing.

doesn't string_view have a std::string&& constructor? pretty sure i saw a thread about that the other day but can't find it now...

I don't know about a thread, but library fundamentals v1 doesn't have one. It only has a `const&` one.

[Sam Kellett]

On 18 October 2015 at 20:56, Nicol Bolas <jmck...@gmail.com> wrote:

On Sunday, October 18, 2015 at 2:24:46 PM UTC-4, Sam Kellett wrote:

On 18 October 2015 at 15:48, Nicol Bolas <jmck...@gmail.com> wrote:

In short: stop trying to use `initializer_list` like it's a compact way of writing a temporary array. It's for initializing objects, not for making temporary arrays.

does this include using initializer_list in for-range loops?

for (auto i : {1, 3, 5, 7}) {

  std::cout << i;

}

am i right in thinking that this is using initializer_list's std::begin/end and not constructing a vector and using it's std::begin/end overloads?

Yes, that's true.

The issue is not that the feature cannot possibly ever work. It's that promoting such use will inevitably lead unwary users to use it incorrectly. 

i know it's not official, but cppreference appears to promote this use of initializer_list: http://en.cppreference.com/w/cpp/utility/initializer_list/begin2

i admit this is the first time i've read that we shouldn't be using it for for loops like the one above...

 

I can understand your concern about the current proposal explicitly disallowing creating `array_view` from a temporary (via deleting the `Container&&` constructors). I'd rather that such constructors just be made explicit. But either way, constructing a view of something that is always temporary like `initializer_list`, is not a good thing.

doesn't string_view have a std::string&& constructor? pretty sure i saw a thread about that the other day but can't find it now...

I don't know about a thread, but library fundamentals v1 doesn't have one. It only has a `const&` one.

so weird, just had another search for it and came up with nothing. i must be having some crazy dreams

i think the reasoning behind it was so a function returning a string could be passed as a parameter expecting a string_view:
 

  void foo(const std::string_view &);

  std::string bar();

  foo(bar());
 

whether or not this is a good idea i'm not sure -- although if it is, does that mean it would be for array_view too? personally, i'm not entirely convinced the convenience of this is worth making this legal:

  std::string_view s = std::string{"hello"};

but like you said this constructor doesn't appear to be there anyway!

[Sam Kellett]

On 18 October 2015 at 21:12, Sam Kellett <samke...@gmail.com> wrote:

so weird, just had another search for it and came up with nothing. i must be having some crazy dreams

i think the reasoning behind it was so a function returning a string could be passed as a parameter expecting a string_view:
 

  void foo(const std::string_view &);

  std::string bar();

  foo(bar());
 

whether or not this is a good idea i'm not sure -- although if it is, does that mean it would be for array_view too? personally, i'm not entirely convinced the convenience of this is worth making this legal:

  std::string_view s = std::string{"hello"};

but like you said this constructor doesn't appear to be there anyway!

ah i found it!

it was here: https://www.reddit.com/r/cpp/comments/3owd3d/cppcon_2015_marshall_clow_string_view/cw15ih4

[Thiago Macieira]

On Sunday 18 October 2015 19:24:44 Sam Kellett wrote:
> does this include using initializer_list in for-range loops?
>
> for (auto i : {1, 3, 5, 7}) {
> std::cout << i;
> }

This is as dangerous as

auto makeVector = [] { return vector<int>{16, 17, 18}; };

for (auto i : makeVector())
cout << i << ", ";

Both are iterating over a dangling temporary.

--
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
Software Architect - Intel Open Source Technology Center
PGP/GPG: 0x6EF45358; fingerprint:
E067 918B B660 DBD1 105C 966C 33F5 F005 6EF4 5358

[Gabriel Dos Reis]

Sam Kellett <samke...@gmail.com> writes:

| On 18 October 2015 at 15:48, Nicol Bolas <jmck...@gmail.com> wrote:
|
| In short: stop trying to use `initializer_list` like it's a
| compact way of writing a temporary array. It's for initializing
| objects, not for making temporary arrays.
|
|
| does this include using initializer_list in for-range loops?
|
| for (auto i : {1, 3, 5, 7}) {
|   std::cout << i;
| }

No, it does not -- as I explicitly pointed out in the issue linked in
Matt's original message. And, it you did explicitly write out
std::initializer_list<int> there, many would find it suspicious -- is
the author unfamiliar with the whole concept? Or is there something
deeper going on?

As I explained, many times, the issue has to do with *lifetime* and
assumptions a good and safe interface can make.

Nicol was right in the message you were replying to.

It is amusing that the use-cases presented in the original message
attempt to pretend (both in the function names, and in the template
parameter) that std::initializer_list<T> is a container, when it clearly
isn't. That should ring an alarm bell.

-- Gaby

[Matt Newport]

On Sunday, 18 October 2015 07:48:10 UTC-7, Nicol Bolas wrote:

It seems pretty obvious.

`initializer_list` is a non-modifiable range of elements. The storage for that range is created by the compiler and has a lifetime that is non-obvious to C++ programmers. Or better yet, C++ programmers should not have to concern themselves with the lifetime of an `initializer_list`'s storage.

If you restrict your usage of `initializer_list` to constructors of containers, then you will never have problems with such lifetime issues. This naturally includes functions that forward items to said constructors. So long as the destination within the callstack of the function is some kind of constructor, you'll be fine.

Gaby made a similar claim and referred me to the original paper  N2215 on the motivation for initializer lists being added to the language. That document however has this section (emphasis mine):

4.4 Syntax
In the EWG there were strong support for the idea of the sequence constructor, but initially no consensus about the syntax needed to express it. There was a strong preference for syntax to make the “special” nature of a sequence constructor explicit. 
...
Based on extensive discussions, we prefer the X(initializer_list) design, because this “special compiler-recognized class name” approach
...

• The initializer_list type can be used for any argument that can accept an initializer list. For example int f(int, initializer_list, int) can accept calls such as f(1, {2,3}, 4). This eliminates the need for variable argument lists (… arguments) in many (most?) places.
  

Which appears to me to support using intializer lists as general function arguments. Anyway, regardless of the original intent of the design of a language feature I don't think it is especially significant to discussions of appropriate uses to which it can be put once it is part of the language. Templates are used for many things that were not part of the original motivation for introducing them but that doesn't mean all of those uses are automatically to be considered bad.

Now std::initializer_list is not a normal container type and I think it rarely if ever makes sense as a local variable and I can't think of a good use for one as a member variable but if it is a good parameter type for a container constructor I don't see the logic for saying it is a bad parameter type for a function if the function intends to access the contents inside the body for some purpose other than container initialization and never reference the contents after the function returns. Semantically, syntactically and safety wise how are those uses different?

Creating temporary arrays inline may not have been the primary motivation for initializer lists (although it still appears to me that the quote above supports that use as legitimate) but I still haven't seen any real reason for why such a use is bad. Specifically, if it's ok to pass a temporary vector to a function taking a const vector<T>&, why should it not be ok to allow passing an inline initializer_list directly without the overhead of allocating a temporary vector? Lifetime wise both are perfectly safe providing the person writing the function doesn't take a reference to the argument that will outlive returning from the function and equally broken if they do.

Now I do acknowledge that constructing an array_view from any temporary (initializer list or container) is dangerous but as I said in the discussion on the issue, array_view is a fundamentally unsafe class and it is already proposed that there be static analysis support to catch unsafe uses that cannot be prevented at the library API level. If we get static analysis that can catch this:

auto v = makeVector();

auto av = array_view<int>{v};

v.push_back(4);

// oops, potential dangling reference

Then it should be relatively easy to catch this:

auto av = array_view<int>{makeVector()};

// oops, definite dangling reference

So why allow the first and not the second? In particular, writing code like this makes the push_back bug impossible:

void f(array_view<int>);

f(makeVector());

Which is one of the reasons I think that is strictly better style than 

auto v = makeVector();

auto av = array_view<int>{v};

f(av);

v.push_back();

// oops, better not reference av after this point

[Matt Newport]

They are both as dangerous because they are both not dangerous. There is no dangling temporary here. The lifetime of the temporary is clearly specified to encompass the entirety of the loop body.

[Thiago Macieira]

On Sunday 18 October 2015 18:21:47 Matt Newport wrote:
> They are both as dangerous because they are both not dangerous. There is no
> dangling temporary here. The lifetime of the temporary is clearly specified
> to encompass the entirety of the loop body.

Hmm... my bad, the temporary is life-time-extended for the lambda. I always
get confused about when things get extended, so as a rule of thumb, I *never*
write any rvalue / xvalue in the right side of the : in a range-based for.

[Matt Newport]

I know we're all seasoned C++ programmers who've been burned by lifetime issues enough times that we are cautious of them but sometimes it seems like there's a little too much paranoia around things that fortunately are well specified as quite safe. It would be a sorry state of affairs if f(g()) worked for void f(int) and int g() but not for f(const int&) or f(const vector<int>&) and vector<int> g(). Fortunately we don't have to worry about such things.

Similarly it would be a sad state of affairs if for(auto x : makeVector()) { ... } was not safe code and we had to try and explain why to novice C++ developers but happily it is also perfectly safe.

Equally I can't think of a situation where the caller needs to worry about lifetimes in something like f({1, 2, 3}) whether we have void f(initializer_list<int>) or void f(const vector<int>&). Meanwhile the author of f() just has to not rely on the lifetime of his argument extending beyond the return from the function, which doesn't seem anything to be considered terribly confusing to understand.

[Nicol Bolas]

On Sunday, October 18, 2015 at 10:11:31 PM UTC-4, Matt Newport wrote:

I know we're all seasoned C++ programmers who've been burned by lifetime issues enough times that we are cautious of them but sometimes it seems like there's a little too much paranoia around things that fortunately are well specified as quite safe.

I think you're equating things that aren't equal.

Lifetime extension for temporaries bound to references is one thing. Temporaries are real objects, easily visible in the code. You can see that they're being bound to a temporary (or that binding is hidden in code transformation like range-based for). Overall, it's clear what's happening.

Initializer lists aren't like that. The object itself is invisible, its definition unknown and unseen. The object can not be manipulated or treated like a real object. You can only talk about it through a reference. And that reference doesn't even look like a reference.

When you do something wrong with references, like returning a reference to a local from a function... you have to actually return one. You need to put "&" in the return type, thus making it abundantly clear that you're returning a reference. And thus at least potentially doing something wrong.

Returning an `initializer_list<int>` looks exactly like returning a `vector<int>`. And yet, it is almost always undefined behavior.

The other element you're falsely equating is the gain.

Relying on the lifetime extension of temporaries is sometimes absolutely unavoidable, with range-based for being a prime example. That's something a C++ programmer simply needs to know works in order to pass values around.

What good does learning the rules of `initializer_list` do? What's the point of having parameters of type `initializer_list`?

It allows some syntactic sugar on the user's side.

That's it. It isn't a need. It isn't a case of "I can't really do this without it". It's merely nicer syntax. Slightly.

I'd rather people use what is obviously correct than have them memorize and follow a bunch of complex rules. Like "it's safe to use `initializer_list` in parameters and locals, but not in return values or NSDMs." Or whatever.

C++ has hundreds of rules like this. We shouldn't be encouraging people to learn such esoteric minutiae. Good coding practice should minimize such rules. A simple rule like "don't use initailizer_lists outside of eventually 'initializing' a 'list'," is much easier to follow.

[Nicol Bolas]

On Sunday, October 18, 2015 at 9:13:56 PM UTC-4, Matt Newport wrote:

On Sunday, 18 October 2015 07:48:10 UTC-7, Nicol Bolas wrote:

It seems pretty obvious.

`initializer_list` is a non-modifiable range of elements. The storage for that range is created by the compiler and has a lifetime that is non-obvious to C++ programmers. Or better yet, C++ programmers should not have to concern themselves with the lifetime of an `initializer_list`'s storage.

If you restrict your usage of `initializer_list` to constructors of containers, then you will never have problems with such lifetime issues. This naturally includes functions that forward items to said constructors. So long as the destination within the callstack of the function is some kind of constructor, you'll be fine.

Gaby made a similar claim and referred me to the original paper  N2215 on the motivation for initializer lists being added to the language. That document however has this section (emphasis mine):

4.4 Syntax
In the EWG there were strong support for the idea of the sequence constructor, but initially no consensus about the syntax needed to express it. There was a strong preference for syntax to make the “special” nature of a sequence constructor explicit. 
...
Based on extensive discussions, we prefer the X(initializer_list) design, because this “special compiler-recognized class name” approach
...

• The initializer_list type can be used for any argument that can accept an initializer list. For example int f(int, initializer_list, int) can accept calls such as f(1, {2,3}, 4). This eliminates the need for variable argument lists (… arguments) in many (most?) places.
  

Which appears to me to support using intializer lists as general function arguments.

... Did you read the same paragraph that I did? Like the part about "eliminates the need for variable argument lists (… arguments) in many (most?) places?"

Because that totally did not happen. You do not see a rash of C++ programmers abandoning template parameter packs in favor of initializer lists. And really, why would they?

In short, sometimes people, even on the standards committee, say things that don't work out. Sometimes they say things, then change their mind later based on actual practice.

Oh, and that statement doesn't change the fact that they named initializer_list "initializer_list". If they wanted a generic value list, they could have called it `value_list`. So it seems clear that they had a goal in mind for the type.

Anyway, regardless of the original intent of the design of a language feature I don't think it is especially significant to discussions of appropriate uses to which it can be put once it is part of the language.

Thinking like this is what's going to lead a bunch of people to use `await` on `optional` and `expected` objects, when they're not actually "waiting" on values.

That's not to say that original intent is the end-all-be-all. But to go against the very name of a thing is a very slippery slope. You don't want to encourage people to use `std::string` when processing a very-non-stringlike array of bytes (`vector<uint8_t>`).

Templates are used for many things that were not part of the original motivation for introducing them but that doesn't mean all of those uses are automatically to be considered bad.

Now std::initializer_list is not a normal container type

Stop. It's not a container type of any kind, normal or otherwise. It's a view. The actual container is a compiler-generated temporary. The initializer_list is just a view associated with it.

and I think it rarely if ever makes sense as a local variable and I can't think of a good use for one as a member variable but if it is a good parameter type for a container constructor I don't see the logic for saying it is a bad parameter type for a function if the function intends to access the contents inside the body for some purpose other than container initialization and never reference the contents after the function returns. Semantically, syntactically and safety wise how are those uses different?

Just look at that big list of rules you spat out:

1) Don't use initializer_list in member variables.
2) Don't use initializer_list as a container.
3) Don't use initializer_list as local variables.
4) Don't use initializer_list as return types.
5) But function parameters are OK. As long as you aren't storing pointers/references to the values.

Wouldn't it be so much easier to just say:

1) Use initializer_list to initialize a container with elements. Or to forward to code initializing a container with elements.

You don't have to remember 5 do's and dont's. It's just one rule. One rule is a lot easier to get right than 5.
 

It should also be noted that rule #1 about member variables? Well, `array_view` has member variables. And its constructor initializes them. Which means that any constructor from a compatible initializer list would... have to effectively initialize the array view with the initializer list.

So you're basically asking to violate rule #1.

Now I do acknowledge that constructing an array_view from any temporary (initializer list or container) is dangerous but as I said in the discussion on the issue, array_view is a fundamentally unsafe class and it is already proposed that there be static analysis support to catch unsafe uses that cannot be prevented at the library API level. If we get static analysis that can catch this:

auto v = makeVector();

auto av = array_view<int>{v};

v.push_back(4);

// oops, potential dangling reference

Then it should be relatively easy to catch this:

auto av = array_view<int>{makeVector()};

// oops, definite dangling reference

So why allow the first and not the second?

That's a rather silly question.

Whether that `push_back` operation causes a dangling reference depends on two factors both being true:

1) `av` is used after the `push_back`

2) The `push_back` caused reallocation

Unless reallocation actually happened, this code is fine. The user may well have reserved sufficient space. Or maybe the user checked the capacity before doing the push_back. In any case, local static analysis cannot possibly be sufficient to prove that this code is broken.

By contrast, in your second case any use of `av` after its creation is always undefined behavior and obviously so. Static analysis wouldn't take long to find that dangling reference.

We forbid #2 in syntax because we can forbid it. There's no way to syntactically forbid #1, even if it weren't perfectly legitimate in some cases.

Yes, `array_view` is not safe. That should not be an excuse to just let it do whatever.

In particular, writing code like this makes the push_back bug impossible:

void f(array_view<int>);

f(makeVector());

Which is one of the reasons I think that is strictly better style than 

auto v = makeVector();

auto av = array_view<int>{v};

f(av);

v.push_back();

// oops, better not reference av after this point

Or you could just do this:

auto v = makeVector();
f(v);
v.push_back();

Store your containers in variables; it's the views of them that should be created as needed, as temporaries.

`array_view`'s constructors aren't explicit for a reason.

That being said, while I'm against having `array_view` have an implicit constructor from these things, I wouldn't be against allowing `as_array_view` or having to use an explicit constructor for them. The point here is that it's making it clear when you're doing something dangerous.

But really, worse-comes-to-worse, you can just define a quickie function of your own:

auto adapt(initializer_list<T> list) {return array_view<T>{list.begin(), list.size()};}

Similar code could be written for adapting rvalue references.

[Matt Newport]

On Sunday, 18 October 2015 19:36:55 UTC-7, Nicol Bolas wrote:

I think you're equating things that aren't equal.

I don't think I am. If I'm reading or reviewing some code I don't see any safety / correctness reason to consider any of these particularly suspicious:

const auto v = vector<int>{1, 2, 3}; // nobody argues with this use of an initializer list right?

f(v);

f(makeVector());

f(vector<int>{1, 2, 3});

f({1, 2, 3}); // possible temporary vector or other container created?

for (auto x : v) { ... }

for (auto x : {1, 2, 3}) { ... }

And I don't even need to know the declaration of f() to say that. It could be void f(const vector<int>&) or something like one of the more flexible variants I posted above and if I can assume (reasonably) that it is not doing something broken for all these uses like holding on to a reference to the passed in argument then I can assume that all of these are perfectly safe. If I'm looking out for performance issues, I might wonder whether the version being passed a plain initializer list allocates a temporary vector and whether that is necessary but safety / correctness wise I don't see any potential issues on the calling side with any of these.

 

Lifetime extension for temporaries bound to references is one thing. Temporaries are real objects, easily visible in the code. You can see that they're being bound to a temporary (or that binding is hidden in code transformation like range-based for). Overall, it's clear what's happening.

Initializer lists aren't like that. The object itself is invisible, its definition unknown and unseen. The object can not be manipulated or treated like a real object. You can only talk about it through a reference. And that reference doesn't even look like a reference.

For the use case here (passing a braced initalizer list as an argument to a function) I still don't see the issue. It's fine for the same reason passing it as an argument to a container constructor is fine (they're essentially the same thing after all). There's nothing weird or magical about an initializer list in this context as far as I can see. If it's ok for a constructor argument why is it not ok for a function argument?

 

When you do something wrong with references, like returning a reference to a local from a function... you have to actually return one. You need to put "&" in the return type, thus making it abundantly clear that you're returning a reference. And thus at least potentially doing something wrong.

Returning an `initializer_list<int>` looks exactly like returning a `vector<int>`. And yet, it is almost always undefined behavior.

Nothing in the discussion so far has mentioned returning an initializer_list from a function. I'm talking about taking it as a parameter to a function / passing it as an argument to a function and using it directly as the initializer for a range based for. 

What good does learning the rules of `initializer_list` do? What's the point of having parameters of type `initializer_list`?

It allows some syntactic sugar on the user's side.

That's it. It isn't a need. It isn't a case of "I can't really do this without it". It's merely nicer syntax. Slightly.

I believe syntax is important. That belief presumably was a motivator for being able to finally write

const vector<int> v{1, 2, 3};

Rather than:

vector<int> v;

v.reserve(3);

v.push_back(1);

v.push_back(2);

v.push_back(3);

And as a direct consequence being able to write:

void f(const vector<int>&);

f({1, 2, 3});

I'm very glad we got that 'merely nicer syntax'. The way we got it is by allowing parameters of type initializer_list to constructors. It was explicitly decided according to that quote to allow it for any function and again, I'm glad that was the decision.

 

I'd rather people use what is obviously correct than have them memorize and follow a bunch of complex rules. Like "it's safe to use `initializer_list` in parameters and locals, but not in return values or NSDMs." Or whatever.

One rule: it's safe to use initializer_list as a parameter. Avoid other uses. You need to know that rule anyway to write a container constructor and it's useful to understand it to even use a container constructor. I'd say that's simpler than "don't use initailizer_lists outside of eventually 'initializing' a 'list'," which has several loosely defined terms and unnecessarily prohibits the already common practice of for (auto x : {1, 2, 3}) { ... }.

[Matt Newport]

On Sunday, 18 October 2015 21:34:34 UTC-7, Nicol Bolas wrote:

... Did you read the same paragraph that I did? Like the part about "eliminates the need for variable argument lists (… arguments) in many (most?) places?"

Because that totally did not happen. You do not see a rash of C++ programmers abandoning template parameter packs in favor of initializer lists. And really, why would they?

As I said before, I'm not particularly interested in dissecting the motivations underlying the original design of the language feature. I'm more interested in figuring out what good style means for the language rules we've actually ended up with. The original intent can be helpful guidance for that but that's about as far as it goes.

That said, I really don't think there's much overlap between the use cases for template parameter packs and initializer_list arguments. If you're constructing a container of ints, a parameter of initializer_list<int> makes a hell of a lot more sense than a variadic template constructor. Equally, if you have a function that expects a container of ints as an argument, accepting an initializer_list as well as other compatible containers makes a lot more sense than trying to use a template parameter pack. In my original motivating use case that got this whole discussion started a template parameter pack would make no sense at all.

 

It should also be noted that rule #1 about member variables? Well, `array_view` has member variables. And its constructor initializes them. Which means that any constructor from a compatible initializer list would... have to effectively initialize the array view with the initializer list.

So you're basically asking to violate rule #1.

That doesn't follow at all - array_view doesn't have a member variable of type initializer_list and I'm not suggesting it should. I'm suggesting it be possible to directly construct an array_view from an initializer_list. As you point out below, it will still be possible to do that anyway using the array_view constructor that takes a pointer and a length.

 

That's a rather silly question.

Whether that `push_back` operation causes a dangling reference depends on two factors both being true:

1) `av` is used after the `push_back`

2) The `push_back` caused reallocation

Unless reallocation actually happened, this code is fine. The user may well have reserved sufficient space. Or maybe the user checked the capacity before doing the push_back. In any case, local static analysis cannot possibly be sufficient to prove that this code is broken.

If I saw this code while reading code or doing a code review I'd be suspicious, and I'd argue anyone else should be too. Even if I see a reserve() I'd still want to check the logic carefully to ensure that sufficient space is reserved for all possible execution paths. Unless there's a very good reason for the code to be written that way I'd say it should be rewritten in fact. This is why I still claim that 1) is strictly better style than 2):

void f(array_view<int>);

// 1

f(vector<int>{1, 2, 3})

// 2

auto v = vector<int>{1, 2, 3};

auto av = array_view<int>{v};

f(av);

Because if you don't need av after the call to f(), I as a reader don't want to have to manually check you don't actually use it. It's also preferable that your allocation has the shortest necessary lifespan (not particularly significant for this case but could be for f(makeVector()). Of course for efficiency this would be much better: f({1, 2, 3}) if an array_view could be constructed from a temporary initializer_list.

 

By contrast, in your second case any use of `av` after its creation is always undefined behavior and obviously so. Static analysis wouldn't take long to find that dangling reference.

Which is why I don't consider it a major enough problem to outlaw the safe uses like f({1, 2, 3}). I made that point in the original issue: GSL is intended to work alongside static analysis tools that actually do try to catch the push_back() scenario. If they can catch that, then catching creation of an array_view from a dangling temporary seems like it should be relatively trivial. So why use a coarse facility like removing && constructors that as a side effect disable safe and convenient uses?

But really, worse-comes-to-worse, you can just define a quickie function of your own:

auto adapt(initializer_list<T> list) {return array_view<T>{list.begin(), list.size()};}

Similar code could be written for adapting rvalue references.

Yes, I pointed out as much in the discussion of a related issue and I will if necessary. I'd just rather not have to. The fact that the wrapper code to work around the removal of && constructors of containers is a relatively trivial

template<typename C>
array_view<std::add_const_t<typename C::value_type>> av(const C& x) { return {x}; }

Highlights to me that deleting the && constructors in the first place is a crude attempt to work against the spirit of the language which happily converts temporaries to const references (and even the strictest compilers and static analyzers will rightly not warn about it as far as I'm aware).

[Nicol Bolas]

On Monday, October 19, 2015 at 2:00:55 AM UTC-4, Matt Newport wrote:

On Sunday, 18 October 2015 19:36:55 UTC-7, Nicol Bolas wrote:

I think you're equating things that aren't equal.

I don't think I am. If I'm reading or reviewing some code I don't see any safety / correctness reason to consider any of these particularly suspicious:

The two things you were equating was the lifetime extension rules of temporaries and the use of initializer_lists. That a good C++ programmer should rely on the former, so why not rely on the latter too.

Your examples don't address that point. I was explaining how they're not the same thing, since temporaries are visible, while the object storing the initializer_list is not.
 

When you do something wrong with references, like returning a reference to a local from a function... you have to actually return one. You need to put "&" in the return type, thus making it abundantly clear that you're returning a reference. And thus at least potentially doing something wrong.

Returning an `initializer_list<int>` looks exactly like returning a `vector<int>`. And yet, it is almost always undefined behavior.

Nothing in the discussion so far has mentioned returning an initializer_list from a function. I'm talking about taking it as a parameter to a function / passing it as an argument to a function and using it directly as the initializer for a range based for.

If you normalize the use of initializer_list by encouraging them to be used as temporary arrays for any purposes, you will encourage people to think that plenty of other use cases are legal. Like returning them.

What good does learning the rules of `initializer_list` do? What's the point of having parameters of type `initializer_list`?

It allows some syntactic sugar on the user's side.

That's it. It isn't a need. It isn't a case of "I can't really do this without it". It's merely nicer syntax. Slightly.

I believe syntax is important. That belief presumably was a motivator for being able to finally write

const vector<int> v{1, 2, 3};

Rather than:

vector<int> v;

v.reserve(3);

v.push_back(1);

v.push_back(2);

v.push_back(3);

And as a direct consequence being able to write:

void f(const vector<int>&);

f({1, 2, 3});

I'm very glad we got that 'merely nicer syntax'. The way we got it is by allowing parameters of type initializer_list to constructors. It was explicitly decided according to that quote to allow it for any function and again, I'm glad that was the decision.

That's "merely nicer syntax" for initializing variables, not for using `initializer_lists` as shorthands for temporary arrays. The latter is what you are trying to do.

I'd rather people use what is obviously correct than have them memorize and follow a bunch of complex rules. Like "it's safe to use `initializer_list` in parameters and locals, but not in return values or NSDMs." Or whatever.

One rule: it's safe to use initializer_list as a parameter.

But that's not the rule you're talking about. You want to use `initializer_list` as a way to construct an array_view. That's rather different from being "a parameter", don't you think? The effect is to take a reference to the temporary initializer list.

That's a very different kind of thing.

[Nicol Bolas]

On Monday, October 19, 2015 at 2:39:00 AM UTC-4, Matt Newport wrote:

On Sunday, 18 October 2015 21:34:34 UTC-7, Nicol Bolas wrote:

... Did you read the same paragraph that I did? Like the part about "eliminates the need for variable argument lists (… arguments) in many (most?) places?"

Because that totally did not happen. You do not see a rash of C++ programmers abandoning template parameter packs in favor of initializer lists. And really, why would they?

As I said before, I'm not particularly interested in dissecting the motivations underlying the original design of the language feature. I'm more interested in figuring out what good style means for the language rules we've actually ended up with. The original intent can be helpful guidance for that but that's about as far as it goes.

That said, I really don't think there's much overlap between the use cases for template parameter packs and initializer_list arguments.

Agreed. Which is what made me wonder why you quoted it in your defense, when it's clearly not very well founded?

Equally, if you have a function that expects a container of ints as an argument, accepting an initializer_list as well as other compatible containers

You keep proving exactly why we should forbid such things. Because you keep believing that `initializer_list` is a container. It is not. And treating it as though it were a container is very much wrong.

Let me lay out the argument very succinctly.

1) `initializer_list` is not a container.

2) `array_view` is meant to be initialized from containers.

3) Therefore, allowing `array_view` to be initialized from `initializer_list` is wrong.

#1 is undeniably true, and the C++ standard makes that abundantly clear. So the only point of contention is #2: whether `array_view` ought to be initialized from non-containers (other than itself, of course).

It should also be noted that rule #1 about member variables? Well, `array_view` has member variables. And its constructor initializes them. Which means that any constructor from a compatible initializer list would... have to effectively initialize the array view with the initializer list.

So you're basically asking to violate rule #1.

That doesn't follow at all - array_view doesn't have a member variable of type initializer_list and I'm not suggesting it should. I'm suggesting it be possible to directly construct an array_view from an initializer_list.

... what?

You do know that an `initializer_list` is just a pair of pointers, right? And that `array_view` is (or can be implemented as) a pair of pointers. And therefore, if an array_view is constructed from an `initializer_list`, then it is storing an `initializer_list` in its NSDMs.

Oh sure, `array_view` does not literally store an `initializer_list` member. But it is exactly equivalent to storing one, with all of the inherent dangers therein.

As you point out below, it will still be possible to do that anyway using the array_view constructor that takes a pointer and a length.

Yes. And the fact that it requires special syntax should clue readers in that something potentially dangerous is going on. That's why it should not be implicitly allowed.

Potentially unsafe or il-advised things should not be implicitly allowed.

That's a rather silly question.

Whether that `push_back` operation causes a dangling reference depends on two factors both being true:

1) `av` is used after the `push_back`

2) The `push_back` caused reallocation

Unless reallocation actually happened, this code is fine. The user may well have reserved sufficient space. Or maybe the user checked the capacity before doing the push_back. In any case, local static analysis cannot possibly be sufficient to prove that this code is broken.

If I saw this code while reading code or doing a code review I'd be suspicious, and I'd argue anyone else should be too.

Suspicious, perhaps. But that's the thing about static analyzers. The last thing you want is a bunch of false positives, because eventually... you'll just start ignoring them.

It is perfectly valid, and quite frankly entirely reasonable, to rely on the ability to add more elements to a vector without invalidating iterators. Yes, you want to make sure that said code isn't broken. But static analysis alone can't resolve that.

This is why I still claim that 1) is strictly better style than 2):

void f(array_view<int>);

// 1

f(vector<int>{1, 2, 3})

// 2

auto v = vector<int>{1, 2, 3};

auto av = array_view<int>{v};

f(av);

Because if you don't need av after the call to f(), I as a reader don't want to have to manually check you don't actually use it.

Or, you could just do `f(v)` like most people. Your insistence on making the `array_view` a variable that persists after its use is a strawman argument: designed specifically to fail, yet it misrepresents the actual position.

By contrast, in your second case any use of `av` after its creation is always undefined behavior and obviously so. Static analysis wouldn't take long to find that dangling reference.

Which is why I don't consider it a major enough problem to outlaw the safe uses like f({1, 2, 3}). I made that point in the original issue: GSL is intended to work alongside static analysis tools that actually do try to catch the push_back() scenario. If they can catch that, then catching creation of an array_view from a dangling temporary seems like it should be relatively trivial. So why use a coarse facility like removing && constructors that as a side effect disable safe and convenient uses?

Because not everyone is going to be using static analysis tools with `array_view`. The core guidelines are what are intended to be used with static analysis. `gsl::array_view` was developed from (and tracks) various proposals to the standard library.

And you can't expect everyone to be using such static analysis tools.
 

But really, worse-comes-to-worse, you can just define a quickie function of your own:

auto adapt(initializer_list<T> list) {return array_view<T>{list.begin(), list.size()};}

Similar code could be written for adapting rvalue references.

Yes, I pointed out as much in the discussion of a related issue and I will if necessary. I'd just rather not have to. The fact that the wrapper code to work around the removal of && constructors of containers is a relatively trivial

template<typename C>
array_view<std::add_const_t<typename C::value_type>> av(const C& x) { return {x}; }

It'd be better to write that by just using `as_array_view`:

template<typename C> auto av(const C &x) {return as_array_view(x);}

Highlights to me that deleting the && constructors in the first place is a crude attempt to work against the spirit of the language which happily converts temporaries to const references

Um, no. See, a `const&` as a parameter could be filled in by a temporary. But it could also be a constant reference to a non-temporary. Because there's no way to know which, and because the latter is valid, `array_view` must allow the direct use of `const&` types. What it doesn't allow is direct use of actual temporaries. Indirect use (as in your code) is allowed, through the use of a `const&`.

Also, what happened to "I'm not particularly interested in dissecting the motivations underlying the original design of the language feature?" Or are you just dismissing the "spirit of the language" when it's not in your favor?

Also, the fact that the language converts temporaries to `const&` doesn't mean that a type forbidding conversion from temporaries is somehow wrongly designed. Indeed, if the "spirit of the language" was what you claim it to be, the language would make it impossible to forbid such conversion. That it wouldn't let you recognize whether something is a temporary at all.

[Matt Newport]

On Monday, 19 October 2015 06:39:09 UTC-7, Nicol Bolas wrote:

The two things you were equating was the lifetime extension rules of temporaries and the use of initializer_lists. That a good C++ programmer should rely on the former, so why not rely on the latter too.

Your examples don't address that point. I was explaining how they're not the same thing, since temporaries are visible, while the object storing the initializer_list is not.

I think perhaps the crux of the disagreement is here somewhere. To me, if there is a difference here at all, it is a difference that makes no difference (at least for any of the uses we are discussing here). For all practical purposes, you can treat the braced init list itself as a 'visible temporary' and apply ordinary C++ lifetime rules to it to conclude that any of the following are safe (without needing to assume anything about the implementations that allow these to compile other than that they are correct):

auto v = vector<int>{1, 2, 3};

for (auto x : {1, 2, 3}) { ... }

f({1, 2, 3});

auto e = some2DArrayClass[{x, y}];

struct Foo { vector<int> v; Foo() : v{1, 2, 3} {} };

We can agree that these are all valid and safe right? And that they all essentially boil down to a call to a constructor or function with an std::initializer_list parameter? And that the validity and safety does not depend on the implementation details of anything, beyond that they are correct? Specifically that there is no requirement that the contents of the braced init list is ultimately used to initialize a container for these to be valid and safe, e.g. if f() is:

void f(std::initializer_list<int> x) { for (auto i : x) { std::cout << i << ", "; }

Then the call f({1, 2, 3}) is safe and valid. The disagreement on these uses is not around safety directly, it's around style and whether such uses somehow encourage other unsafe uses, correct? We both agree that returning an initializer_list from a function would be bad. Can you give me an example of another situation where the simplifying assumption that ordinary C++ lifetime rules for temporaries apply if you view a braced init list as a 'visible temporary' would lead to writing bad code?

 

 If you normalize the use of initializer_list by encouraging them to be used as temporary arrays for any purposes, you will encourage people to think that plenty of other use cases are legal. Like returning them.

I still think that the straightforward rule that std::initializer_list is safe to use as a parameter and should be avoided elsewhere covers this. I can understand where the concern comes from but it's a hypothetical concern and it doesn't worry me overly much to be honest. I don't find any argument I've seen so far persuasive that this is something I should actually be concerned about.

 

That's "merely nicer syntax" for initializing variables, not for using `initializer_lists` as shorthands for temporary arrays. The latter is what you are trying to do.

Under current language rules, such use is guaranteed safe and valid I believe. If there's some situation where it's not, that's exactly the kind of information I was looking for when I started this thread. Assuming I'm right though, I'd like to use it where it makes for more concise, more efficient and more semantically clear code - e.g. if what I want is to pass a temporary array of ints to a function then the best possible syntax for that is f({1, 2, 3}) and if the language provides a way for me to make that work safely then I'll take advantage of it.

 

But that's not the rule you're talking about. You want to use `initializer_list` as a way to construct an array_view. That's rather different from being "a parameter", don't you think? The effect is to take a reference to the temporary initializer list.

It's true that I do want to do that but it was not the primary issue behind my question at the top of this thread. I agree that it is a different case from merely passing an std::initializer_list as a parameter and that the additional concern of easily creating an array_view with a dangling reference to a temporary is not a non-issue. I'm interested in discussing that further but I'd like to keep it somewhat separate from the primary motivation for this thread, which is whether it should really be considered bad style to use an std::initalizer_list parameter in a context other than constructing a container:

[Matt Newport]

On Monday, 19 October 2015 08:42:22 UTC-7, Nicol Bolas wrote:

Equally, if you have a function that expects a container of ints as an argument, accepting an initializer_list as well as other compatible containers

You keep proving exactly why we should forbid such things. Because you keep believing that `initializer_list` is a container. It is not. And treating it as though it were a container is very much wrong.

No, std::initializer_list is not a container. A braced init list is a kind of container however (albeit a rather unusual one) and an std::initializer_list is a view over that container in the same way that an array is a container and an array_view is a view over that container. 

I should have been more precise in my wording above. What I should have said is "if you have a function that expects a view over a container of ints as an argument, accepting an initializer_list as well as other compatible views over a container of ints makes a lot more sense than trying to use a template parameter pack". Here a 'view' would be 'something on which I can call std::begin() and std::end() to obtain a pair of const int iterators'. Suitable 'views' in this sense would include std::initializer_list<int>, array_view<const int>, const std::vector<int>&, std::vector<int>&&, const std::list<int>&, const int (&)[N], const std::array<int, N>&, etc.

 

That doesn't follow at all - array_view doesn't have a member variable of type initializer_list and I'm not suggesting it should. I'm suggesting it be possible to directly construct an array_view from an initializer_list.

... what?

You do know that an `initializer_list` is just a pair of pointers, right? And that `array_view` is (or can be implemented as) a pair of pointers. And therefore, if an array_view is constructed from an `initializer_list`, then it is storing an `initializer_list` in its NSDMs.

Oh sure, `array_view` does not literally store an `initializer_list` member. But it is exactly equivalent to storing one, with all of the inherent dangers therein.

Yes, I know all of that. I was making this point in the context of my claim that a good rule for std::initializer_list usage would be "it's safe to use std::initializer_list as a parameter, other uses should be avoided", and 'other uses' would include 'declaring a class member of type std::initializer_list<T>'. Nobody is suggesting array_view do that. Note that this rule says absolutely nothing about avoiding declaring a class containing a pair of pointers. 

The question of whether it's a good idea to have an array_view over a braced initializer list, constructed via an std::initializer_list, is a separate issue. There are many ways to use a non-owning array_view unsafely. There also many ways of using an array_view over underlying storage of a braced initializer list safely. The whole discussion around array_view boils down to when we should make the safe uses less convenient in an attempt to prevent the unsafe uses. There is no library level way to prevent all unsafe uses of array_view and even static analysis will not be able to distinguish all safe from all unsafe uses.

 

As you point out below, it will still be possible to do that anyway using the array_view constructor that takes a pointer and a length.

Yes. And the fact that it requires special syntax should clue readers in that something potentially dangerous is going on. That's why it should not be implicitly allowed.

Potentially unsafe or ill-advised things should not be implicitly allowed.

In general I agree with that sentiment. The issue with array_view is that there is no library level way to distinguish between safe implicit things and unsafe implicit things:

void f(array_view<int>);

f(makeVector()) // safe implicit construction

f({1, 2, 3}); // safe implicit construction 

auto av = array_view<int>{makeVector()}; // unsafe implicit construction

auto av = array_view<int>{{1, 2, 3}}; // unsafe implicit construction

If there was a library level way to allow the first and not the second I don't think there'd be any need for debate. Now I'd actually be ok with explicit constructors here:

f(array_view<int>{makeVector()});

f(array_view<int>{{1, 2, 3}});

But the redundantly specified 'int' means I'd rather use a helper function.

 

Suspicious, perhaps. But that's the thing about static analyzers. The last thing you want is a bunch of false positives, because eventually... you'll just start ignoring them.

It is perfectly valid, and quite frankly entirely reasonable, to rely on the ability to add more elements to a vector without invalidating iterators. Yes, you want to make sure that said code isn't broken. But static analysis alone can't resolve that.

I don't disagree with any of this. GSL is trying to make code like that disallowed by default though: see the section on "Invalidation by modifying owners" on page 10 of the CppCoreGuidelines Lifetime Safety paper. I think that context is relevant for discussions of the design of array_view specifically.

This is why I still claim that 1) is strictly better style than 2):

void f(array_view<int>);

// 1

f(vector<int>{1, 2, 3})

// 2

auto v = vector<int>{1, 2, 3};

auto av = array_view<int>{v};

f(av);

Because if you don't need av after the call to f(), I as a reader don't want to have to manually check you don't actually use it.

Or, you could just do `f(v)` like most people. Your insistence on making the `array_view` a variable that persists after its use is a strawman argument: designed specifically to fail, yet it misrepresents the actual position.

What I'm actually arguing for is 1) above: f(vector<int>{1, 2, 3}) or f({1, 2, 3}). People keep telling me that is bad because it allows auto av = array_view{vector<int>{1, 2, 3}; which I do think is largely a strawman argument designed specifically to fail. I very much don't want an array_view to persist beyond its use which is the entire basis of my desire to allow 1).

 

Also, the fact that the language converts temporaries to `const&` doesn't mean that a type forbidding conversion from temporaries is somehow wrongly designed. Indeed, if the "spirit of the language" was what you claim it to be, the language would make it impossible to forbid such conversion. That it wouldn't let you recognize whether something is a temporary at all.

It seems to me that the main intent of introducing rvalue references and giving you the ability to detect the case where your argument is a temporary was to allow for optimizations in that case, not to forbid it. A secondary major use case was to only allow temporary arguments, as in the case of move only types. Outlawing temporaries is not a common use of rvalue reference arguments from what I've seen.

[Nicol Bolas]

On Monday, October 19, 2015 at 2:09:07 PM UTC-4, Matt Newport wrote:

On Monday, 19 October 2015 06:39:09 UTC-7, Nicol Bolas wrote:
 

Can you give me an example of another situation where the simplifying assumption that ordinary C++ lifetime rules for temporaries apply if you view a braced init list as a 'visible temporary' would lead to writing bad code?

This is perfectly legal code, based on temporary life extension rules and copy construction:

vector<int> func()
{
  const vector<int> &le = ...;
  //do stuff
  return le;
}

This is based on the same rules:

initializer_list<int> func()
{
  initializer_list<int> le = ...;
  return le;
}

Yet it does not work.

A user who believes that `initializer_list` works the same way as temporaries for any type will expect both of these to work. Yet the latter does not. And if a user has to understand why #2 fails and #1 works, then they know too much about C++.

It is far safer to provide a general rule: "use initializer_lists only to initialize objects." If users are trained to think of a braced-init-list that is deduced as an initializer_list as simply a temporary, then sooner or later they will use it wrong.

We want to stop that.
 

If you normalize the use of initializer_list by encouraging them to be used as temporary arrays for any purposes, you will encourage people to think that plenty of other use cases are legal. Like returning them.

I still think that the straightforward rule that std::initializer_list is safe to use as a parameter and should be avoided elsewhere covers this.

But you encourage their use in range-based for loops too. That's hardly a function parameter.
 

I can understand where the concern comes from but it's a hypothetical concern and it doesn't worry me overly much to be honest. I don't find any argument I've seen so far persuasive that this is something I should actually be concerned about.

Here's the thing: You asked for the reasoning. We've provided it. You have agreed that there is a concern. You simply decide that it doesn't "worry me overly much."

Well, it does worry other people. And we don't believe that the few minor syntactic conveniences are worth encouraging users to use `initializer_list`.

[Matt Newport]

On Tuesday, 20 October 2015 09:36:12 UTC-7, Nicol Bolas wrote:

On Monday, October 19, 2015 at 2:09:07 PM UTC-4, Matt Newport wrote:

Can you give me an example of another situation where the simplifying assumption that ordinary C++ lifetime rules for temporaries apply if you view a braced init list as a 'visible temporary' would lead to writing bad code?

This is perfectly legal code, based on temporary life extension rules and copy construction:

vector<int> func()
{
  const vector<int> &le = ...;
  //do stuff
  return le;
}

This is based on the same rules:

initializer_list<int> func()
{
  initializer_list<int> le = ...;
  return le;
}

Yet it does not work.

A user who believes that `initializer_list` works the same way as temporaries for any type will expect both of these to work. Yet the latter does not. And if a user has to understand why #2 fails and #1 works, then they know too much about C++.

This does not address the question I asked which was regarding the lifetime of a temporary braced init list. I've already stated repeatedly that an std::initializer_list should not be returned from a function. It behaves like a view over the braced init list not a container and therefore the code you posted is just as problematic as:

array_view<int> func() {

    vector<int> v{1, 2, 3};

    array_view<int> av{v};

    ...

    return av;

}

If you're using any type of non-owning view you have to understand lifetime rules in C++ and the semantics of the type you are using. I don't think that is asking too much of a user.

 

Here's the thing: You asked for the reasoning. We've provided it. You have agreed that there is a concern. You simply decide that it doesn't "worry me overly much."

Well, it does worry other people. And we don't believe that the few minor syntactic conveniences are worth encouraging users to use `initializer_list`.

Yes, I think the discussion has more or less reached a conclusion. I asked because I thought perhaps there was an actual safety or other concern I was missing rather than it being just a stylistic preference. It seems there is no actual problem here, it's just a matter of taste so ultimately we can agree to disagree. 

[Nicol Bolas]

I wouldn't be too sure.

Check out the current draft of the ranges proposal (pdf). In particular, appendix C.10, which says:

-> Algorithms that do not mutate their input should accept `initializer_list` arguments wherever `Iterable`s are allowed.

So you're not the only person who agrees with you.

[Matt Newport]

On Wednesday, 21 October 2015 05:54:17 UTC-7, Nicol Bolas wrote:

I wouldn't be too sure.

Check out the current draft of the ranges proposal (pdf). In particular, appendix C.10, which says:

-> Algorithms that do not mutate their input should accept `initializer_list` arguments wherever `Iterable`s are allowed.

So you're not the only person who agrees with you.

Thanks for the link, that's interesting to see. By "I wouldn't be too sure" you mean you think discussion of this question is likely to continue, in part because of the ranges proposal? 

Ranges were brought up in the discussion around allowing construction of array_view from a Container&& since Ranges also appear to disallow this:

The downside of this design is that it is sometimes desirable to do this:// Try to adapt an rvalue containerauto rng = vector<int>{1,2,3,4} | view::reverse; // OK?Adaptors operate on and yield Ranges; other Iterables (i.e., containers) are used to construct Ranges by first taking their begin and end. The code above is unsafe because rng will be left holding invalid iterators into a container that no longer exists. Our solution is to disallow the above code. It is illegal to adapt an rvalue non-Range. (Adapting rvalue Ranges, however, is perfectly acceptable; indeed necessary if adaptor pipelines are to work.) See Appendix 6 for the mechanics of how we distinguish between Iterables and Ranges.

I haven't spent enough time studying the Range proposal to fully understand how it approaches this issue but my current understanding is that Actions allow you to write pipeline style code using temporary inputs:

Actions
When you want to mutate a container in-place, or forward it through a chain of mutating operations, you can use actions. The following examples should make it clear.
Read data into a vector, sort it, and make it unique.

extern std::vector<int> read_data();
using namespace ranges;
std::vector<int> vi = read_data() | action::sort | action::unique;

I'm not entirely clear on whether these are allowed (and I can't easily test right now since VS2015 doesn't support the current Range v3 implementation) but I assume not for the same reason auto rng = vector<int>{1,2,3,4} | view::reverse is not allowed:

void f(view<int> v); // I don't know the correct syntax to express this but hopefully the intent is clear

f(read_data()); // ok ?

f(read_data() | view::reverse); // ok?

But I would prefer such safe code to compile, even at the expense of allowing the dangerous version.

[Matthew Woehlke]

On 2015-10-21 13:26, Matt Newport wrote:
> Ranges were brought up in the discussion

> <https://github.com/Microsoft/GSL/issues/138>around allowing construction

> of array_view from a Container&& since Ranges also appear to disallow

> <https://ericniebler.github.io/std/wg21/D4128.html#ranges-cannot-own-elements>
> this:

That sounds correct. Interestingly, this same point has come up over in
the Qt list regarding a proposed QStringView¹. The problem is that this
lets you do fundamentally broken things like:

std::vector<int> foo();
void bar(std::array_view<int>);

bar(foo()); // oops, bar receives a dangling reference!

(Same reason that the std::add_const proposal is inadequate.)

Lifetime extension should fix this, but we're not there yet. When/if
that gets sorted and into the standard, maybe we can revisit and remove
this restriction.

(¹ http://permalink.gmane.org/gmane.comp.lib.qt.devel/23582)

--
Matthew

[Matt Newport]

On Wednesday, 21 October 2015 10:49:28 UTC-7, Matthew Woehlke wrote:

  std::vector<int> foo();
  void bar(std::array_view<int>);

  bar(foo()); // oops, bar receives a dangling reference!

That's already perfectly safe, that's kind of the entire point of this discussion... 

[Matt Newport]

Sorry, that link was to the wrong code. Here's the example I meant to link.

[Matthew Woehlke]

On 2015-10-21 14:42, Matt Newport wrote:
> On Wednesday, 21 October 2015 10:49:28 UTC-7, Matthew Woehlke wrote:
>>
>> std::vector<int> foo();
>> void bar(std::array_view<int>);
>>
>> bar(foo()); // oops, bar receives a dangling reference!
>>
>
> That's already perfectly safe

> <http://coliru.stacked-crooked.com/a/0802ad2eee2d9f4e>, that's kind of the

> entire point of this discussion...

Oh. Ugh... here's where it breaks down:

auto&& av = std::array_view<int>(foo());
// oops, av is invalid!

That's more subtle than I was recalling that the problem was. Which...
is perhaps even more scary.

--
Matthew

[David Krauss]

On 2015–11–03, at 2:17 AM, Matthew Woehlke <mwoehlk...@gmail.com> wrote:

Oh. Ugh... here's where it breaks down:

 auto&& av = std::array_view<int>(foo());
 // oops, av is invalid!

That's more subtle than I was recalling that the problem was. Which...
is perhaps even more scary.

I’ve implemented the basic parts of P0066R0, Lifetime extension for accessors and views: https://github.com/potswa/clang/tree/lifetime_extension. You should be able to use it to fix such cases. If array_view has a defaulted copy constructor and a member pointer, then the fix is automatic.

It might be worth a spin, and I’d appreciate usage feedback from anyone who’s interested in dangling reference problems, especially as exacerbated by idiomatic use of views. (It is an experimental prototype, though. You need to build it yourself, and do not install it over your mainline Clang! Testing is minimal so far, and the extension is not controlled by any command-line flag.)