reference to a vector

M

ma740988

I'm perusing source that quite frankly looks like a disaster in the
making. This is a case though where return by reference gets muddled
with my understand based on readings.

struct bar {
double variable;
void init() { memset ( this, 0, sizeof ( bar ) ); }
bar() { init(); }
bool operator <( bar & f ) const
{ return ( variable > f.variable ) ; }
};

std::vector< bar >& run_it ()
{
std::vector< bar > *ptr_f = new std::vector<bar >;
bar f;

f.variable = 99.;
ptr_f->push_back ( f );
return ( *ptr_f );
}

// later
int main()
{
std::vector<bar > f = run_it();
std::cout << f.size() << std::endl;
std::cout << f[ 0 ].variable << std::endl;

}
For starters, the memset seems to make an assumption that bar is a pod
type and it's not, but that aside my question is this:
The vector allocated in run_it is returned by reference to the local
variable f. I think though it's safe to state that the reference (i.e
temporary returned variable) has been reseated and copied to f. This
means the location of the allocated memory has been lost. If my
assessment is correct. I'm surpised the compiler (.NET) allowed me to
reseat ( right terminology ? ) the reference? Even more suprising is I
got the right answer ( 99 ).

I would think
const std::vector<bar>& f = run_it();
would be more prudent and it's now f's responsibility to clean up.

Thanks for the clarification..
 
R

red floyd

ma740988 said:
I'm perusing source that quite frankly looks like a disaster in the
making. This is a case though where return by reference gets muddled
with my understand based on readings.

struct bar {
double variable;
void init() { memset ( this, 0, sizeof ( bar ) ); }
bar() { init(); }
bool operator <( bar & f ) const
{ return ( variable > f.variable ) ; }
};

std::vector< bar >& run_it ()
{
std::vector< bar > *ptr_f = new std::vector<bar >;
bar f;

f.variable = 99.;
ptr_f->push_back ( f );
return ( *ptr_f );
}

// later
int main()
{
std::vector<bar > f = run_it();
std::cout << f.size() << std::endl;
std::cout << f[ 0 ].variable << std::endl;

}
For starters, the memset seems to make an assumption that bar is a pod
type and it's not,

Agreed. It's horrible, and quite possibly UB.

but that aside my question is this:
The vector allocated in run_it is returned by reference to the local
variable f. I think though it's safe to state that the reference (i.e
temporary returned variable) has been reseated and copied to f.

No, ptr_f is a pointer to memory allocated from the free store. The
memory doesn't go away when run_it() exits, and run_it returns a
reference to the vector allocated on the free store. The problem is
that the memory leaks -- there's no way to delete the new'ed vector.
 
B

benben

struct bar {
double variable;
void init() { memset ( this, 0, sizeof ( bar ) ); }
bar() { init(); }
bool operator <( bar & f ) const
{ return ( variable > f.variable ) ; }
};

std::vector< bar >& run_it ()
{
std::vector< bar > *ptr_f = new std::vector<bar >;
bar f;

f.variable = 99.;
ptr_f->push_back ( f );
return ( *ptr_f );
}

// later
int main()
{
std::vector<bar > f = run_it();
std::cout << f.size() << std::endl;
std::cout << f[ 0 ].variable << std::endl;

}
For starters, the memset seems to make an assumption that bar is a pod
type and it's not,

The memory layout is most likely the same as a double so memsetting it
has the same effect of memsetting a double.
Agreed. It's horrible, and quite possibly UB.

I don't see it that way. I think it is just a simple matter made
complicated but the behavior is still well defined (but platform dependent.)

The definition could have been trivially the following:

struct bar {
double variable;
bar():variable(0.0){}
bool operator <( bar & f) const{
return variable > f.variable;
}
};
No, ptr_f is a pointer to memory allocated from the free store. The
memory doesn't go away when run_it() exits, and run_it returns a
reference to the vector allocated on the free store. The problem is
that the memory leaks -- there's no way to delete the new'ed vector.

I agree with you that the memory in OP's code is leaked. But it is
possible, though, to delete that memory. Simply

delete &f;

Regards,
Ben
 
B

Bo Persson

benben said:
The memory layout is most likely the same as a double so memsetting
it has the same effect of memsetting a double.

But you don't know for sure, because the struct is not a POD. The fact
that the struct has a constructor makes it a non-POD, and disqualifies
it from any use of memset on itself.
I don't see it that way. I think it is just a simple matter made
complicated but the behavior is still well defined (but platform
dependent.)

The other problem is that the code assumes that double 0.0 has the
same bit pattern as char 0. This is not guaranteed either.
The definition could have been trivially the following:

