Non-const lvalue reference error when attempting to use default parameter values across MSVC and Clang Compilers

[mark_in_gr]

I have a function declared as such:

static long ExecuteBinary(const QByteArray & sInputBuffer, QByteArray & sOutputBuffer = QByteArray(), long sTimeout = 2000, bool bLogError = true);

I want to accomplish various things with this function; I need it to be able to modify sOutputBuffer and I also want to have sOutputBuffer set up as a default parameter so I can make calls to the function with only the first param.

Here are two example calls which I need to execute:

InstrumentSource::ExecuteBinary(eso, rso); // Where eso and rso are declared as QByteArray objects prior to the call

InstrumentSource::ExecuteBinary(QByteArray("Do Some Work")); // Where no output QByteArray is needed to be returned.

I would really prefer to have one function to handle both types of calls and continue to use defaults for the second, third and fourth arguments. But based on C++ rules, this causes problems for the second argument. I suppose I could change the structure of the function and return the QByteArray used as output from the function, but I would prefer not to do that.

The interesting part about this is that when compiling this code on a MSVC 2013 compiler on Windows 8, it compiles w/o errors. However, when compiling with Clang or gcc on Mac OS X Mavericks(which appear to be much stricter than MSVC), the complier issues the following error:

"non-const lvalue reference to type 'QByteArray' cannot bind to a temporary of type 'QByteArray'"

Does anyone have any alternatives other than what I have mentioned here to get this to work w/Clang?

[Chris Kawa]

Please use code tags when posting code (easier to read).

A reference as an optional output parameter is not the best idea in general. References are not really good at expressing optionality.

One way is to switch to a pointer and use nullptr as the default argument. I think that is much more clear intention statement and compilers won't complain too.

Btw. In MSVC this is a level 4 warning (the default is level 3) so you need to pass /W4 to the compiler and then you'll get something along the lines of:
[quote]warning C4239: nonstandard extension used :
'default argument' : conversion from 'QByteArray' to 'QByteArray &'
A non-const reference may only be bound to an lvalue[/quote]

[hskoglund]

2nd that, nullptr is the best way to declare the optional parameter.

If you don't want to add a check for the nullptr or change the function signature, another trick you can do is adding a 2nd flavor of your function, i.e.

static long ExecuteBinary(const QByteArray & sInputBuffer);
static long ExecuteBinary(const QByteArray & sInputBuffer, QByteArray & sOutputBuffer, long sTimeout = 2000, bool bLogError = true);

Then for the code in the new flavor of your function, just declare a dummy/throwaway sOutputBuffer and call the original function with sInputBuffer, sOutputBuffer.

EDIT: just realized you have to change the code signature of your original function anyway, the signatures will otherwise be the same for the compiler. So easiest is to make the 2nd argument nonoptional, getting rid of that compiler error as well :-)

[mark_in_gr]

Thank you for the responses. I actually thought of both of those refactoring possibilities(the use of pointers vs. references and creating a second function signature then passing a dummy object to it), but I was hoping to continue to use a reference(for consistency sake, seeing the code is replete with references already) and use it as an output parameter as opposed to returning the value from the function.

To me, the initial error is a bit ambiguous. I understand the differences of using a Reference vs. a Pointer here for purposes of how they differ as far as initialization, however, one would think that the compiler would be able to discern what is attempting to be accomplished here. For example, if I have the following code:

           QByteArray ba;
           foo(ba);

           void foo(QByteArray & returnBa);

This is acceptable. However, this is not:

          void foo(QByteArray & = QByteArray())

Perhaps it's due to how the compiler regards the instantiation of ba in the first example. It's not a temporary value because it's on the stack, whereas the second example isn't regarded the same way.

[Chris Kawa]

This is one of the few places where C++ tries to stop you from shooting yourself in the foot (performance-wise). Non-const references are usually used to modify an object. When you bind a temporary(they are called r-values actually) to that reference it's a potential performance problem, because it's very possible at this point that you will be modifying a temporary object that will be discarded anyway.

You mentioned that compiler distinguishes how a variable is created. That is true indeed, though not related to the memory placement(stack or otherwise). These are called l-values and r-values and they are categorized by syntactic context, not by runtime location. These are further complicated with references.

In C++11 there is a way to recognize temporaries. There's a new type called an r-value reference, and specified by && (called ref-ref). It is a complicated beast but quite simple in simple cases :)
Lucky for you, yours is such a case, so you can also do this:

//shut up compiler, I know it *might* be a temporary and i will handle it myself, thank you very much
void foo(QByteArray&& arr = QByteArray())

It's not really what r-values were introduced for and I would still go for a pointer, but this is as close as you get to your original code.

Although not directly related to this case there is another very important difference between const and non-const references.
A const reference prolongs a lifetime of a temporary object bound to it, so it is destroyed only when the reference goes out of scope. C++ does not give that feature to non-const references:

std::string foo() { return "foo"; };

void bar() {
  const std::string& a = foo();
  std::cout << a;
} //~a() called here

void bar_illegal() {
  std::string& a = foo(); //illegal c++
  std::cout << a; //a would be garbage at this point
}

The second example will work under MSVC by means of that same non-standard extension, but it's not portable.

[ambershark]

