On a forwarding bug

Recently I got caught into a funny bug with forwarding references that I want to share here. In theory, I knew all the principles that led to it, but then it turned out that it was not easy for me to see how they apply precisely and to understand their full extent. Maybe reading this can save a few hours of your time someday.

Let’s start with a code snippet.

  struct element
    {
    int v=0;
    };

    template<class What>
  struct holder
    {
    What what;
    holder(What&& w) : what(w) {}
    };

    template<class T>
  auto make_holder(T&& t) -> auto
    {
    return holder<T>(std::forward<T>(t));
    }

  auto ret_holder1(void) -> auto
    {
    element vl{17};
    auto h = make_holder(std::move(vl));
    // use h
    return h;
    }

  auto ret_holder2(void) -> auto
    {
    element vl{17};
    auto h = make_holder(vl);
    return h;
    }

  auto main(void) -> int
    {
    auto rh1 = ret_holder1();
    auto rh2 = ret_holder2();
    return 0;
    }

This builds and runs, but some of it is broken. Do you feel uneasy why this compiles in the first place? Can you see the bug(s) already?

As a disclaimer, I need to say that I know this code has other serious weaknesses, but I have to admit that I don’t regret writing it, as it turned out to be instrumental in improving my understanding. What I tried to do in this snippet was to produce a function helper make_holder that would leverage the template function type inference and serve as a creator of holder objects, and those objects were meant to be used with rvalue element objects in an immediate context, like inside the function ret_holder1. The constructor of holder accepting only rvalues of the template parameter What was supposed to shield against usage such as in ret_holder2. That it builds at all rings the alarm bell.

The fallacy hidden in the previous paragraph is due to a wrong understanding of what the forwarding reference is. If you casually read the advice on it (for instance http://en.cppreference.com/w/cpp/language/reference#Forwarding_references), you may retain that when a type is inferred through an rvalue parameter, such as in

    template<class T>
  auto make_holder(T&& t) -> auto
    {
    return holder<T>(std::forward<T>(t));
    }

then you may use std::forward as above to do the perfect forwarding and not think about it any more, but when a type is not inferred, such as in

    template<class What>
  struct holder
    {
    What what;
    holder(What&& w) : what(w) {}
    };

then the reference symbols keep their face value.

Life, alas, is more complicated than that. There are two rules of the Standard that intervene in the forwarding process. One is in paragraph 3 of [temp.deduct.call] (n4296) and says (with P denoting the actual argument type and A the inferred argument type):

If P is a cv-qualified type, the top level cv-qualifiers of P’s type are ignored for type deduction. If P is a reference type, the type referred to by P is used for type deduction. A forwarding reference is an rvalue reference to a cv-unqualified template parameter. If P is a forwarding reference and the argument is an lvalue, the type “lvalue reference to A” is used in place of A for type deduction.

The second rule is the reference collapsing rule of paragraph 6 in [dcl.ref] that says

If a typedef-name or a decltype-specifier denotes a type TR that is a reference to a type T, an attempt to create the type “lvalue reference to cv TR” creates the type “lvalue reference to T”, while an attempt to create the type “rvalue reference to cv TR” creates the type TR.

So, technically speaking, only the T&& t argument of make_holder deserves the name of forwarding reference, and the magic works because according to the first rule, the type designated by T will differ depending on the actual parameter, and if the inference establishes T to be an lvalue reference, according to the second rule, the type denoted by T&& will also be an lvalue reference. The helper std::forward also uses this information, see the [forward] section of the Standard.

In the snippet we have, however, used the type T for more than just simple forwarding. The other use is in the holder<T> instantiation. Now that we spelled the rules, we can apply them to understand what happens.

Let’s start with the simpler case. When make_holder is called from ret_holder1, the argument is an rvalue, the type T is inferred as element and the constructor of holder takes element&&, that is, a genuine rvalue of type element, which is correctly forwarded by the call to std::forward in make_holder. The constructor copies the element object into the holder object, in this way, the holders returned from make_holder and from ret_holder1 remain valid in the calling context.

Now, things get more interesting in the second case, when make_holder is called with an lvalue from ret_holder2. The first rule says that T is now element&, an lvalue reference to element. That’s the type with which we instantiate the holder template. There are two problems with that. One is that it is going to contain a reference that will become dangling as soon as the original object goes out of scope, that is, at the return from function ret_holder2. The second, and most interesting, is that the && symbol in its constructor applied on What of type element& will be subject to the reference collapsing rule and will, in fact, result in element&, that is, in an lvalue reference to element. This is why ret_holder2 compiles at all.

The deduction of T on the T&& pattern follows the special rule that has to be remembered. The fact that the perfect forwarding mechanism is split between the two aforementioned prescriptions means that the effects can be distant and not look like forwarding on first sight. What I should have done with the snippet was to remove the reference from T:

      template<class T>
    auto make_holder(T&& t) -> auto
      {      
      return holder<std::remove_reference_t<T>>(std::forward<T>(t));
      }

This brings the && symbol in holder(What&&) to its original meaning and results in ret_holder2 not compiling as intended.

Advertisements

6 Responses to On a forwarding bug

  1. Adam Badura says:

    Shouldn’t you use std::move in holder’s constructor as well? Otherwise a copy will be made. Furthermore, I think you would get a compilation error with your case if you would have used std::move there (without adding std::remove_reference).

    • ljwo says:

      I probably should have, however, in my original code that I wrote in record time cutting every corner the attribute was an lvalue reference, so this would not show up. There are other places here that could indicate problems, like if I did not (ab)use auto at the return from ret_holder2 but tried proper type instead. I said that many times yesterday and today and will put it here: this code is not pretty in any sense and has serious problems other than the bug I write about, but it was a minimal working example showing the disconnect between the two applying rules.

  2. Adam Badura says:

    Another interesting point is picking std::remove_reference as a solution.

    In this case it leads to compilation error. However, are there any cases where it would be useful without making a compilation error?

    If not, then why not instead static_assert for it, probably at class level (for cases where someone uses it without the make_holder). I believe it would lead to more readable and earlier error message.

    • ljwo says:

      remove_reference brings back the type inferred through the special rule for pattern T&& to what it would have been in case of other patterns, like const T& or T* or whatever. That was my rationale for picking this choice here. And this bit of code is transformed from some innards of a would-be library, so user-safety was not priority… Anyway, thanks for both your comments.

  3. abigagli says:

    Interesting post. I’ve myself fallen into this several times, an alternative solution I often use is to define the “what” member in the holder template as
    std::decay_t<What> what;

    This leaves the make_holder function template untouched, and also capable to compile when invoked with both lvalue and rvalue arguments, but holder is guaranteed to always have a copy so no risk of dangling references regardless the value category at the call site.

    • ljwo says:

      Thanks for your comment. Yes, that’s a viable alternative, but the downside would be that one would have objects of two distinct types holder<element> and holder<element&> playing the same role, which might prove problematic if they happen to meet somewhere in code.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Google+ photo

You are commenting using your Google+ account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s

This site uses Akismet to reduce spam. Learn how your comment data is processed.

%d bloggers like this: