C++ declares a function instead of calling a complex constructor

Question

First of, I know there are similar questions already on stackoverflow (this, this and this one) and that is why I understand the why of my problem. Unfortunately, that doesn't help me to solve it.

While the above questions are all concerning the default, no-parameter constructor, I have a problem while using a two parameter constructor with default values - I tried to construct an object calling the constructor with only the first value given, and it is parsed as a function declaration instead of an object.

Here are some snippets of my code (I renamed the class names because they're long and not relevant):

class algoContainer{
public:
algoContainer(algo1Virtual &alg1 = algo1Concrete::emptyInstance(),
      algo2Virtual &alg2 = algo2Concrete::instance());

someUsefulFunction();
};

class algo1Concrete : public algo1Virtual{
    private:
    algo1Concrete();
    public:
    static algo1Concrete &emptyInstance(); // "empty" instance treated
                                           // specifically
                                           //  -- uses private no arg constructor
    algo1Concrete(const std::vector<data> &myData); // construcotr
};

class algo1Virtual{
    // ... all functions virtual, no implementations ...
};


// ... similar for algo2Virtual/Concrete ...

All the functions in the Concrete classes are implemented, while none of them in the Virtual classes are (except the constructors and the destructors).

So, my problem now is that I want to do something like:

std::vector <data> workData;
// fill workData
algoContainer myAC(algo1Concrete(workData));
myAC.someUsefulFunction(); // this line gives compile error

Nice, cute and ellegant, but it does not work (error the same as all the questions I linked). I've found this forum-tutorial that does refer to the problem as most vexing parse, but it's solution (put parenthesis around argument) doesn't solve the problem (it's a long bunch of error msgs in that case, but I can edit it in the question later if it helps - those are all related to inheriting a virtual function).

I've tested my code if I use the constructor with all the parameters on default, and even if I just construct the first parameter separately:

std::vector <data> workData;
// fill workData
algo1Concrete myA1(workData);
algoContainer myAC(myA1);

myAC.someUsefulFunction(); // now it works fine

algoContainer myAC2;
myAC2.someUsefulFunction(); // this also works

I can use the code as it is, but it would be greatly appreciated if somebody could give me a more elegant solution to the one I'm using right now.

EDIT: error msgs I get when I fix the most vexing parse

If I use the code with parenthesis:

algoContainer myAC((algo1Concrete(workData)));

My errors are:

/some_path/main.cpp:47:65: error: no matching function for call to ‘algoContainer::algoContainer(algo1Concrete)’
/some_path/main.cpp:47:65: note: candidates are:
/some_path/algo/algocont.h:45:5: note: algoContainer::algoContainer(algo1Virtual&, algo2Virtual&)
/some_path/algo/algocont.h:45:5: note:   no known conversion for argument 1 from ‘algo1Concrete’ to ‘algo1Virtual&’
/some_path/algo/algocont.h:36:7: note: algoContainer::algoContainer(const algoContainer&)
/some_path/algo/algocont.h:36:7: note:   no known conversion for argument 1 from ‘algo1Concrete’ to ‘const algoContainer&’

I renamed the paths and inserted the example file and class names (the same as above) for readability. Just a remark: line 45 is the definition of the constructor in question. On the other hand, line 36 is the line class algoContainer.

I also tried with this code:

algoContainer myDect((algo1Virtual)(algo1Concrete(workData)));

And then the errors are completely different:

