www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - [OT] efficient return values

reply "Lionello Lunesu" <lio remove.lunesu.com> writes:
Hi,

This really should go in a C++ newsgroup, but I've noticed that I have more 
in common with the people here than on a C++ group. Must be all the 
frustrations with C/C++ and the hope for a better tomorrow. More than hope, 
an actual vision ; )

Here's a construct I can't seem to get right in C++. It works, but I hate 
the way I've implemented it. Maybe somebody has a better idea?

It starts like this: I convert some handle into a class to make use of C++'s 
RAII, ctor/dtor. I do this all the time, but today it happens to be for 
sqlite3_get_table / sqlite3_free_table. Needless to say, the handle (char**) 
obtained by sqlite3_get_table must be freed by sqlite3_free_table. The 
wrapper for this table handle I've called SqliteRS (from Result Set):

class SqliteRS
{
  char** table;
  unsigned int cursor, rows, columns;
public:
  SqliteRS(char** t, int c, int r) : table(t), cursor(c), columns(c), 
rows(r) {}
  ~SqliteRS() { sqlite3_free_table(table); }
....
};

And you obtain an SqliteRS by calling Exec of the class SqliteDB, which 
wraps the database handle, sqlite3* (error handling omitted):

SqliteRS SqliteDB::Exec( const char* zSql )
{
  sqlite3_get_table(pdb, zSql, &t, &r, &c, NULL );
  return SqliteRS(t,c,r);
}

All this to be able to do:

SqliteRS results = database.Exec("select * from X");

And everything should be freed nicely.
The problem of course is that the temporary return value will actually be 
copied into the variable "results", causing the table to be freed when the 
temporary return value gets destructed, leaving "results" with an invalid 
handle. This is the problem I'm trying to solve.

It's noteworthy that Visual Studio optimizes this code, even in debug mode! 
There won't be a temporary return value that gets copied to the actual 
variable. However, it won't compile if I create a private copy-ctor, 
eventhough it's never called when it's public.

At the moment I have two ways around this:

1) create a copy ctor that steals the pointer (sets it to NULL in the 
temporary return value); or
2) create a seperate class for the return value and a ctor that takes this 
class in SqliteRS.

1) would be implemented like this:

SqliteRS::SqliteRS( const SqliteRS &st )
{
  memcpy(this,&st,sizeof st);
  const_cast<SqliteRS&>(st).table = NULL;
}

2) is a bit messier, but basically goes like this: SqliteDB::Exec returns an 
_SqliteRS with only the basic information (char**,int,int in this case). 
_SqliteRS has not dtor. SqliteRS becomes a ctor that takes an _SqliteRS, and 
the dtor that frees the handle. For added DBC one could add an assertion in 
the _SqliteRS dtor to assert that it was in fact copied into an SqliteRS, 
but this would require a const_cast<> similar to 1).

Returning handles from other functions / factories happens only too often, 
and wrapping these handles in a class to make use of RAII (ctor/dtor) seems 
the way to go. But why is this contruction so tricky to do nicely? Am I 
forgetting something?

In D the class will be created on the heap and you're returning the handle 
of the class, so there's never a copy involved. One can even use RAII:

class SqliteDB
{
....
  SqliteRS Exec( char[] sql ) { .... return new SqliteRS(...); }
}

auto SqliteRS result = database.Exec("...");

This works just fine.

Lionello.

