Create an object by paasing to the constructor other object reference

Question

I created in an include.hpp two C++ classes:

#include <string>
#include <map>
#include <iostream>
#include <sstream>
#include<vector>
using namespace std;

class A {
        public:
                vector<char> v;
                A(){};
                A(A &other) {
                    v = other.v;
                }
                A(string s) {
                    v.assign(s.begin(), s.end());
                }

                A& operator << (std::string s) {
                    v.insert(v.end(), s.begin(), s.end());
                    return(*this);
                };
};

class B {
        protected:
                A a;
        public:
                template<typename T>
                B(T &t): a(t) {}
};

I need now to create an instance by entering to its constructor an A object. In addition the A object has to be a returned value of an other function that creates the returned A instance with the operator <<

I'm trying with the following main:

#include<iostream>
#include "include.hpp"

using namespace std;

A& createstrbyappend() {
        A a;
        a << "aa";
        return a;
}

int main() {
        A aa;
        aa << "kkk";
        cout << "create b1 object " << endl;
        B* b1 = new B(aa);
        cout << "create b2 object " << endl;
        B* b2 = new B(createstrbyappend());
        cout << "End program " << endl;
}

But I get a segmentation fault with this main, I get as output:

create b1 object 
create b2 object 
Segmentation fault (core dumped)

b1 is created successfully, but b2 that's created through the function createstrbyappend gives the segmentation fault.

Is there a suggestion how I can fix the issue ?

EDIT: add compiler version

I'm compiling with g++ command under ubuntu. my g++ version is the following:

$ g++ --version
g++ (Ubuntu 5.4.0-6ubuntu1~16.04.12) 5.4.0 20160609
Copyright (C) 2015 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

cigien · Accepted Answer · 2020-05-12T21:04:30.283

2

You are returning a function local object by reference from createstrbyappend. You should return a copy instead:

A createstrbyappend() {
 //...
}

This requires you to take the argument to Bs constructor by value as well:

template<typename T>
B(T t): a(std::move(t)) {}

Now, you need to take the argument to A's copy constructor by const&:

A(A const &other) {
   v = other.v;
}

Here's a working demo.

edited May 12 '20 at 21:04

answered May 12 '20 at 17:49

cigien

57,834
11
73
112

or `const T& t` but yes by-value is better (and do `a(std::move(t))`) – Asteroids With Wings May 12 '20 at 17:50
That requires changing the constructor of `A` as well. Trying to make minimal changes to get it working. – cigien May 12 '20 at 17:52
It doesn't require anything (though doing that would complete the optimisation yes) – Asteroids With Wings May 12 '20 at 17:53
In general do a move to signal when it's applicable even if it won't do anything (unless it's on an `int` or something) so future changes elsewhere can "make it work". – Asteroids With Wings May 12 '20 at 17:54
@cigien Thank you for your answer but I got this compiling error: error: invalid initialization of non-const reference of type ‘A&’ from an rvalue of type ‘A’ B* b2 = new B(createstrbyappend()); – Kallel Omar May 12 '20 at 19:26
@KallelOmar Did you change the constructor of `B` as the answer says? – cigien May 12 '20 at 20:25
@cigien: yes I did but I got the compiling error. Then I take your demo code https://godbolt.org/z/a_X2fk as it in just one cpp file and I compile but I got the same error. Seems like this error is related to g++ compiler version? – Kallel Omar May 12 '20 at 20:55
I have just made an update to my question – Kallel Omar May 12 '20 at 20:56
@cigien just done – Kallel Omar May 12 '20 at 21:02
@KallelOmar Updated the solution. – cigien May 12 '20 at 21:04
@cigien : it works in this small program, hope that I don't find issues if I apply this solution in my project especially A and B classes are used and called many times in my project – Kallel Omar May 12 '20 at 21:18
Depends. There may be more issues that are not visible here. Good luck :) – cigien May 12 '20 at 21:21
@cigien Thank you very much it works in my project also, just no need of std::move ;) – Kallel Omar May 12 '20 at 21:48
`std::move` is an optimization, it's never *needed*. But I'm glad it worked out. – cigien May 12 '20 at 21:51

Create an object by paasing to the constructor other object reference

1 Answers1