Taking a reference to a temporary: why illegal

[Chris Stankevitz]

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;
}

 

[Victor Bazarov]

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).

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

 

[Öö Tiib]

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-/):

 

[SG]

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

 

[Chris Stankevitz]

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]

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]

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

 

[SG]

Am 07.03.2013 20:52, schrieb Chris Stankevitz:

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

 

[Seungbeom Kim]

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.

 

[Öö Tiib]

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.