[cfe-commits] r60900 - in /cfe/trunk: include/clang/Parse/Ownership.h

[Howard Hinnant]

In an effort to "de-mystify" the subject around this "smart pointer"  
application I would like to take include/llvm/ADT/OwningPtr.h as  
example code.  I've enclosed a modified OwningPtr.h just as an  
example, not as a proposed change.  I encourage everyone to do a diff  
between the enclosure and the current OwningPtr.h (the diff is small &  
boilerplate).

Consider this very simplistic sample code which is written to the  
current OwningPtr.h interface:

#include <iostream>
#include "OwningPtr.h"

int* source(int i)
{
     llvm::OwningPtr<int> p(new int(i));
     std::cout << "source: " << *p << '\n';
     return p.take();
}

void sink1(int* i)
{
     llvm::OwningPtr<int> p(i);
     std::cout << "sink1: " << *p << '\n';
}

void sink2(int* i)
{
     std::cout << "sink2: " << *i << '\n';
}

int main()
{
     sink1(source(1));
     sink2(source(2));
}

There is a "source" function which creates an OwningPtr, but does not  
return an OwningPtr because OwningPtr isn't copy constructible.  It  
instead returns a pointer to the data (just an int here, likely an  
Expr in clang).

There are two clients: sink1 and sink2.  The clients have similar  
signatures, but different semantics.  sink1 takes ownership of the  
pointer.  sink2 does not.  sink2 may be this way by design, or this  
could be coding error.

The example above compiles, and prints out:

source: 1
sink1: 1
source: 2
sink2: 2

Because I ran it with a memory-leak checker, it also printed out:

Leaking memory (8 bytes) at 0xb010
Total memory leaked is 8 bytes

Now consider substituting in the modified OwningPtr.h:

source: 1
sink1: 1
source: 2
sink2: 2
Leaking memory (8 bytes) at 0xb010
Total memory leaked is 8 bytes

well, same problem.

Note:  This modification won't alter existing behavior.  However it  
enables precautions to be taken to make the code safer.  The first  
precaution to be made is to alter "source" to return an OwningPtr.   
Before we couldn't do this.  Now we can:

llvm::OwningPtr<int> source(int i)
{
     llvm::OwningPtr<int> p(new int(i));
     std::cout << "source: " << *p << '\n';
     return move(p);
}

Compile time errors:

test.cpp: In function ‘int main()’:
test.cpp:52: error: cannot convert ‘llvm::OwningPtr<int>’ to ‘int*’  
for argument ‘1’ to ‘void sink1(int*)’
test.cpp:53: error: cannot convert ‘llvm::OwningPtr<int>’ to ‘int*’  
for argument ‘1’ to ‘void sink2(int*)’

The compiler is now using the type system to alert the programmer to  
possible memory leaks.  The first error is pointing to:

     sink1(source(1));

Ok, OwningPtr (correctly) will not implicitly convert to a raw  
pointer.  We note that sink1 was designed to accept ownership of the  
pointer so we modify it like so:

void sink1(llvm::OwningPtr<int> p)
{
     std::cout << "sink1: " << *p << '\n';
}

Note that we can now use OwningPtr to pass by value to functions.   
This is a pattern, like auto_ptr, that indicates to the client that  
sink1 is accepting ownership of the referenced resource.

The second error refers to:

     sink2(source(2));

Same problem.  We must now decide if sink2 is correct in not claiming  
ownership of the resource, or if it is simply buggy.  If it is a bug,  
we fix it the same way we fixed sink1.  So let's assume that sink2 is  
correct as is.  We have to change our call site then:

     sink2(source(2).get());

Now the sample compiles and prints out:

source: 1
sink1: 1
source: 2
sink2: 2

There are no memory leaks.  By eliminating the use of "take()", we  
have greatly reduced our exposure to leaks because unowned raw  
pointers are never passed around.  Yet we can still pass in (owned)  
raw pointers (as in the sink2 example) when the API calls for it.

Portability:  All compilers that enforce the rule that temporaries can  
not bind to non-const references will compile this code.  And there is  
a workaround for VC2005 (/Za off) which allows this example to compile  
and behave identically.

On VC2005 (/Za off), this modified OwningPtr will have approximately  
the behavior of std::auto_ptr.  Everywhere else it will refuse to move  
from lvalues with copy syntax:

    OwningPtr<int> p(new int(1));
    sink1(p);  // compile time error

Summary:  This change allows us to use the type system to help enforce  
resource ownership.  Exposure to unowned pointers (such as what take()  
returns) is minimized but still allowed.

For reference here is the complete altered example:

#include <iostream>
#include "OwningPtr.h"

llvm::OwningPtr<int> source(int i)
{
     llvm::OwningPtr<int> p(new int(i));
     std::cout << "source: " << *p << '\n';
     return move(p);
}

void sink1(llvm::OwningPtr<int> p)  // owns int*
{
     std::cout << "sink1: " << *p << '\n';
}

void sink2(int* i)  // doesn't own int*
{
     std::cout << "sink2: " << *i << '\n';
}

int main()
{
     sink1(source(1));
     sink2(source(2).get());
}

-Howard
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OwningPtr.h
Type: application/octet-stream
Size: 3826 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20081213/761bf9ed/attachment.obj>
-------------- next part --------------