By the way, has somebody else noticed that I can put an auto variable (the 
RAII kind) in a global handle and it still gets destructed when the 'auto' 
leaves the scope? Shouldn't the compiler complain?
Jan 25 2006
next sibling parent reply Oskar Linde <oskar.lindeREM OVEgmail.com> writes:
Lionello Lunesu wrote:

 Hi,
 
 This really should go in a C++ newsgroup, but I've noticed that I have more 
 in common with the people here than on a C++ group. Must be all the 
 frustrations with C/C++ and the hope for a better tomorrow. More than hope, 
 an actual vision ; )
 
 Here's a construct I can't seem to get right in C++. It works, but I hate 
 the way I've implemented it. Maybe somebody has a better idea?
 
 It starts like this: I convert some handle into a class to make use of C++'s 
 RAII, ctor/dtor. I do this all the time, but today it happens to be for 
 sqlite3_get_table / sqlite3_free_table. Needless to say, the handle (char**) 
 obtained by sqlite3_get_table must be freed by sqlite3_free_table. The 
 wrapper for this table handle I've called SqliteRS (from Result Set):
 
 class SqliteRS
 {
   char** table;
   unsigned int cursor, rows, columns;
 public:
   SqliteRS(char** t, int c, int r) : table(t), cursor(c), columns(c), 
 rows(r) {}
   ~SqliteRS() { sqlite3_free_table(table); }
 ....
 };
 
 And you obtain an SqliteRS by calling Exec of the class SqliteDB, which 
 wraps the database handle, sqlite3* (error handling omitted):
 
 SqliteRS SqliteDB::Exec( const char* zSql )
 {
   sqlite3_get_table(pdb, zSql, &t, &r, &c, NULL );
   return SqliteRS(t,c,r);
 }
 
 All this to be able to do:
 
 SqliteRS results = database.Exec("select * from X");
 
 And everything should be freed nicely.
 The problem of course is that the temporary return value will actually be 
 copied into the variable "results", causing the table to be freed when the 
 temporary return value gets destructed, leaving "results" with an invalid 
 handle. This is the problem I'm trying to solve.



 At the moment I have two ways around this:
 
 1) create a copy ctor that steals the pointer (sets it to NULL in the 
 temporary return value); or
 2) create a seperate class for the return value and a ctor that takes this 
 class in SqliteRS.


[snip]


 Returning handles from other functions / factories happens only too often, 
 and wrapping these handles in a class to make use of RAII (ctor/dtor) seems 
 the way to go. But why is this contruction so tricky to do nicely? Am I 
 forgetting something?


This is what std::auto_ptr<> is perfect for. An auto_ptr will, using 
your words, steal the reference being assigned to it. A auto_ptr is 
perfect when you want to transfer responsibility.


 In D the class will be created on the heap and you're returning the handle 
 of the class, so there's never a copy involved. One can even use RAII:
 
 class SqliteDB
 {
 ....
   SqliteRS Exec( char[] sql ) { .... return new SqliteRS(...); }
 }
 
 auto SqliteRS result = database.Exec("...");


Yes, but with this, the user needs to remember to make the reference 
auto. There is (AFAIK) no way in D to make this automatic like the c++ 
auto_ptr does. If D had more power (assignment overloading, struct 
destructors/constructors), an auto_ptr would be straight-forward to 
implement in D too.

/Oskar
Jan 25 2006
next sibling parent "Lionello Lunesu" <lio remove.lunesu.com> writes:
 auto SqliteRS result = database.Exec("...");


 Yes, but with this, the user needs to remember to make the reference auto. 
 There is (AFAIK) no way in D to make this automatic like the c++ auto_ptr 
 does. If D had more power (assignment overloading, struct 
 destructors/constructors), an auto_ptr would be straight-forward to 
 implement in D too.


Maybe the prototype of the Exec function could already mention the 'auto'?

auto SqliteRS Exec( char[] sql ) { ... }

At the moment the compiler (correctly) complains with "functions cannot be 
const or auto".

This kind of "auto" could also be used to force the caller to catch the 
return value in a variable. I guess exceptions can be used to prevent the 
caller from ignoring the result value.

Exec("select * from X"); // not allowed; use ExecNonQuery

Lio.
Jan 25 2006
prev sibling next sibling parent Sean Kelly <sean f4.ca> writes:
Oskar Linde wrote:

 
 This is what std::auto_ptr<> is perfect for. An auto_ptr will, using 
 your words, steal the reference being assigned to it. A auto_ptr is 
 perfect when you want to transfer responsibility.


Yes, but it's not perfect if you wish to simply store a reference to 
something as a class member, because auto_ptr performs a destructive 
copy.  Just something to be aware of for those who haven't used auto_ptr 
before.