struct bar {
double variable;
bar():variable(0.0){}
bool operator <( bar & f) const{
return variable > f.variable;
}
};

Much better!


Bo Persson
 
P

Pete Becker

benben said:
The memory layout is most likely the same as a double so memsetting it
has the same effect of memsetting a double.

If the memory layout is most likely the same as a double, then
memsetting it might have the same effect as memsetting a double.
I don't see it that way. I think it is just a simple matter made
complicated but the behavior is still well defined (but platform
dependent.)

The behavior is undefined. That doesn't mean that it won't do what you
expect. It means that if you do that, you're outside the realm of the
C++ language definition. The danger is twofold: there's the slight
chance that all 0's for a floating-point type isn't 0.0, and there's the
greater change that the compiler puts some internal data in the object
that shouldn't be set to 0. So, in a nearly meaningless sense, it is,
indeed, platform dependent: what happens depends on your compiler. But
it's definitely not well defined.
The definition could have been trivially the following:

The definition SHOULD have been the following:
struct bar {
double variable;
bar():variable(0.0){}
bool operator <( bar & f) const{
return variable > f.variable;
}
};

I'm all in favor of taking advantage of formally undefined behavior in
appropriate cases. In this case it isn't appropriate, because the
well-defined, portable form of initialization works just fine.
I agree with you that the memory in OP's code is leaked. But it is
possible, though, to delete that memory. Simply

delete &f;

NO!!!!

Sorry to shout, but f is a COPY of the object that the returned
reference referred to. It will be destroyed at the end of main. Deleting
it will blow things up horribly, because f was not allocated on the free
store.
 
M

ma740988

Pete Becker wrote:
[...]
Sorry to shout, but f is a COPY of the object that the returned
reference referred to. It will be destroyed at the end of main. Deleting
it will blow things up horribly, because f was not allocated on the free
store.
Pete just so I understand this.
Put another way.
f _does_not_ get a copy but instead is an alias for the object that
the returned reference referred to. Correct?
 
B

benben

Sorry to shout, but f is a COPY of the object that the returned
reference referred to. It will be destroyed at the end of main. Deleting
it will blow things up horribly, because f was not allocated on the free
store.

Code in question:

std::vector< bar >& run_it ()
{
std::vector< bar > *ptr_f = new std::vector<bar >;
bar f;

f.variable = 99.;
ptr_f->push_back ( f );
return ( *ptr_f );
}

Please read carefully. Especially the *RETURN TYPE*.

Now tell me again if the vector ever gets copied.

Regards,
Ben
 
B

benben

Sorry to shout, but f is a COPY of the object that the returned
reference referred to. It will be destroyed at the end of main. Deleting
it will blow things up horribly, because f was not allocated on the free
store.

Oh ya I overlooked. Thanks for the correction!
 
B

benben

Pete just so I understand this.
Put another way.
f _does_not_ get a copy but instead is an alias for the object that
the returned reference referred to. Correct?

Pete was correct. f is copy constructed with a reference (alias)
returned from run_it().

My mistake in my previous post, sorry for that.

The code that allows deletion should be:

int main()
{
std::vector<bar>& f = run_it(); // Note its a reference now.
std::cout << f.size() << std::endl;
std::cout << f[ 0 ].variable << std::endl;
delete &f;
}

But a better way to do the same thing is to return a vector directly:

//std::vector< bar >& run_it ()
std::vector<bar> run_it()
{
std::vector<bar> v;
std::vector<bar> *ptr_f = &v;
bar f;

f.variable = 99.;
ptr_f->push_back ( f );
// return ( *ptr_f );
return v;
}

// later
int main()
{
std::vector<bar> f = run_it();
std::cout << f.size() << std::endl;
std::cout << f[ 0 ].variable << std::endl;

}
 
P

Pete Becker

ma740988 said:
Pete Becker wrote:
[...]

Sorry to shout, but f is a COPY of the object that the returned
reference referred to. It will be destroyed at the end of main. Deleting
it will blow things up horribly, because f was not allocated on the free
store.

Pete just so I understand this.
Put another way.
f _does_not_ get a copy but instead is an alias for the object that
the returned reference referred to. Correct?

f is a copy of the object. Look at its definition. It's an object, not a
reference.
 
P

Pete Becker

benben said:
Please read carefully. Especially the *RETURN TYPE*.

Lest people be confused, the relevant original code was:
std::vector<bar > f = run_it();

f is an object, initialized from a reference returned by the call to
run_it. The initialization copies the object that the reference refers to.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,997
Messages
2,570,239
Members
46,827
Latest member
DMUK_Beginner

Latest Threads

Top