Discussion:
[std-discussion] std::initializer_list and good modern C++ style
Matt Newport
2015-10-17 22:42:43 UTC
Permalink
I raised an issue <https://github.com/Microsoft/GSL/issues/137> 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
<http://coliru.stacked-crooked.com/a/a897b1943902419a>.

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.
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at http://groups.google.com/a/isocpp.org/group/std-discussion/.
Nicol Bolas
2015-10-18 14:48:10 UTC
Permalink
Post by Matt Newport
I raised an issue <https://github.com/Microsoft/GSL/issues/137> 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
Post by Matt Newport
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
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.
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at http://groups.google.com/a/isocpp.org/group/std-discussion/.
Sam Kellett
2015-10-18 18:24:44 UTC
Permalink
Post by Nicol Bolas
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
Post by Nicol Bolas
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...
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at http://groups.google.com/a/isocpp.org/group/std-discussion/.
Nicol Bolas
2015-10-18 19:56:05 UTC
Permalink
Post by Sam Kellett
Post by Nicol Bolas
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.
Post by Sam Kellett
I can understand your concern about the current proposal explicitly
Post by Nicol Bolas
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
<http://www.open-std.org/JTC1/SC22/WG21/docs/papers/2015/n4480.html#string.view>
doesn't have one. It only has a `const&` one.
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at http://groups.google.com/a/isocpp.org/group/std-discussion/.
Sam Kellett
2015-10-18 20:12:04 UTC
Permalink
Post by Nicol Bolas
Post by Sam Kellett
Post by Nicol Bolas
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...
Post by Nicol Bolas
I can understand your concern about the current proposal explicitly
Post by Sam Kellett
Post by Nicol Bolas
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
<http://www.open-std.org/JTC1/SC22/WG21/docs/papers/2015/n4480.html#string.view>
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!
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at http://groups.google.com/a/isocpp.org/group/std-discussion/.
Sam Kellett
2015-10-18 20:15:24 UTC
Permalink
Post by Sam Kellett
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
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
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
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at http://groups.google.com/a/isocpp.org/group/std-discussion/.
Thiago Macieira
2015-10-18 20:51:23 UTC
Permalink
Post by Sam Kellett
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
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at http://groups.google.com/a/isocpp.org/group/std-discussion/.
Matt Newport
2015-10-19 01:21:47 UTC
Permalink
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.
Post by Thiago Macieira
Post by Sam Kellett
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
E067 918B B660 DBD1 105C 966C 33F5 F005 6EF4 5358
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at http://groups.google.com/a/isocpp.org/group/std-discussion/.
Thiago Macieira
2015-10-19 01:55:45 UTC
Permalink
Post by 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.
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.
--
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
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at http://groups.google.com/a/isocpp.org/group/std-discussion/.
Matt Newport
2015-10-19 02:11:30 UTC
Permalink
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.
Post by Matt Newport
Post by Matt Newport
They are both as dangerous because they are both not dangerous. There is
no
Post by Matt Newport
dangling temporary here. The lifetime of the temporary is clearly
specified
Post by Matt Newport
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.
--
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
Software Architect - Intel Open Source Technology Center
E067 918B B660 DBD1 105C 966C 33F5 F005 6EF4 5358
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at http://groups.google.com/a/isocpp.org/group/std-discussion/.
Nicol Bolas
2015-10-19 02:36:55 UTC
Permalink
Post by 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.
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.
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at http://groups.google.com/a/isocpp.org/group/std-discussion/.
Matt Newport
2015-10-19 06:00:54 UTC
Permalink
Post by Nicol Bolas
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.
Post by Nicol Bolas
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?
Post by Nicol Bolas
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
Post by Nicol Bolas
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.
Post by Nicol Bolas
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}) { ... }.
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at http://groups.google.com/a/isocpp.org/group/std-discussion/.
Nicol Bolas
2015-10-19 13:39:09 UTC
Permalink
Post by Matt Newport
Post by Nicol Bolas
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
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.
Post by Matt Newport
When you do something wrong with references, like returning a reference to
Post by Nicol Bolas
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
Post by Matt Newport
Post by Nicol Bolas
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};
vector<int> v;
v.reserve(3);
v.push_back(1);
v.push_back(2);
v.push_back(3);
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
Post by Matt Newport
Post by Nicol Bolas
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.
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at http://groups.google.com/a/isocpp.org/group/std-discussion/.
Matt Newport
2015-10-19 18:09:06 UTC
Permalink
Post by Nicol Bolas
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?
Post by Nicol Bolas
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.
Post by Nicol Bolas
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.
Post by Nicol Bolas
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:

