Taking a reference to a temporary: why illegal

Discussion in 'C++' started by Chris Stankevitz, Mar 7, 2013.

  1. Hello,

    Is there a good reason that the code below is illegal?

    a) No. It's an artifact of some other situation and you get to bear
    the pain by having your cute one-liner rendered illegal

    b) Yes. That code could cause a segfault on hardware due to
    assumptions that the compiler makes about things that are too
    complicated for you to understand.

    c) Well if you don't like it why don't you create your own programming
    language you lazy leech.

    d) This is not the correct forum for this question, now scram before I
    have to get snooty.

    e) [your answer here]


    I'm leaning toward (a).

    Thank you,

    Chris

    //===

    #include <iostream>
    #include <sstream>

    void PrintValues(std::istream& Stream)
    {
    std::string String;

    while (Stream >> String)
    {
    std::cout << String << std::endl;
    }
    }

    int main()
    {
    PrintValues(std::istringstream("Hello, world!"));

    return 0;
    }
     
    Chris Stankevitz, Mar 7, 2013
    #1
    1. Advertising

  2. On 3/6/2013 7:53 PM, Chris Stankevitz wrote:
    > Is there a good reason that the code below is illegal?
    >
    > a) No. It's an artifact of some other situation and you get to bear
    > the pain by having your cute one-liner rendered illegal
    >
    > b) Yes. That code could cause a segfault on hardware due to
    > assumptions that the compiler makes about things that are too
    > complicated for you to understand.
    >
    > c) Well if you don't like it why don't you create your own programming
    > language you lazy leech.
    >
    > d) This is not the correct forum for this question, now scram before I
    > have to get snooty.
    >
    > e) [your answer here]


    It's such an old question that has been asked and answered here and
    elsewhere so many times that it's probably not warranted to repeat the
    answer, and is better to give you another chance to find the proper
    explanation on the Web by yourself. Start with "bind non-const
    reference to a temporary C++" or something the like. Dr. Stroustrup
    himself has given an answer to this one, I believe. Somewhere, and I
    don't recall where. Could be one of his books. Actually, likely.

    >
    >
    > I'm leaning toward (a).


    <shrug> "Cute" has never been the underlying philosophy of C++ design.

    >
    > Thank you,
    >
    > Chris
    >
    > //===
    >
    > #include <iostream>
    > #include <sstream>
    >
    > void PrintValues(std::istream& Stream)
    > {
    > std::string String;
    >
    > while (Stream >> String)
    > {
    > std::cout << String << std::endl;
    > }
    > }
    >
    > int main()
    > {
    > PrintValues(std::istringstream("Hello, world!"));
    >
    > return 0;
    > }
    >


    V
    --
    I do not respond to top-posted replies, please don't ask
     
    Victor Bazarov, Mar 7, 2013
    #2
    1. Advertising

  3. Chris Stankevitz

    Öö Tiib Guest

    On Thursday, 7 March 2013 02:53:38 UTC+2, Chris Stankevitz wrote:
    > Is there a good reason that the code below is illegal?


    e) The feature is very nice in C++. It does not let lot of ugly,
    inefficient and overengineered bloat attempts to compile. For people
    who insist for such bloat there is now far more fun possibility to mess
    with overloads. Overloadst may have right value reference parameter
    among others.

    Just see a monster below taking place of usual 5-line "Hello World!"
    program (its author called it cute? 8-/):

    > #include <iostream>
    > #include <sstream>
    >
    > void PrintValues(std::istream& Stream)
    > {
    > std::string String;
    > while (Stream >> String)
    > {
    > std::cout << String << std::endl;
    > }
    > }
    >
    > int main()
    > {
    > PrintValues(std::istringstream("Hello, world!"));
    > return 0;
    > }
     
    Öö Tiib, Mar 7, 2013
    #3
  4. Chris Stankevitz

    SG Guest

    On Mar 7, 1:53 am, Chris Stankevitz wrote:
    >
    > Is there a good reason that the code below is illegal?


    Yes.

    > void PrintValues(std::istream& Stream)
    > {
    >   std::string String;
    >   while (Stream >> String) {
    >     std::cout << String << std::endl;
    >   }
    > }
    >
    > int main()
    > {
    >   PrintValues(std::istringstream("Hello, world!"));
    >   return 0;
    > }


    Though, I wonder that the "right" way to do this is. And I guess it
    depends on how you look at it. My first thought that came up was to
    add an overload

    inline void PrintValues(std::istream&& Stream)
    { PrintValues(Stream); }

    My second thaught was to only provide a function taking an rvalue
    reference. That would force people to use std::move in case of a named
    stream:

    int main()
    {
    std::istringstream iss ("Hello");
    // #1
    PrintValues(std::move(iss));
    // #2
    PrintValues(std::istringstream("world!"));
    }

    What I like about this is that due to std::move it's more obvious to
    the reader that the state of iss at #2 will probably not be the same
    as before at #1. That's something one might not expect of a function
    with an innocent name like "PrintValues".

    SG
     
    SG, Mar 7, 2013
    #4
  5. On Mar 6, 10:46 pm, Paavo Helde <> wrote:
    > > Is there a good reason that the code below is illegal?

    >
    > > a) No. It's an artifact of some other situation and you get to bear
    > > the pain by having your cute one-liner rendered illegal

    >
    > Yes, it's more like this, and there are non-cute workarounds like adding
    > .seekg(0) to get it compiling.


    Paavo,

    Thank you. I suspected there was "no good reason" for my particular
    use of pass-by-reference to be disallowed.

    I don't see how adding seekg gets my code to compile. I'm not even
    sure where you propose placing seekg but nevertheless I don't see
    anywhere I could place that to make my "original post code" compile.

    Another "non-cute" workaround is to do this:

    void PrintValues(const std::istream& Stream_)
    {
    std::istream& Stream = const_cast<std::istream&>(Stream_);

    // ...


    If this breaks code/compiler/makes my cpu catch on fire, then I won't
    do it. Otherwise, I'll just consider it fighting fire with fire...

    Chris
     
    Chris Stankevitz, Mar 7, 2013
    #5
  6. On Mar 7, 3:24 am, SG <> wrote:
    > My second thaught was to only provide a function taking an rvalue
    > reference. That would force people to use std::move in case of a named
    > stream:


    SG:

    Thank you for your reply. The intent of my question was not so much
    to wonder how I could get my code to compile. It was to understand
    why my particular code was deemed to be illegal by the c++ committee.

    Is my particular code is dangerous?

    Is my particular code safe, but rendered illegal as "collateral
    damage" for some other kind dangerous behavior the c++ committee was
    trying to stop?

    After hearing the arguments I'm still leaning toward the latter.

    Chris
     
    Chris Stankevitz, Mar 7, 2013
    #6
  7. On Mar 7, 12:32 am, Öö Tiib <> wrote:
    > Just see a monster below taking place of usual 5-line "Hello World!"
    > program (its author called it cute? 8-/):


    Ooo:

    My actual code is a little more complicated:

    Serious version that will please the c++ gods:

    const int Aspect = GetAspect(RawPacket);

    switch (Aspect)
    {
    case 0xF0:
    case 0xF1:
    case 0xF2:
    case 0xF3:
    {
    auto pCdo = make_shared<TCCdo>();

    TCCdoDecoder CdoDecoder(Payload);

    TCCdoTraits::GetInstance().Visit(CdoDecoder, *pCdo);

    SignalCdo(static_cast<unsigned char>(Aspect - 0xF0), pCdo);

    break;
    }


    Hideous wild-child perverted version that sullies everything that is
    pure and holy in what amounts to a narcissistic attempt at being cute:

    const int Aspect = GetAspect(RawPacket);

    switch (Aspect)
    {
    case 0xF0:
    case 0xF1:
    case 0xF2:
    case 0xF3:
    {
    auto pCdo = make_shared<TCCdo>();

    TCCdoTraits::GetInstance().Visit(TCCdoDecoder(Payload), *pCdo);

    SignalCdo(static_cast<unsigned char>(Aspect - 0xF0), pCdo);

    break;
    }

    Chris
     
    Chris Stankevitz, Mar 7, 2013
    #7
  8. Chris Stankevitz

    SG Guest

    Am 07.03.2013 20:52, schrieb Chris Stankevitz:
    > On Mar 7, 3:24 am, SG <> wrote:
    >> My second thaught was to only provide a function taking an rvalue
    >> reference. That would force people to use std::move in case of a named
    >> stream:

    >
    > SG:
    >
    > Thank you for your reply. The intent of my question was not so much
    > to wonder how I could get my code to compile. It was to understand
    > why my particular code was deemed to be illegal by the c++ committee.


    I know. Please use your internet search skills for this. The motivation
    for disallowing lvalue references to non-const to bind to rvalues has
    been well-documented.

    > Is my particular code safe, but rendered illegal as "collateral
    > damage" for some other kind dangerous behavior the c++ committee was
    > trying to stop?


    Sort of. There are two use cases for a function that mutates some object
    through a reference. The difference between the two cases is that in one
    case the caller expects the function to mutate some of his/her variables
    in a specific way while in the second use case the caller does not care
    about the object's state anymore. Only in the last case it would be safe
    for the function to mutate temporary objects. In the first case a
    mutation of a temporary object would be an error because the caller has
    no way of accessing that temporary object again. Unexpected implicit
    conversions would probably the main cause of temporary objects in this
    error cases.

    The interesting question I see here is: What's the "right way" to handle
    the second case? There are a couple options:

    - use a single function taking an rvalue reference and requiring
    the caller to use std::move when needed.

    - do overloading on const&/&& so that std::move is not needed

    - write your custom pseudo-reference type that emulates a
    reference that binds to both lvalues and rvalues.

    - ask for the standardization committee to create a new reference
    for this use case.

    Currently I prefer the first option for your case because having to use
    std::move when needed is kind of self-documenting the lack of any
    exceptations w.r.t. the object's state. It might be less error-prone in
    that it avoids some misuses of the function.

    SG
     
    SG, Mar 8, 2013
    #8
  9. On 2013-03-08 04:38, SG wrote:
    >
    > Currently I prefer the first option for your case because having to use
    > std::move when needed is kind of self-documenting the lack of any
    > exceptations w.r.t. the object's state. It might be less error-prone in
    > that it avoids some misuses of the function.


    But nobody is confused just because they wrote "std::cin >> x" instead of
    "std::move(std::cin) >> x", expecting std::cin not to change, are they?
    We all know that some change will happen to the given stream object.

    The OP's example is a typical case of a function taking a stream argument,
    and we're all used to passing named stream objects without std::move.
    Now that we have rvalue references allowing temporary objects to be bound,
    you provide only the rvalue reference version and require named objects
    to be passed with std::move... and suddenly break the expectation users
    have built for over a decade.

    --
    Seungbeom Kim
     
    Seungbeom Kim, Mar 8, 2013
    #9
  10. Chris Stankevitz

    Öö Tiib Guest

    On Thursday, 7 March 2013 22:13:13 UTC+2, Chris Stankevitz wrote:
    > On Mar 7, 12:32 am, Öö Tiib <> wrote:
    > Ooo:
    > My actual code is a little more complicated:
    > Serious version that will please the c++ gods:
    >
    > const int Aspect = GetAspect(RawPacket);


    Why not 'const unsigned'? Or isn't it? If it isn't then most people
    are confused by (valid) signed hexes like '-0xF2'.

    > switch (Aspect)
    > {
    > case 0xF0:
    > case 0xF1:
    > case 0xF2:
    > case 0xF3:


    I would use named constants instead of magic ones.

    > {
    > auto pCdo = make_shared<TCCdo>();
    > TCCdoDecoder CdoDecoder(Payload);


    There is reason why otherwise is not allowed. I explain later.

    > TCCdoTraits::GetInstance().Visit(CdoDecoder, *pCdo);


    TCCdoTraits::Visit() with two parameters? Usually visitor is tons of
    overloads like 'virtual void visit( concretetype& )=0;'. Overloaded
    public virtuals are also dangerous but that is necessary evil with
    visitor that implements virtual hierarchy outside of concrete classes.
    I would expect a coder, decoder or codec to be visitors. So ... I'm
    confused. Likely misuse or misunderstanding of visitor pattern.

    > SignalCdo(static_cast<unsigned char>(Aspect - 0xF0), pCdo);


    I tend to move ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ magic formulas
    to function. Compiler inlines those anyway and the code is more readable.

    >
    > break;
    > }


    The whole language feature is needed to catch errors. People have some
    function like:

    void capitalize(char* str) { /* hopefully obvious */ }

    It causes headache and crashes since callee can't guarantee much here
    and so caller must be careful. Decision is made to take 'std::string&'
    instead.

    void capitalize(std::string& str) { /* hopefully obvious */ }

    That however causes error in some legacy code deep in your code-base that
    I put into main here (in light of your OP example):

    int main()
    {
    char text[] = "hello world!";
    capitalize( text ); // expect it to do the obvious
    std::cout << text << std::endl; // but it can't!!
    }

    With current C++ it is compile-time error. By your suggestion (and with
    MSVC I think) it is run-time error. The cases with something error
    prone (like array/pointer) refactored into something that encapsulates
    and converts from it would lead to same problem.

    You could fight it by forbidding all implicit conversion constructors and
    so all things in standard library (starting from std::string) that
    have implicit conversion constructors. Coding standards suggest to avoid implicit conversions (operators and constructors) rare forbid.
     
    Öö Tiib, Mar 10, 2013
    #10
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Mr. SweatyFinger
    Replies:
    2
    Views:
    1,970
    Smokey Grindel
    Dec 2, 2006
  2. Henning Hasemann
    Replies:
    17
    Views:
    1,146
    mangesh
    Jun 30, 2006
  3. Replies:
    0
    Views:
    1,119
  4. Replies:
    7
    Views:
    3,222
    James Kanze
    Feb 12, 2008
  5. Jim Cain
    Replies:
    1
    Views:
    209
    Yukihiro Matsumoto
    Jul 18, 2003
Loading...

Share This Page