/some_path/main.cpp:47:86: error: cannot allocate an object of abstract type ‘algo1Virtual’
/some_path/algo/alg1/algo1virtual.h:31:7: note:   because the following virtual functions are pure within ‘algo1Virtual’:
/some_path/algo/alg1/algo1virtual.h:42:8: note:     virtual algo1Virtual::~algo1Virtual()
/some_path/algo/alg1/algo1virtual.h:39:18: note:    virtual void algo1Virtual::someAlgo1Function(std::vector<data>&)
/some_path/main.cpp:47:87: error: no matching function for call to ‘algoContainer::algoContainer(algo1Virtual)’
/some_path/main.cpp:47:87: note: candidates are:
/some_path/algo/algocont.h:45:5: note: algoContainer::algoContiner(algo1Virtual&, algo2Virtual&)
/some_path/algo/algocont.h:45:5: note:   no known conversion for argument 1 from ‘algo1Virtual’ to ‘algo1Virtual&’
/some_path/algo/algocont.h:36:7: note: algo1Virtual::algo1Virtual(const algo1Virtual&)
/some_path/algo/algocont.h:36:7: note:   no known conversion for argument 1 from ‘algo1Virtual’ to ‘const algo1Virtual&’

Hope this helps.

Can you tell us the error you get from `algoContainer myAC((algo1Concrete(workData)));` with the disambiguating parenthesis? — Mark B, Mar 19 '12 at 14:47
As Kerrek mentioned, this is a *Most Vexing Parse* issue. If you're getting errors when you disambiguate with parens, the error messages will be necessary to help solve the problem. — Justin ᚅᚔᚈᚄᚒᚔ, Mar 19 '12 at 14:49
@Akanksh I know it's a long question, but the last link I mentioned explains the most vexing parse in detail, I knew about it when asking the question. I never explicitly used the words "most vexing parse" in my Q, but now I've edited that in and added it in the tags for the question. — penelope, Mar 19 '12 at 16:16
@penelope Sorry about being preemptive with the link. I think the issue might be due to the algoContainer constructor taking inputs as non-const references, which cannot bind to anonymous temporaries. You would need named variables, or take arguments by rvalue reference, and "move" the algoConcrete into it. Essentially, think about who "owns" the arguments, and what their lifetimes are supposed to be. — Akanksh, Mar 19 '12 at 16:29
@Akanksh I think your comment is actually the most useful here. I just looked up rvalue references and `std::move` and it was what I was looking for... Unfortunately, I can't use it because the group library is still not using *C++11*. If you could type up what you just said in you last command nicely as an answer, I'll accept it. — penelope, Mar 20 '12 at 09:46

score 3 · Answer 1 · answered Mar 19 '12 at 14:52

3

algoContainer myAC(algo1Concrete(workData));

This line is illegal. You cannot bind an rvalue to a mutable lvalue reference. It must be const- and even if it was, the object would die before it could be used by any function except the constructor.

answered Mar 19 '12 at 14:52

Puppy

144,682
38
256
465

+1 you spotted the problem that crops up once the most vexing parse is fixed, even without the error message. – Mark B Mar 19 '12 at 14:56
This seems right on. You must either take the argument by value or (as DeadMG said) by const&. – Jack Morrison Mar 19 '12 at 14:57
@DeadMG I think I do understand what you are saying... And I know the line is illegal, it's what got me to post a question. But the thing is, the way I wish to use this code, the constructed `algo1Concrete` is needed and used only by and in the `algoContainer` it is constructed for, *BUT* the `algo1Concrete` object *is* being changed during usage. I'm interested in another way I could do this in only 1 line. – penelope Mar 19 '12 at 15:15
@penelope You could pass by value into the constructor. Without knowing the specifics of what `algoContainer`'s constructor does there may be other alternatives as well. – Mark B Mar 19 '12 at 15:25
1

@penelope: You have little choice because of the lifetime. You must either dynamically allocate it, or make it on the stack for as long as the `algoContainer` lives. – Puppy Mar 19 '12 at 15:26
@DeadMG unfortunately, I think you're right. I'm gonna sleep on it, see if there are any useful comments and suggestions in the morning and then decide how best to change my code – penelope Mar 19 '12 at 16:13

score 3 · Accepted Answer · edited May 23 '17 at 10:28

The issue seems to be due to the arguments taken by the constructor:

algoContainer( algo1Virtual &alg1,
               algo2Virtual &alg2 );

Note: I have removed the default arguments for brevity.

this takes the arguments as non-const references. So, when you make a call like:

algoContainer myAC(algo1Concrete(workData));

the construction of:

