C++ Logo

sg20

Advanced search

Re: [SG20] A draft paper to fix the range-based for loop to make it teachable

From: Arthur O'Dwyer <arthur.j.odwyer_at_[hidden]>
Date: Tue, 10 Nov 2020 12:23:03 -0500
On Tue, Nov 10, 2020 at 10:35 AM Nicolai Josuttis via SG20 <
sg20_at_[hidden]> wrote:

> Am 10.11.2020 um 15:38 schrieb Amir Kirsh:
> > The problem is there. No argument. And the paper presents it clearly
> and brilliantly.
> > And it may bite our students.
> > But I would argue that the problem is with using temporaries,
> not specifically with range-based-for.
> >
> > [...] But fixing just the range-based-for may lead to even more
> confusion.
> > That would require additional thought of how to explain things, like:
> >
> > class Person {
> > std::string name;
> > public:
> > Person(const std::string& name): name(name) {}
> > const std::string& getName() const {
> > return name;
> > }
> > ~Person() {
> > name = "dead!";
> > }
> > };
> >
> > int main() {
> > // [a]
> > auto&& name = Person("Mo").getName(); // UB
> > std::cout << name << std::endl; // UB, may print "dead!"
> >
> > // [b]
> > for(char c: Person("Mo").getName()) { // UB
> > std::cout << c;
> > } // UB, may print "dead!", should it become a defined behavior
> > and print "Mo"? while leaving the above UB?
> > }
>

I like this example, and I generally agree with Amir's line of argument
here: Nico, you're proposing to fix one corner-case source of dangling
references, without fixing all sources of dangling references. In this
specific case, I weigh the arguments against each other and come down on
Nico's side overall; but Nico, I think it's important to engage with this
example in your paper. You're shuffling the "weirdness" around — and maybe
overall decreasing it! — but you're not eliminating it completely.

There are three relevant examples here: Amir's [a] and [b], and one more.

    // [a]
    auto&& name = Person("Mo").getName();
    for (char c : name) std::cout << c; // UB

    // [b]
    for (char c : Person("Mo").getName()) std::cout << c; // UB today

    // [c]
    auto loopover = [](auto&& name) { for (char c : name) std::cout << c; };
    loopover(Person("Mo").getName()); // well-defined

Today, [b] behaves like [a]. Nico proposes to make [b] behave like [c]
instead. Notice that *both *[a] and [c] contain `auto&&`. Notice also that
we could rewrite [a] to use decltype(auto) instead of auto&&. In fact, the
compiler's lowering of [b] actually does use decltype(auto), not auto&&.

So the teacher will still have to explain why [a] has UB but [c] doesn't.
So making [b] behave like [c] instead of [a] doesn't really make C++ more
*teachable*, IMO.
But it does fix a potential source of dangling-reference bugs, for
basically zero mental cost. I like it.

[...]

> It seems we all are incredible blind now because we see auto&& when
> using the range-based for loop.


Well, watch out, because there are two places that the student might think
`auto&&` is happening. There's
    for (auto&& x : mySequenceType())
which is still idiomatic IMO (especially in template code), where you
explicitly bind a reference to the *element*.
But then there's the compiler-generated variable bound to the sequence
object *itself*, which is actually decltype(auto), but in practice
generally behaves like auto&&.
If you teach C++17 structured binding, watch out; the student's intuition
will be confused by the similarity between
    for (auto&& x : mySequenceType())
and
    auto&& [x,y] = myProductType();
where the `auto&&` applies to the product object *itself*, and has nothing
to do with anything bound to the individual elements x and y.
I suspect that there's been a little bit of that confusion leaking into
this thread already.

–Arthur

Received on 2020-11-10 11:23:18