0

I have a question about lifetime of onstack created arrays and about transforming them to c++ vectors. For ex I have two functions:

void getVector()
{ 
    auto myVector = createVectorFromArray();
}

vector<float> createVectorFromArray()
{
    float myArr[10];
    readDataFromSomewhere(myArr, 10); //some read data function with signature void(float*, size_t)
    vector<float> resVector;
    resVector.assign(myArr, myArr+10);
    return resVector;
}

As I understand array myArr will be killed as soon as we leave function createVectorFromArray. So iterators of vector myVector in function getVector will point to nowhere. Am I right or this works in another way? And how should I better make vector from array to return it from function in this situation?

Thank you!

Daniil Vlasenko
  • 457
  • 3
  • 11

2 Answers2

3

You can rewrite createVectorFromArray like that and you don't have anymore trouble with assigning an array to vector:

vector<float> createVectorFromArray()
{
    vector<float> resVector(10);

    readDataFromSomewhere(&resVector[0], resVector.size());

    return resVector;
}

std::vector is contiguous memory container, so you can use it as interface with plain old C functions

Garf365
  • 3,619
  • 5
  • 29
  • 41
  • This will add a massive overhead, as the vector is first filled with the data and then is copied again for the return... (comments came at the same time, im no downvoter, just a critic) – Nidhoegger Aug 31 '16 at 08:35
  • @Nidhoegger hardly a massive overhead for 10 floats. if it's a concern, just change the initialiser to resVector(10). Then the floats won't be initialised. – Richard Hodges Aug 31 '16 at 08:36
  • Yes, for 10 floats. But whats about 100? or 10000? Its unneccessary to write a function for that... And im not talking about the floats. I am talking about the vector object that copies the floats... – Nidhoegger Aug 31 '16 at 08:37
  • 1
    @Nidhoegger in the current implementation, the floats will be uninitialised. This answer is correct and idiomatic. You're wrong to downvote it. – Richard Hodges Aug 31 '16 at 08:38
  • Have not downvoted, but ideally `vectors` are not meant to be accessed (`&resVector[0]`) in that way. It breaks the framework of abstraction. – sameerkn Aug 31 '16 at 08:39
  • 1
    @RichardHodges: I am just stating that this is a suboptimal approach! Nothing more, nothing less! – Nidhoegger Aug 31 '16 at 08:39
  • 2
    @Nidhoegger you're wrong to say that. It's absolutely optimal. – Richard Hodges Aug 31 '16 at 08:40
  • @sameerkn not agree with you, we find this kind of implementation in "Effective stl" from Scott Meyers – Garf365 Aug 31 '16 at 08:40
  • IMHO its not. Also sameerkn is right. Its not the way to access vectors – Nidhoegger Aug 31 '16 at 08:41
  • 2
    @sameerkn `&vector[0]` does not break encapsulation. A vector's elements are guaranteed by the standard to be contiguous. This use case is idiomatic. – Richard Hodges Aug 31 '16 at 08:41
  • How to return efficiently the vector ? – Garf365 Aug 31 '16 at 08:41
  • 3
    @Garf365 the vector is returned efficiently by RVO. There is no overhead. – Richard Hodges Aug 31 '16 at 08:42
  • @Garf365: It would be "more efficient" to pass a reference of the vector from the point where you call the function rather then creating and copying the vector. – Nidhoegger Aug 31 '16 at 08:42
  • 1
    @RichardHodges, I would not advice users to rely on compiler optimization. Its a nice feature, but its wrong to rely on it... (also IMHO) – Nidhoegger Aug 31 '16 at 08:43
  • @RichardHodges: I was talking about abstraction. – sameerkn Aug 31 '16 at 08:43
  • @Nidhoegger But returning a local variable by reference isn't a good practice. Also, OP doesn't have use c++11 flag, so we can't use move semantic – Garf365 Aug 31 '16 at 08:44
  • 3
    @Nidhoegger RVO is ubiquitous. As of c++17 it is mandated by the standard. You can rely on it. – Richard Hodges Aug 31 '16 at 08:44
  • 1
    @Garf365 RVO happens prior to c++11 – Richard Hodges Aug 31 '16 at 08:44
  • @sameerkn abstraction of vector allows this kind of use, accordign it's own definiton – Garf365 Aug 31 '16 at 08:45
  • @Garf365, this is not what I told you. Pass a reference from outside to that function and manipulate it like: `void createVectorFromArray(vector &res)` – Nidhoegger Aug 31 '16 at 08:45
  • @RichardHodges: as long as you have the right optimization level. And there are not many users out there that use c++17 yet (e.g. which stable linux distributions have a recent GCC suite?) – Nidhoegger Aug 31 '16 at 08:46
  • @Garf365: OP didsn't use C++03 flag neither, so why assuming old standard over the current one ? – Jarod42 Aug 31 '16 at 08:48
  • @Nidhoegger right for prototype, I add this solution to my answer. – Garf365 Aug 31 '16 at 08:48
  • @Garf365: Container `vector` only tell that its a random access and sequential data structure. Whether it stores data in continuous manner is implementation/encapsulation. `vector` provides `operator[]` for indexed/random/sequential data access and this is how it should be accessed via `[]` i.e abstraction. – sameerkn Aug 31 '16 at 08:50
  • @Jarod42: Because its more likely he didnt add a flag to define the standard. So assume the default one. – Nidhoegger Aug 31 '16 at 08:50
  • 1
    @sameerkn totally wrong, vector stores always data in continuous memory, it's the definition of vector abstraction. Standard says that (see http://stackoverflow.com/questions/849168/are-stdvector-elements-guaranteed-to-be-contiguous) – Garf365 Aug 31 '16 at 08:51
  • And what should I do? Previous post answers my question I think, but someone downwoted it. In this post Garf365 shown another approach which also good I think. Why someone downvote previous answer? And what is the better way, an why? – Daniil Vlasenko Aug 31 '16 at 08:52
  • 1
    @DanielVlasenko better way is mine (of course :p)... joking aside, mine will save you a temporary useless array, less copy from one to another container. – Garf365 Aug 31 '16 at 08:54
  • The second approach should be used only in a heavy-duty loop, where the first one would cause performance downgrade due to allocations/deallocations. `std::getline` is an example. – LogicStuff Aug 31 '16 at 08:55
  • 1
    @Nidhoegger: *"default"* is not the same depending of OS/compiler/version. I personally assume last standard unless specified. – Jarod42 Aug 31 '16 at 09:20
  • @Jarod42, then I make a bet that your assumtions are more often wrong than right – Nidhoegger Aug 31 '16 at 09:23
  • @Garf365: Your example will also generate a useless copy in some cases. E.g. when optimization is turned off or you use a compiler without RVO. – Nidhoegger Aug 31 '16 at 09:24