algo1Concrete(workData)

leads to the construction of an anonymous temporary. Anonymous temporaries cannot bind to non-const references, simple because they are temporary and any changes you might make to them will instantly go away (thats not the real reason, but seems to make sense. It doesn't mean anything to modify an anonymous temporary as you have no way to use it later (no name) or eventually (its temporary)). Actually, non-const references can only bind to l-values, and anonymous temporaries are r-values. (Details: Non-const reference may only be bound to an lvalue)

In general, this kind of use means that one wants to give complete ownership of the object being constructed to the function. This can be done by either passing-by-value (expensive), or in C++11, by passing by rvalue reference.

Passing by value will look like:

algoContainer( algo1Virtual alg1,
               algo2Virtual alg2 );

This will result in unnecessary copies.

The other option is to pass by rvalue reference in C++11 like:

algoContainer( algo1Virtual &&alg1,
               algo2Virtual &&alg2 );

Now your first usage will work out of the box:

std::vector <data> workData;
// fill workData
algoContainer myAC(algo1Concrete(workData));
myAC.someUsefulFunction();

but you second usage will need to be modified so that your object is "moved" into the constructor, and the algoContainer takes ownership of the data (the names local variable is then "bad" and should NOT be used at all.

std::vector <data> workData;
// fill workData
algo1Concrete myA1(workData);
algoContainer myAC(std::move(myA1)); //NOTICE THE std::move call.
//myA1 is now a dummy, and unusable as all the internals have gone.
myAC.someUsefulFunction();

For this above example to work, you will have to implement a move constructor for algo1Concrete with the following signature:

algo1Concrete ( algo1Concrete&& other )

which will simply transfer the internals to the current and leave the "other" in an undefined state. (Details: http://msdn.microsoft.com/en-us/library/dd293665.aspx)

NOTE: Regarding default arguments.

I generally suggest that default arguments to functions be avoided, as they lead to more confusion than convenience. All default parameters can be "simulated" simply by overloading the function. Thus, in your case, you would have three ctors:

algoContainer(); //This assumes that the args were both the statics
algoContainer( algo1Virtual alg1 ); //This assumes that arg2 was the static.
algoContainer( algo1Virtual alg1, algo2Virtual alg2 ); //This uses both input.

I agree its more verbose, and not a lot of compilers currently implement inheriting constructors, so we also quite often have copied code. But this will isolate one from a number of debugging/magic value issues which pop up while investigating an issue. But, FWIW, its just an opinion.

when i asked you to expand a little, i didn't even dream about this :) this is awesome, thanks :D — penelope, Mar 20 '12 at 15:40

Kerrek SB · Answer 3 · 2012-03-19T22:29:01.153

0

Write this:

algoContainer myAC((algo1Concrete(workData)));

Then search the internet for "most vexing parse". There are also hundreds of duplicates of this question here on StackOverflow, but they're hard to find, because the problem is never identified in the question itself. (If it were, there'd be no question.)

(After your edit:) Temporary objects (such as those created by a function call that returns by value) do not bind to non-constant references. You need to say:

algo1Concrete ac = algo1Concrete(workData);
algoContainer myAC(ac);

edited Mar 19 '12 at 22:29

answered Mar 19 '12 at 14:45

Kerrek SB

464,522
92
875
1,084

The OP already said that this resulted in a bunch of other errors (although I would expect the parens to be the solution). – Mark B Mar 19 '12 at 14:48
@Kerrek SB I have already included a link in my question explaining the most vexing parse, as I knew about it when I posted it. I didn't explicitly use the words "most vexing parse" in the Q, but I edited those words in now. But still, that is not my main problem. – penelope Mar 19 '12 at 16:18

score 0 · Answer 4 · answered Mar 19 '12 at 14:48

0

As alternatives to the current solution, you can use an extra pair of parentheses, or you can use copy initialization.

answered Mar 19 '12 at 14:48

Cheers and hth. - Alf

142,714
15
209
331

C++ declares a function instead of calling a complex constructor

4 Answers4