Why it does not work when I pass reference to this function?

Question

The code below:

void test(string &s){ // if the argument is "string s", it works
    return test(s+',');
}

The compiler reports cannot find the function: test(std::basic_string).

I think the compiler would create a temporary string (== s+','), and I can pass its reference. But it seems I am wrong. I do not know why I cannot pass the reference of this temporary string.

You are attempting to bind a temporary to a non-const reference. This is not allowed in standard C++. The answer would depend on what you want to achieve. Presumably not a stack overflow. — juanchopanza, Jan 03 '14 at 16:32
just an aside - were you aware that your example (once corrected) would result in infinite recursion? That would cause a stack overflow. — Glenn Teitelbaum, Jan 03 '14 at 17:02

score 2 · Answer 1 · answered Jan 03 '14 at 16:32

2

make it const:

void test(const std::string &s){ // if the argument is "string s", it works
    return test(s+',');
}

answered Jan 03 '14 at 16:32

marcinj

48,511
9
79
100

why I need make it const? what if I want to change the content of s? That would be impossible. – city Jan 03 '14 at 16:42
You would change contents of temporary object, that is not allowed. To modify it change this parameter to std::string s, no reference – marcinj Jan 03 '14 at 16:44

Mike Seymour · Accepted Answer · 2014-01-04T17:14:37.583

2

You can't bind a temporary to a non-constant reference. You could either take the argument by const reference (or, as you point out, by value)

void test(string const & s){ // or string s
    return test(s+',');
}

or use a named variable rather than a temporary

void test(string & s){
    std::string s2 = s + ',';
    return test(s2);
}

_{As noted, at great length, in the comments, this code has undefined runtime behaviour and shouldn't be used in "real" code; it's purpose is just a minimal example of how to fix the observed compilation error}

edited Jan 04 '14 at 17:14

answered Jan 03 '14 at 16:44

Mike Seymour

249,747
28
448
644

Of course this is still broken since `test` doesn't return anything. – Lightness Races in Orbit Jan 03 '14 at 19:45
1

@LightnessRacesinOrbit: Indeed, but the question is just "why doesn't this compile?", and there's no need to complicate the minimal test case by making it recurse correctly. – Mike Seymour Jan 04 '14 at 16:43
Then at best, you've only half-answered the question, since it still doesn't compile. – Lightness Races in Orbit Jan 04 '14 at 16:45
@LightnessRacesinOrbit: What to you mean, doesn't compile? It compiles just fine: http://ideone.com/F9CWQv. Obviously it won't run correctly, since the recursion never ends, but that's beyond the scope of the question. – Mike Seymour Jan 04 '14 at 16:47
@LightnessRacesinOrbit: Oh yes, I didn't spot the `void` that I copied from the question; and neither did my compiler for some reason. Perhaps you could have pointed that out, rather than being an arse about it. Anyway, it's fixed now. – Mike Seymour Jan 04 '14 at 16:51
I don't believe I was "an arse about it". I would have thought you'd already seen it and it seemed to me like you were ignoring it for it "not being relevant". Now, about GCC compiling it: http://stackoverflow.com/q/20923800/560648 – Lightness Races in Orbit Jan 04 '14 at 16:52
@LightnessRacesinOrbit: In fact, having thought a bit more (and seen the answer to your other question), it is perfectly valid to return a `void` expression from a `void` function, so I'll put my answer back how it was. Have a nice day. – Mike Seymour Jan 04 '14 at 16:57
Yeah, it's compilable. It's still _obviously_ not going to behave as the OP intends and you didn't even mention it. – Lightness Races in Orbit Jan 04 '14 at 17:02
@LightnessRacesinOrbit: I've no idea what you're going on about. The OP has provided a minimal test-case for a compiler error, and I've explained how to fix that error. Could we please drop this tedious and irrelevant argument now? – Mike Seymour Jan 04 '14 at 17:04
I'm sorry that you see it as an "argument", or "tedious" or "irrelevant". When executing this code that you have provided to the OP, it will cause (a) a stack overflow, (b) not what the OP wants. There is no data being passed between recursion steps, because the return type of the function is `void`. In my mind, that is _not_ a good answer. If you are satisfied with it then that's great, but I certainly have a right to voice my disagreement with that! For the benefit of the OP. There's no need to get defensive or aggressive with me about it :( – Lightness Races in Orbit Jan 04 '14 at 17:05
@LightnessRacesinOrbit: The question is specifically asking why a the code doesn't compile when you try to pass a temporary by reference; as such, the code is fine as a test-case for that error. It's run-time behaviour is completely irrelevant, which is why I've got no idea why you're spoiling the answer by banging on about it at such length. – Mike Seymour Jan 04 '14 at 17:11
*sigh* Fine. It was so nice talking with you. Next time I'll just keep my improvement suggestions to myself and leave the downvote unexplained. Have a nice day – Lightness Races in Orbit Jan 04 '14 at 17:12
@LightnessRacesinOrbit: I wish I could say the same; you often have a good contribution to make, and I'm genuinely baffled as to why you want the code to be any more than a direct answer to a direct question. Anyway, I've added a note that the code is purely intended to be a test-case for the observed compiler error, just in case anyone else mistakes it for an example of writing recursive functions. – Mike Seymour Jan 04 '14 at 17:16
+1: That's all you had to do, this whole time. Not sure why you felt the need to kick up such a stink about it. – Lightness Races in Orbit Jan 04 '14 at 17:27

brunocodutra · Answer 3 · 2014-01-03T17:42:04.490

0

Standard C++ does not allow a non-const lvalue reference to be bound to an rvalue. In your example, the result of the expression s+',' is a temporary and thus an rvalue, so the compiler is required to discard the overload of test expecting an lvalue reference, leaving it no overload available to call. That's why it complains about not being able to find the function test.

To solve this issue you have to provide an overload whose parameter may be bound to an rvalue. As you realized yourself, expecting an argument by-copy works, but it may imply unnecessary overhead by calling copy/move-ctors. A better option would be to expect the argument by reference. Since const lvalue references may be bound to rvalues, declaring the function as follows solves the problem.

void test(std::string const& s)

edited Jan 03 '14 at 17:42

answered Jan 03 '14 at 16:43

brunocodutra

2,329
17
23

"but it naturally implies unnecessary overhead" - wouldn't move ctor be used here? – marcinj Jan 03 '14 at 16:46
@marcin_j in this particular case, assuming a compiler which implements at least the C++11 standard and does not accomplish copy/move elision, I believe it would. However a move ctor is in itself a greater overhead than no ctor at all, as accomplished by taking references as function parameters. So, even though it may not, it still might imply unnecessary overhead. I'll amend the sentence to make it clear that this is a possibility rather than certainty. – brunocodutra Jan 03 '14 at 17:41

score 0 · Answer 4 · edited May 23 '17 at 10:25

0

But first you should have seen this question String Concatenation

concatenating strings using "+" in c++

Alternative Solution

void test(string  &s)
{
    s.append(",");
    return test(s);
}

edited May 23 '17 at 10:25

Community

1
1

answered Jan 03 '14 at 17:02

Arun Sharma

517
9
16

Why it does not work when I pass reference to this function?

4 Answers4