Sean
Jan 25 2006
prev sibling parent reply Bruno Medeiros <daiphoenixNO SPAMlycos.com> writes:
Oskar Linde wrote:

 Lionello Lunesu wrote:
 

 Hi,

 This really should go in a C++ newsgroup, but I've noticed that I have 
 more in common with the people here than on a C++ group. Must be all 
 the frustrations with C/C++ and the hope for a better tomorrow. More 
 than hope, an actual vision ; )

 Here's a construct I can't seem to get right in C++. It works, but I 
 hate the way I've implemented it. Maybe somebody has a better idea?

 It starts like this: I convert some handle into a class to make use of 
 C++'s RAII, ctor/dtor. I do this all the time, but today it happens to 
 be for sqlite3_get_table / sqlite3_free_table. Needless to say, the 
 handle (char**) obtained by sqlite3_get_table must be freed by 
 sqlite3_free_table. The wrapper for this table handle I've called 
 SqliteRS (from Result Set):

 class SqliteRS
 {
   char** table;
   unsigned int cursor, rows, columns;
 public:
   SqliteRS(char** t, int c, int r) : table(t), cursor(c), columns(c), 
 rows(r) {}
   ~SqliteRS() { sqlite3_free_table(table); }
 ....
 };

 And you obtain an SqliteRS by calling Exec of the class SqliteDB, 
 which wraps the database handle, sqlite3* (error handling omitted):

 SqliteRS SqliteDB::Exec( const char* zSql )
 {
   sqlite3_get_table(pdb, zSql, &t, &r, &c, NULL );
   return SqliteRS(t,c,r);
 }

 All this to be able to do:

 SqliteRS results = database.Exec("select * from X");

 And everything should be freed nicely.
 The problem of course is that the temporary return value will actually 
 be copied into the variable "results", causing the table to be freed 
 when the temporary return value gets destructed, leaving "results" 
 with an invalid handle. This is the problem I'm trying to solve.


 
 

 At the moment I have two ways around this:

 1) create a copy ctor that steals the pointer (sets it to NULL in the 
 temporary return value); or
 2) create a seperate class for the return value and a ctor that takes 
 this class in SqliteRS.


 
 
 [snip]
 

 Returning handles from other functions / factories happens only too 
 often, and wrapping these handles in a class to make use of RAII 
 (ctor/dtor) seems the way to go. But why is this contruction so tricky 
 to do nicely? Am I forgetting something?


 
 
 This is what std::auto_ptr<> is perfect for. An auto_ptr will, using 
 your words, steal the reference being assigned to it. A auto_ptr is 
 perfect when you want to transfer responsibility.
 

 In D the class will be created on the heap and you're returning the 
 handle of the class, so there's never a copy involved. One can even 
 use RAII:

 class SqliteDB
 {
 ....
   SqliteRS Exec( char[] sql ) { .... return new SqliteRS(...); }
 }

 auto SqliteRS result = database.Exec("...");


 
 
 Yes, but with this, the user needs to remember to make the reference 
 auto. There is (AFAIK) no way in D to make this automatic like the c++ 
 auto_ptr does. 
 
 /Oskar


What do you mean "automatic like the c++ auto_ptr" ? What exactly are 
the differences? In C++ you must also make the reference an auto_ptr,no?

-- 
Bruno Medeiros - CS/E student
"Certain aspects of D are a pathway to many abilities some consider to 
be... unnatural."
Jan 31 2006
parent Oskar Linde <oskar.lindeREM OVEgmail.com> writes:
In article <dro5lt$14bf$1 digitaldaemon.com>, Bruno Medeiros says...


Oskar Linde wrote:

 auto SqliteRS result = database.Exec("...");


 
 
 Yes, but with this, the user needs to remember to make the reference 
 auto. There is (AFAIK) no way in D to make this automatic like the c++ 
 auto_ptr does. 
 
 /Oskar





What do you mean "automatic like the c++ auto_ptr" ? What exactly are 
the differences? In C++ you must also make the reference an auto_ptr,no?


Yes, but you can not by mistake make it a SqliteRS * if the function is defined
to return an auto_ptr<SqliteRS>. In D, if you forget the auto, everything will
compile and run, but the resources are not guaranteed to be freed.

/Oskar
Jan 31 2006
prev sibling next sibling parent Sean Kelly <sean f4.ca> writes:
Lionello Lunesu wrote:

 
 Returning handles from other functions / factories happens only too often, 
 and wrapping these handles in a class to make use of RAII (ctor/dtor) seems 
 the way to go. But why is this contruction so tricky to do nicely? Am I 
 forgetting something?


A lot of it has to do with the messed-up way the ODBC API was 
written--it just doesn't map well into an object model.  What I've done 
in the past is store the handles in an RAII-type object as you are:

