A scoped_ptr class that does not allow NULL ?

[Tim H]

Does such a thing exist? It seems handy for self-documenting, at
least.

I whipped this up. Am i missing something?



struct NullPointerError: public std::runtime_error
{
explicit NullPointerError(const std::string &str)
: runtime_error(str)
{
}
};

// This is a scoped_ptr<> that can never be NULL.
template<typename Tptr>
class NeverNullScopedPtr : public boost::scoped_ptr<Tptr>
{
private:
// This allows us to strip the const off of Tptr, if present.
This
// allows us to have const and non-const ctors.
template<typename T>
struct UnConst {
typedef T type;
};
template<typename T>
struct UnConst<const T> {
typedef T type;
};

public:
NeverNullScopedPtr(typename UnConst<Tptr>::type *pointer)
: boost::scoped_ptr<Tptr>(pointer)
{
if (pointer == NULL) {
throw NullPointerError(
"NeverNullScopedPtr initialized to NULL");
}
}
NeverNullScopedPtr(const typename UnConst<Tptr>::type
*pointer)
: boost::scoped_ptr<Tptr>(pointer)
{
if (pointer == NULL) {
throw NullPointerError(
"NeverNullScopedPtr initialized to NULL");
}
}
// This catches static initializations to NULL.
template<typename T>
NeverNullScopedPtr(T catches_null)
{
// This symbol does not exist, but makes for better
errors.
dont_init_NeverNullScopedPtr_to_NULL(catches_null);
}

void
reset(typename UnConst<Tptr>::type *pointer)
{
if (pointer == NULL) {
throw NullPointerError(
"NeverNullScopedPtr reset to NULL");
}
boost::scoped_ptr<Tptr>::reset(pointer);
}
void
reset(const typename UnConst<Tptr>::type *pointer)
{
if (pointer == NULL) {
throw NullPointerError(
"NeverNullScopedPtr reset to NULL");
}
boost::scoped_ptr<Tptr>::reset(pointer);
}
// This catches static resets to NULL.
template<typename T>
void reset(T catches_null)
{
// This symbol does not exist, but makes for better
errors.
dont_reset_NeverNullScopedPtr_to_NULL(catches_null);
}

void
swap(NeverNullScopedPtr &other)
{
boost::scoped_ptr<Tptr>::swap(other);
}

// This prevents swap() with NULL. It's never defined.
void
swap(boost::scoped_ptr<Tptr> &other);
};

 

[Öö Tiib]

Does such a thing exist?  It seems handy for self-documenting, at
least.

I whipped this up.  Am i missing something?

<snip implementation>

Why you need it not to be NULL? NULL is important value of pointer.
Typical usage:

class Bar
{
public:
Bar();
beTricky();
};

void foo()
{
boost::scoped_ptr<Bar> BarPtr; // is NULL

try
{
// can't declare BarPtr here, otherwise would
// be destroyed at end of block
BarPtr.reset( new Bar() );
}
catch( ... )
{
// error handling, not sure it rethrows
// and may result BarPtr remains NULL
}

if ( BarPtr )
BarPtr->beTricky();
}

IOW i do not understand usage of pointers that can not be NULL. Can
you elaborate?

 

[Tim H]

<snip implementation>

Why you need it not to be NULL? NULL is important value of pointer.
Typical usage:

 class Bar
 {
 public:
    Bar();
    beTricky();
 };

 void foo()
 {
    boost::scoped_ptr<Bar> BarPtr; // is NULL

    try
    {
       // can't declare BarPtr here, otherwise would
       // be destroyed at end of block
       BarPtr.reset( new Bar() );
    }
    catch( ... )
    {
       // error handling, not sure it rethrows
       // and may result BarPtr remains NULL
    }

    if ( BarPtr )
       BarPtr->beTricky();
 }

IOW i do not understand usage of pointers that can not be NULL. Can
you elaborate?

I have a lot of classes that take ownership of pointers (part of a
syntax tree for a language grammar). Those pointers can not be NULL,
or else the classes do not work.

I found myself littering the code with either asserts or with tests
for NULL, or else I kept referring back to the grammar to see if NULL
was viable (in some cases it is, in many it is not). I found myself
wanting a scoped_ptr class member that gets initialized from the ctor
and is very clear about ownership and expectations.

Now, I just look at the data member and it is
"NeverNullScopedPointer<Expression> m_expr;" and I know that the
expression is required to be non-NULL.

Adding the extra traps for NULL literals actually caught one place
where I had screwed up, and making the ctor arg not have a default
value of NULL caught 2 places where I had forgotten to init a pointer.

So that was my use case. I was just wondering if there was already
something like this or if this was actually useful. Also hoping to
get feedback on any screw-ups I made.

FWIW: I also made a MaybeNullScopedPtr<T> class which is just
boost::scoped_ptr.

Tim

 

[Öö Tiib]

I have a lot of classes that take ownership of pointers (part of a
syntax tree for a language grammar).  Those pointers can not be NULL,
or else the classes do not work.