Yea this is absolutely a case where you would use a pointer over a reference. In my opinion you should always use a pointer when you modify a variable inside a function. It makes it much clearer to future developers what is intended.

A lot of times others may just assume you forgot to const the definition and not expect their variable to be modified. I would never expect a function to modify a variable I didn't pass as a pointer.

I would change that function proto to be:

static long ExecuteBinary(const QByteArray & sInputBuffer, QByteArray *sOutputBuffer = 0, long sTimeout = 2000, bool bLogError = true);

// this allows calls like you would want i.e.
InstrumentSource::ExecuteBinary(eso, &rso); // Where eso and rso are declared as QByteArray objects prior to the call

InstrumentSource::ExecuteBinary(QByteArray(“Do Some Work”)); // Where no output QByteArray is needed to be returned.

Both your calls would work but all you need to do on call #1 is change 'rso' to '&rso'. This also lets you know for sure that you expect a change to that variable.

It's not against the rules in C++ to use a non-const reference but I think it lends to massive confusion and potential bugs. Sometimes even for the original developer, but definitely for future maintainers.

[Chris Kawa]

It's not against the rules in C++ to use a non-const reference but I think it lends to massive confusion and potential bugs.

The problem here is that it actually is against the rules when you pass it a temporary as a default value. That's what this thread is all about.

Using a non-const reference param is not something bad. In fact a lot of libraries are designed around that because they expect the user to construct the output variable and they return an error code eg.

enum class ErrorCode { OK, NotOk, IWantMyCookie, ... };

ErrorCode fillSomething(Something& buffer);
ErrorCode getSomeState(State& state);

You'll find a lot of this type of code around, especially in C or C-compatible libraries. In these cases the usual assumption is that const ref is an [in] parameter while non-const is [out] parameter. There's nothing wrong with that.

The problem arises when you mix that with default parameters, because C++ forbids binding a non-const reference to a temporary. In that case you can either: create another overload and not use defaults, use pointers, use r-value reference(if using C++11) or a "guard variable" i.e. a global (static?) object that you use as a default wherever you need it.
Pointer seems the cleanest solution IMO and it's also a style Qt happens to use.

[ambershark]

Oh sorry, I should have been more clear when I said that line. It wasn't in context to what he was doing, but in reference to my feelings on passing references vs pointers on variables that change.

So in my case it wouldn't have been against the rules, i.e.

void myFunc(QString &str);
// vs...
void myFunc(QString *str);

// calling...
QString x = "hi";
myFunc(x);
// or...
myFunc(&x);

So what I was saying is both of those are valid but in my opinion the first one can be dangerous and confusing as any new developer (or even the original if he doesn't check his function properly) could be confused at the change in the string 'x' because of the non-const reference.

That is what I was saying isn't against the rules, but that I don't like doing it because it is very confusing.

Sorry for not being more clear. :)

[Chris Kawa]

It's exactly the same with pointers so it's not a real argument.

void foo(QString& s) //can modify s
void foo(QString* s) //also can modify s

void bar(const QString& s) //can't modify s
void bar(const QString* s) //also can't modify s

It's not the ref vs pointer that informs about modifiability. It's the constness. as I said const params are [in], non-const are (potentially) [out]. You should always be explicit about your constness (so called "const-correctness":http://en.wikipedia.org/wiki/Const-correctness).

[ambershark]

Right, it's not the fact that it can modify it's the fact that in a reference pass you have no indication of whether it could be modified or not other than looking at the declaration.

If I invoke a function with xxx(&x); I know that I am passing a pointer and that it could be modified. So by glancing at that code I can see 'x' is a potentially modified variable, and I can look into the function to make sure.

With xxx(x); I would assume a copy or a const reference. I know it's just an assumption. There is no way for me to quickly see that 'x' would be modified and I am stuck having to look at the function.

Like I said it's just personal preference and I find modifiable references to be harder to maintain than a modified pointer.

[ambershark]

Oh, and I agree about constness. I am always very careful about that, especially on refs.

However, something like:

void f(QString s);

Doesn't need const and is not capable of being an [out] parameter. Which is a big reason why I don't assume non-const are [out].

[Chris Kawa]

We're getting off topic here but ok.

First of all I said non-const references, not non-const value copies. There's a huge difference ;)

Well nothing(or very few things) need const to work. Some languages do fine without it. The compiler backend certainly doesn't look at it. It's just an "intention statement" mechanism. Even in this simple case, and even though QString is implicitly shared inside, you're just not clear about your intentions and you pay a potential performance cost. What if copy locks a mutex or does IO dump/readout. You pay. Great example is a QSharedPointer. It's a lot cheaper to pass a ref then a copy, even though pointed data is not copied in any case.
If you want to pass read only stuff your first preference should always be a const& (or a const * if that's how you roll :) ) unless you have a good reason to do otherwise. Exception here being the simple constructor-less types like int or double. But I guess we agree on that.

[ambershark]

I actually agree with 100% on all you've said. :)

I think there was just some confusion on why I preferred pointers for "out" parameters to references. It was just personal preference and didn't have any base in C++, just something I found easier to read and quickly interpret when I maintained code.

As for the topic, mark should have plenty of info (and reasons) to fix his issue now, lol. Probably way too much info.