2

This answer is designed to discuss the various approaches, explain them, and put them into context.

option 1: the native to vector copy:

vector<float> createVectorFromArray()
{
    // data copied once
    float myArr[10];
    readDataFromSomewhere(myArr, 10);

    // vectors are lightweight handles. zero-cost construction
    // since the optimiser will see that the next statement assigns to
    // the vector
    vector<float> resVector;

    //
    // data copied twice
    //
    resVector.assign(myArr, myArr+10);

    //
    // it's almost guaranteed that RVO will elide this copy. As of c++17
    // it's a guarantee. Returning the vector is fine
    return resVector;
}

Problems:

  1. data copied twice
  2. vector will require memory allocation

option 2: use the vector directly

vector<float> createVectorFromArray()
{
    // Will allocate space and
    // default-initialise the floats (i.e. they won't be initialised)
    vector<float> resVector(10);

    //
    // one data copy. perfectly safe and idiomatic.
    // std::addressof expressed intent clearly.
    //

    readDataFromSomewhere(std::addressof(resVector[0]), resVector.size());

    //
    // it's almost guaranteed that RVO will elide this copy. As of c++17
    // it's a guarantee. Returning the vector is efficient.
    return resVector;
}

Problems:

  1. vector will require memory allocation

Better...

option 3 : re-use an existing vector

void createVectorFromArray(vector<float>& resVector)
{
    //
    // if resVector has been used before and has a capacity() of 10
    // or more, no memory allocation necessary
    //
    resVector.resize(10);

    // one copy of data
    readDataFromSomewhere(std::addressof(resVector[0]), resVector.size());
}

Problems:

  1. perhaps not quite to easy to use the reference interface.

How would I choose between option 2 and 3?

Option 2 is more readable (IMHO) but would be expensive if used in a loop. So for a one-off, I'd go for that.

If I'm reading data into a buffer in a loop, I'd probably want to avoid un-necessary memory allocations. So re-using the vector's capacity would be a wise move.

e.g.

std::vector<float> buf;
while (thereIsMoreData())
{
    createVectorFromArray(buf);   // option 3
    useTheData(buf);
    // data no longer needed, so we can re-use the vector
}

the alternative:

while (thereIsMoreData())
{
    // the passing of the vector is efficient, particularly as of c++11
    // however, we still suffer one memory allocation per loop.
    // probably undesirable in a high performance environment.
    useTheData(createVectorFromArray());   // option 2
}

Finally...

option 4:

Provide both. Allow the user the performant approach or the 'readable' approach as he/she wishes

void createVectorFromArray(vector<float>& resVector)
{
    //
    // if resVector has been used before and has a capacity() of 10
    // or more, no memory allocation necessary
    //
    resVector.resize(10);

    // one copy of data
    readDataFromSomewhere(std::addressof(resVector[0]), resVector.size());
}

// overload
std::vector<float> createVectorFromArray()
{
    std::vector<float> result;
    createVectorFromArray(result);
    return result;
}
Community
  • 1
  • 1
Richard Hodges
  • 68,278
  • 7
  • 90
  • 142