class db_handle_ptr :
private	noncopyable
{
public:
	SQLHDBC	hdbc;
	db_handle_ptr() : hdbc( SQL_NULL_HDBC ) {}
	~db_handle_ptr() {
             ::SQLDisconnect( hdbc );
             ::SQLFreeHandle( SQL_HANDLE_DBC, hdbc );
         }
};

And then pass around references to it using a shared pointer (similar to 
the one available in Boost).  I get around returning handles at all by 
instead returning a temporary object that is only accessible by the 
destination object:

class resultset :
private	noncopyable
{
public:
     class resultset_init
     {
     public:
         resultset_init( shared_ptr<db_handle_ptr> const& hdb,
                         shared_ptr<stmt_handle_ptr> const& hs );

     private:
                 friend resultset;

                 shared_ptr<db_handle_ptr>    m_db;
                 shared_ptr<stmt_handle_ptr>  m_stmt;
     };

     resultset( resultset_init const& init );

private:
     shared_ptr<db_handle_ptr>   m_db;
     shared_ptr<stmt_handle_ptr> m_stmt;
};

class statement :
private noncopyable
{
public:
     resultset::resultset_init open();
};

The end result is that all the handles that need to stay alive do so 
until all dependent objects are destroyed.  So even if the connection 
object is deleted, any open statements or resultsets will remain usable.


Sean
Jan 25 2006
prev sibling next sibling parent reply "Lionello Lunesu" <lio remove.lunesu.com> writes:
When I think about it....

Shouldn't an ignored return value be treated as an error?

Or at least a warning, or an assertion (like forgetting to return 
something). Maybe adding a construction like:

void = I_dont_care_about_the_return_value();

L.
Jan 27 2006
parent reply Don Clugston <dac nospam.com.au> writes:
Lionello Lunesu wrote:

 When I think about it....
 
 Shouldn't an ignored return value be treated as an error?
 
 Or at least a warning, or an assertion (like forgetting to return 
 something). Maybe adding a construction like:
 
 void = I_dont_care_about_the_return_value();
 
 L. 


The problem is, not all return values matter.
There are lots of APIs which have some really stupid return values. The 
classic example being printf().

void = printf("hello world\n");

No thanks.
I think you'd need some kind of "must use" syntax on a function 
declaration, which would trigger an error if it wasn't used. Maybe a 
"nonvoid" keyword.

But I don't think this is a very common programming error. Not really 
worth worrying about.
Jan 27 2006
parent reply "Lionello Lunesu" <lio remove.lunesu.com> writes:
 void = printf("hello world\n");


You shouldn't be using printf to begin with : S

But I agree, not a common mistake. I was thinking about the waste of CPU 
time when somebody discards return codes. Fits nicely with D's the notion of 
"statement must have side effect", for example a "{strlen(a);}" compiles 
fine, but is a waste of time.

I made the following template when I noticed that many colleagues were 
ignoring the return value of some Lock() call:

//! Assert that a (return) value gets used (used for handles)
template <typename T>
class _use
{
  const T retval;
  bool unused_return_value;
public:
  _use( const T& rv ) : retval(rv), unused_return_value(false) {}
  ~_use() { assert(unused_return_value); }
  operator T () { unused_return_value=true; return retval; }
};

I defined the call as _use<bool> Lock() and it now asserts when the return 
value is not caught into some variable or expression. I know, I should have 
used C++ exceptions, which I hope to do when we get more time on our 
hands... (probably never :S)

Lio.
Jan 27 2006
parent reply Don Clugston <dac nospam.com.au> writes:
Lionello Lunesu wrote:

 
 I made the following template when I noticed that many colleagues were 
 ignoring the return value of some Lock() call:
 
 //! Assert that a (return) value gets used (used for handles)
 template <typename T>
 class _use
 {
   const T retval;
   bool unused_return_value;
 public:
   _use( const T& rv ) : retval(rv), unused_return_value(false) {}
   ~_use() { assert(unused_return_value); }
   operator T () { unused_return_value=true; return retval; }
 };
 
 I defined the call as _use<bool> Lock() and it now asserts when the return 
 value is not caught into some variable or expression. 



