On Tue, Nov 10, 2020 at 10:35 AM Nicolai Josuttis via SG20 <sg20@lists.isocpp.org> 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