I found myself littering the code with either asserts or with tests
for NULL, or else I kept referring back to the grammar to see if NULL
was viable (in some cases it is, in many it is not).  I found myself
wanting a scoped_ptr class member that gets initialized from the ctor
and is very clear about ownership and expectations.

Now, I just look at the data member and it is
"NeverNullScopedPointer<Expression> m_expr;" and I know that the
expression is required to be non-NULL.

Understandable. When member scoped_ptr may not be NULL you need it to
be checked in constructors and where it is reset().

However when dealing with parsing trees the error messages are most
important thing. Usualy it is written by human what you parse and
human always demands detailed explanations how fool he is.

I would end up (when using your NeverNullScopedPointer) catching
"NeverNullScopedPtr reset to NULL" and "NeverNullScopedPtr initialized
to NULL" asap and then forming readable explanations what went wrong.
That results with double work since if i did put explicit NULL checks
(into constructors/reset locations) i could immediately throw useful
error messages/codes from there. Also if constructor initializes
multiple NeverNullScopedPtr i have to make the checks each time before
calling it otherwise how to make difference between two
"NeverNullScopedPtr initialized to NULL"?

Adding the extra traps for NULL literals actually caught one place
where I had screwed up, and making the ctor arg not have a default
value of NULL caught 2 places where I had forgotten to init a pointer.

You should have some static code analyzer that can detects
uninitialized members. I think most of them do but i may be wrong.

So that was my use case.  I was just wondering if there was already
something like this or if this was actually useful.  Also hoping to
get feedback on any screw-ups I made.

Looks fine. Paranoid like i am most constructors that have 1 member i
declare "explicit" almost automatically.
Also these (T catches_null) members i would perhaps declare private.
But that is just matter of taste really.

 

[Marcel Müller]

Does such a thing exist? It seems handy for self-documenting, at
least.

I whipped this up. Am i missing something?

Feel free to make the things complicated.
Strictly speaking you must not derive from scoped_ptr as you do not
provide the full interface of scoped_ptr.

Furthermore

const scoped_ptr<MyClass> my_ptr(new MyClass());

does the job mostly the same.

However, I still do not catch what you want to document with the
non-NULL pointer.


Marcel

 

[Öö Tiib]

However, I still do not catch what you want to document with the
non-NULL pointer.

But ... his templates name documents that class has a smartpointer
member that can not be NULL because that name tells it in English?

 

[Marcel Müller]

But ... his templates name documents that class has a smartpointer
member that can not be NULL because that name tells it in English?

Well this was not my question. I meant with respect to his program. Why
he wants to emphasize the not NULL property?

Whoever is familiar with the C++ language will see that any dereferenced
pointer must not be NULL.

I personally dislike too much goodies like NeverNullScopedPtr because
they make third party code more difficult to service. You never know how
the class NeverNullScopedPtr exactly behaves unless you have analyzed
it. Furthermore the runtime exceptions are not very helpfull since they
require a very good code coverage during testing.

So I would assent to Markus's hint to use references. This has the
additional advantage that if polymorphism is used the compiler will drop
the unnecessary NULL check when transforming pointers in up or down casts.


Marcel

 

[Tim H]

Understandable. When member scoped_ptr may not be NULL you need it to
be checked in constructors and where it is reset().

However when dealing with parsing trees the error messages are most
important thing. Usualy it is written by human what you parse and
human always demands detailed explanations how fool he is.

An errant NULL is an error in the parser, not the parsee

I would end up (when using your NeverNullScopedPointer) catching
"NeverNullScopedPtr reset to NULL" and "NeverNullScopedPtr initialized
to NULL" asap and then forming readable explanations what went wrong.
That results with double work since if i did put explicit NULL checks
(into constructors/reset locations) i could immediately throw useful
error messages/codes from there. Also if constructor initializes
multiple NeverNullScopedPtr i have to make the checks each time before
calling it otherwise how to make difference between two
"NeverNullScopedPtr initialized to NULL"?


You should have some static code analyzer that can detects
uninitialized members. I think most of them do but i may be wrong.

but uninitialized members such as scoped_ptr are perfectly fine.

Looks fine. Paranoid like i am most constructors that have 1 member i
declare "explicit" almost automatically.
Also these (T catches_null) members i would perhaps declare private.
But that is just matter of taste really.

Hmm, good point.

 

[Tim H]

Why don't you use references?

Ownership is much less clear that way, and I still have to call delete
manually.

 

[Tim H]

Feel free to make the things complicated.
Strictly speaking you must not derive from scoped_ptr as you do not
provide the full interface of scoped_ptr.

I don't follow why that is a rule? The interfaces I do not override
are inherited.

Furthermore

   const scoped_ptr<MyClass> my_ptr(new MyClass());

does the job mostly the same.

Except it allows:
const scoped_ptr<MyClass> my_ptr(NULL);
or
MyClass *p = NULL;
const scoped_ptr<MyClass> my_ptr(p);