I know, I should have

 used C++ exceptions, which I hope to do when we get more time on our 
 hands... (probably never :S)


I'm not sure about that. This is a logical error, and ideally it would 
be caught at compile time. I think an assert is the next best option.
(Because even if the exception is caught, there's still a logic error).

I'm a little biased against exceptions, because the worst bug I've ever 
encountered in my life was caused by a destructor which threw an 
exception, inside a multi-threaded program. (There were virtually no 
symptoms, that single thread just silently disappeared - yikes).
Jan 27 2006
next sibling parent reply Sean Kelly <sean f4.ca> writes:
Don Clugston wrote:

 
 I'm a little biased against exceptions, because the worst bug I've ever 
 encountered in my life was caused by a destructor which threw an 
 exception, inside a multi-threaded program. (There were virtually no 
 symptoms, that single thread just silently disappeared - yikes).


That in itself is basically an illegal operation in C++, since throwing 
an exception while another is in flight terminates the program.  If in 
doubt, wrap the contents of your dtor in a try{}catch(...){} block, and 
report the error by some other means.  I'll admit I do like that D 
doesn't have this problem, even if it means that some exceptions are 
overridden by others and are therefore never reported.  I've wondered 
whether exception chaining via the .next property might be right for 
this purpose, but it's not a perfect fit.


Sean
Jan 27 2006
parent Don Clugston <dac nospam.com.au> writes:
Sean Kelly wrote:

 Don Clugston wrote:
 

 I'm a little biased against exceptions, because the worst bug I've 
 ever encountered in my life was caused by a destructor which threw an 
 exception, inside a multi-threaded program. (There were virtually no 
 symptoms, that single thread just silently disappeared - yikes).


 
 
 That in itself is basically an illegal operation in C++, since throwing 
 an exception while another is in flight terminates the program.


I know. The problem in this case was, (a) the destructor was calling a 
function which called another function (written by someone else) which 
eventually threw a destructor (it was not documented that it could 
throw), and
(b) if it terminated the program, that would be OK. In this case, it 
only terminated the thread. Causing a sequence of further disasters.

   If in

 doubt, wrap the contents of your dtor in a try{}catch(...){} block, and 
 report the error by some other means.


I learnt from this to _always_ be in doubt. It left me with in serious 
doubt of the merits of RAII when used with exceptions. It means that any 
kind of cleanup operation cannot use exceptions to report errors.


   I'll admit I do like that D

 doesn't have this problem, even if it means that some exceptions are 
 overridden by others and are therefore never reported.  I've wondered 
 whether exception chaining via the .next property might be right for 
 this purpose, but it's not a perfect fit.
 
 
 Sean
Jan 28 2006
prev sibling parent "Lionello Lunesu" <lio remove.lunesu.com> writes:
 I know, I should have
 used C++ exceptions, which I hope to do when we get more time on our 
 hands... (probably never :S)


 I'm not sure about that. This is a logical error, and ideally it would be 
 caught at compile time. I think an assert is the next best option.
 (Because even if the exception is caught, there's still a logic error).


I mean in the case of such a Lock call, where a return value of "false" 
means the lock failed and you shouldn't do anything with the object. That's 
way to important to do with a return value and should be an exception that 
can't simply be forgotten about.


 I'm a little biased against exceptions, because the worst bug I've ever 
 encountered in my life was caused by a destructor which threw an 
 exception, inside a multi-threaded program. (There were virtually no 
 symptoms, that single thread just silently disappeared - yikes).


Simple rule: never throw in a dtor :-)

Lio.
Jan 27 2006
prev sibling parent "Lionello Lunesu" <lio remove.lunesu.com> writes:
I'm still not clear on this issue. Please tell me how you people solve the 
"using RAII for returned handles" problem.

Do you use...

a) method 1 from my original post: a wrapper with a copy ctor that steals 
the pointer
b) method 2 from my original post: a separate class to temporarily wrap the 
return value
c) std::auto_ptr<> and therefor a wrapped pointer on the heap
d) making the factory class a friend of the wrapper class

...there are probably even more ways to do this thing. RAII (however bad 
that name may be) is the best thing that OO languages have to offer (if you 
ask me) but it frustrates me that I can't find a nice solution for 
transfering handle-ownership from a factory the handle-wrapper.

Lionello.
Feb 01 2006