In that discussion Gaby Dos Reis said "any use of std::initializer_list in
Post by Nicol Bolas
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.
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at http://groups.google.com/a/isocpp.org/group/std-discussion/.
Nicol Bolas
2015-10-20 16:36:11 UTC
Permalink
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
Post by Nicol Bolas
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`.
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at http://groups.google.com/a/isocpp.org/group/std-discussion/.
Matt Newport
2015-10-20 17:20:51 UTC
Permalink
Post by Nicol Bolas
Post by Nicol Bolas
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
vector<int> func()
{
const vector<int> &le = ...;
//do stuff
return le;
}
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.
Post by Nicol Bolas
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.
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at http://groups.google.com/a/isocpp.org/group/std-discussion/.
Nicol Bolas
2015-10-21 12:54:17 UTC
Permalink
Post by Matt Newport
Post by Nicol Bolas
Post by Nicol Bolas
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
vector<int> func()
{
const vector<int> &le = ...;
//do stuff
return le;
}
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
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.
Post by Nicol Bolas
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.
I wouldn't be too sure.

Check out the current draft of the ranges proposal <http://wg21.link/P0021>
(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.
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at http://groups.google.com/a/isocpp.org/group/std-discussion/.
Matt Newport
2015-10-21 17:26:08 UTC
Permalink
Post by Nicol Bolas
I wouldn't be too sure.
Check out the current draft of the ranges proposal
-> 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
<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:

The downside of this design is that it is sometimes desirable to do this://
Post by Nicol Bolas
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
<https://ericniebler.github.io/std/wg21/D4128.html#on-distinguishing-ranges-from-non-range-iterables> 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
<https://ericniebler.github.io/range-v3/#tutorial-quick-start>you to write
pipeline style code using temporary inputs:

Actions
Post by Nicol Bolas
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();
Post by Nicol Bolas
using namespace ranges
<https://ericniebler.github.io/range-v3/namespaceranges.html>;
std::vector<int> vi = read_data() | action::sort
<https://ericniebler.github.io/range-v3/group__transformation.html#gacb0ffa1c28bd04a71adae30dc02ac000>
| action::unique
<https://ericniebler.github.io/range-v3/group__transformation.html#gad96613af2fa2b8b09f7ba5c907cdd62d>
;
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.
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at http://groups.google.com/a/isocpp.org/group/std-discussion/.
Matthew Woehlke
2015-10-21 17:49:02 UTC
Permalink
Post by Matt Newport
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>
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
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at http://groups.google.com/a/isocpp.org/group/std-discussion/.
Matt Newport
2015-10-21 18:42:13 UTC
Permalink
Post by Matthew Woehlke
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...
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at http://groups.google.com/a/isocpp.org/group/std-discussion/.
Matt Newport
2015-10-21 18:44:05 UTC
Permalink
Sorry, that link was to the wrong code. Here's
<http://coliru.stacked-crooked.com/a/75ae1efb1ddacec1> the example I meant
to link.
Post by Matt Newport
Post by Matthew Woehlke
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...
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at http://groups.google.com/a/isocpp.org/group/std-discussion/.
Matthew Woehlke
2015-11-02 18:17:35 UTC
Permalink
Post by Matt Newport
Post by Matthew Woehlke
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
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at http://groups.google.com/a/isocpp.org/group/std-discussion/.
David Krauss
2015-11-03 03:53:00 UTC
Permalink
Post by Matthew Woehlke
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 <http://wg21.link/p0066>, Lifetime extension for accessors and views: https://github.com/potswa/clang/tree/lifetime_extension <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.)
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at http://groups.google.com/a/isocpp.org/group/std-discussion/.
Gabriel Dos Reis
2015-10-18 20:55:30 UTC
Permalink
Sam Kellett <***@gmail.com> writes:

| On 18 October 2015 at 15:48, Nicol Bolas <***@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
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at http://groups.google.com/a/isocpp.org/group/std-discussion/.
Matt Newport
2015-10-19 01:13:56 UTC
Permalink
Post by Nicol Bolas
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
<http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2215.pdf> on the
motivation for initializer lists being added to the language. That document
however has this section (emphasis mine):

4.4 Syntax
Post by Nicol Bolas
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
Post by Nicol Bolas
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
Post by Matt Newport
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
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.
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at http://groups.google.com/a/isocpp.org/group/std-discussion/.
Nicol Bolas
2015-10-19 04:34:34 UTC
Permalink
Post by Matt Newport
Post by Nicol Bolas
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
<http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2215.pdf> on
the motivation for initializer lists being added to the language. That
4.4 Syntax
Post by Nicol Bolas
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
Post by Matt Newport
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
Post by Matt Newport
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
Post by Matt Newport
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
Post by Matt Newport
(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
auto v = makeVector();
auto av = array_view<int>{v};
v.push_back(4);
// oops, potential dangling reference
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.
Post by Matt Newport
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.
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at http://groups.google.com/a/isocpp.org/group/std-discussion/.
Matt Newport
2015-10-19 06:39:00 UTC
Permalink
Post by Nicol Bolas
... 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
<https://github.com/Microsoft/GSL/issues/137> that got this whole
discussion started a template parameter pack would make no sense at all.
Post by Nicol Bolas
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.
Post by Nicol Bolas
That's a rather silly question.
Whether that `push_back` operation causes a dangling reference depends on
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.
Post by Nicol Bolas
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
Post by Nicol Bolas
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).
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at http://groups.google.com/a/isocpp.org/group/std-discussion/.
Nicol Bolas
2015-10-19 15:42:22 UTC
Permalink
Post by Matt Newport
Post by Nicol Bolas
... 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
Post by Matt Newport
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,
Post by Matt Newport
Post by Nicol Bolas
`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
Post by Matt Newport
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.
Post by Matt Newport
Post by Nicol Bolas
Whether that `push_back` operation causes a dangling reference depends on
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.
Post by Matt Newport
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
Post by Matt Newport
Post by Nicol Bolas
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.
Post by Matt Newport
But really, worse-comes-to-worse, you can just define a quickie function
Post by Nicol Bolas
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
Post by Matt Newport
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.
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at http://groups.google.com/a/isocpp.org/group/std-discussion/.
Matt Newport
2015-10-19 19:13:23 UTC
Permalink
Post by Matt Newport
Equally, if you have a function that expects a container of ints as an
Post by Matt Newport
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.
Post by Matt Newport
That doesn't follow at all - array_view doesn't have a member variable of
Post by Matt Newport
Post by Matt Newport
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.
Post by Matt Newport
As you point out below, it will still be possible to do that anyway using
Post by Matt Newport
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.
Post by Matt Newport
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
<https://github.com/isocpp/CppCoreGuidelines/blob/master/docs/Lifetimes%20I%20and%20II%20-%20v0.9.1.pdf>
paper. I think that context is relevant for discussions of the design of
array_view specifically.
Post by Matt Newport
Post by Matt Newport
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).
Post by Matt Newport
Also, the fact that the language converts temporaries to `const&` doesn't
Post by Matt Newport
Post by Matt Newport
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.
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at http://groups.google.com/a/isocpp.org/group/std-discussion/.
Loading...