Which I want to be an error

However, I still do not catch what you want to document with the
non-NULL pointer.

Any time one of these classes receives a NULL pointer, it is a
programming error. That should not be possible. I just put the
assert()s into the class (as throws).

 

[Öö Tiib]

Well this was not my question. I meant with respect to his program. Why
he wants to emphasize the not NULL property?

If you get the pointer from some legacy factory function that may
return NULL that you may forget to check. However such factories
usually have deleters too and you can not give deleter to scoped_ptr,
so i am also somewhat puzzled.

Whoever is familiar with the C++ language will see that any dereferenced
pointer must not be NULL.

Unfortunately surprizingly few people use such defensive style that
first half or more of each function is error checking and asserting.

I personally dislike too much goodies like NeverNullScopedPtr because
they make third party code more difficult to service. You never know how
the class NeverNullScopedPtr exactly behaves unless you have analyzed
it. Furthermore the runtime exceptions are not very helpfull since they
require a very good code coverage during testing.

I would just check for NULL in constructor anyway. Trouble for me is
that i must have member initializer for such member and so it is again
that geeky try-catch-constructor to catch it ASAP and rethrow
something that makes sense.

Bar::Bar( ParsePosition dirtyData )
try
: noNullThing_( thingFactory( dirtyData ) )
{
}
catch ( NullPointerError const& )
{
throw ParseError( "Thing factory failed to parse Thing of Bar.",
dirtyData );
}

I really dislike it if to compare with:

Bar::Bar( ParsePosition dirtyData )
: usualScopedThing_( thingFactory( dirtyData ) )
{
if ( !usualScopedThing_ )
throw ParseError( "Thing factory failed to parse Thing of
Bar.", dirtyData );
}

So I would assent to Markus's hint to use references. This has the
additional advantage that if polymorphism is used the compiler will drop
the unnecessary NULL check when transforming pointers in up or down casts..

References are no silver bullet since most compilers let you create
invalid references without problems. In our situation:

Bar::Bar( ParsePosition dirtyData )
: refToThing_( *thingFactory( dirtyData ) )
{
if ( !&refToThing_ ) // XXX: UB
throw ParseError( "Thing factory failed to parse Thing of
Bar.", dirtyData );
}

 

[Goran]

Ownership is much less clear that way, and I still have to call delete
manually.

Well, if you have a reference, ownership is rather clear: it's NOT
yours (or else, you're doing delete &ref, which is a major WTF IMHO).

What you seem to have is transfer of ownership when you pass your
pointers around, right?

e.g.

TYPE* p = new TYPE(params);
workworkwork

TYPE2 p2(p); // p2 destroys p.

Yeah, in that case, using something like you propose is OK. But I
wouldn't pretend it's anything like scoped_ptr.

Goran.

 

[Tim H]

Well, if you have a reference, ownership is rather clear: it's NOT
yours (or else, you're doing delete &ref, which is a major WTF IMHO).

What you seem to have is transfer of ownership when you pass your
pointers around, right?
Yes.

e.g.

TYPE* p = new TYPE(params);
workworkwork

TYPE2 p2(p); // p2 destroys p.

Yeah, in that case, using something like you propose is OK. But I
wouldn't pretend it's anything like scoped_ptr.

That's correct. 'delete &m_foo' would be awful. In reality what I
have is a yacc grammar that creates intermediate syntax nodes and
passes them up the grammar to build larger nodes, etc. Ownership is
passed along, too.

E.g.
subscript_expression
: function_call_expression {
$$ = $1;
}
| subscript_expression '[' and ']' {
$$ = new SubscriptExpression($1, $3);
}
;

'subscript_expression', 'function_call_expression', and 'expression'
all produce 'Expression*' as $$. 'class SubscriptExpression' takes
ownership of the $1 and $3 Expression pointers. Whatever larger
construct this rule feeds into will take ownership of the Expression
pointer produced by the 'subscript_expression' rule.

As for not being like scoped_ptr, it *is* a scoped_ptr. The most
useful scope is an object instance. It's conceivably useful outside
of the context of a class taking ownership of a pointer, but that may
be a stretch. Maybe I'll rename it to NeverNullOwnedPointer(), though
the idiom of a "scoped ptr" as a class member is fairly well
understood, I think.

Tim

 

[Goran]

As for not being like scoped_ptr, it *is* a scoped_ptr.

Yes, but only kinda-sorta.

First of, you don't want it to be NULL, so Liskov substitution
principle is broken (clearly, that does not matter to you, but when
looked from a more general perspective).

Second, purpose is of scoped_ptr is to own an object in a given scope,
but you actually have two scopes and ownership transfer. So what you
need looks more like good old auto_ptr than like scoped_ptr, no? Sure,
you can swap scoped_ptr-s, but you can just as well use autoPNew =
autoPOld.

But I concede that what I say is quite nit-picky ;-)

Goran.