104346 - need |nsRefPtr|

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P1)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

We need a refcounting smart pointer that doesn't require inheritance from
nsISupports so that people don't overuse COM just because they want a refcount
and they like nsCOMPtr.  (See bug 104336.)

I'll attach an experimental implementation I was playing with and a test that
actually works on gcc3.  And then I'll start over from scratch (and from
nsCOMPtr.h) later to write something that we could really consider checking in.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: fancy version (don't try this at home), to be abandoned

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: Tests that the previous code passes, with gcc3

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: work in progress (works fine on gcc 3.0.1!)

Comment 4

[Axel Hecht]

should that topic be about nsAutoPtr? ;-) Though there is no impl file, should
this go into glue? Or not depend on glue, doubling already_AddRefed, or 
getting it out of nsCOMPtr.h?

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

There's no need for it to be in glue since it's entirely inlined.

Comment 6

[Brendan MacLean]

Nice start, but looks to me like these classes are a little too lean.

The project on which I am currently working has all of these types of pointers, 
but our implementations also cover the following:

	T& operator*() const
		{
		assert(p!=NULL);
		return *p;
		}
	T * operator->() const
		{
		assert(p!=NULL);
		return p;
		}
	//The assert on operator& usually indicates a bug.  If this is really
	//what is needed, however, take the address of the p member explicitly.
	T** operator&()
		{
		assert(p==NULL);
		return &p;
		}
	bool operator!() const
		{
		return (p == NULL);
		}
	bool operator<(T* pT) const
		{
		return p < pT;
		}
	bool operator==(T* pT) const
		{
		return p == pT;
		}
	bool operator!=(T* pT) const
		{
		return p != pT;
		}
	T* Detach()
		{
		T* pt = p;
		p = NULL;
		return pt;
		}

The first 3 are mostly just debug help over what you have today. On #3 though, 
I notice that you force the use of 'begin_assign' instead, which, I guess, 
seems okay, but it looks to me like it leaks:

T** begin_assignment() { mPtr = 0; return &mPtr; }

My #3 above asserts that the pointer is NULL, and you just set it to NULL.  
What if mPtr is already holding a pointer to allocated memory?  You haven't 
freed it.

The boolean operators are useful, since they make converting existing pointer 
code the an auto-pointer easier, and less prone to introduction of new bugs or 
compiler errors that need to be fixed.

And finally, a Detach method seems crucial for times when you want to release 
the auto-pointer from responsibility for freeing the object, because you are 
transfering that responsibility to another class or function.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Many of those operators you mention should be handled by implicit conversion --
I've yet to test whether that actually works across compilers.

Instead of |Detach| I used |release|, as std::auto_ptr does.

That is a mistake in begin_assignment -- I was thinking |*this = 0|.  I hadn't
tested that yet, though.

I should also probably make operator& private as for nsCOMPtr.

Comment 8

[Brendan MacLean]

Yup.  All but '>' work on my compiler.

I don't see 'release' in the posted patch, but perhaps I am just not good at 
reading these yet.  The term would get pretty overloaded on the nsRefPtr, I'd 
imagine, where release should imply a ref count decrement.

Given the ref counting issue, we use 'release' to 'free' (either by 
decrementing, deleting, or freeing), and 'detach' to remove the pointer 
without 'freeing'.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

There's a newer version checked in to the tree.  There is no |release| on
nsRefPtr.  Perhaps it needs a |swap| like the patch for nsCOMPtr in bug 78016.

Comment 10

[Cathleen]

-> 0.9.8

Comment 11

[jag (Peter Annema)]

r=jag on what's currently in the tree.

Comment 12

[John Keiser (jkeiser)]

I want to use nsRefPtr<> on a COM object (it implements nsIDocumentObserver) so
that I don't have to create an interface and IID for the concrete class.  I have
found that I can use an nsCOMPtr<> in the meantime (since I only implement one
interface) but I'll try switching on nsRefPtr<> first since I think that's
really what I'm trying to do.  It turns out I don't need operator-> but I'll
test that anyway.

Any chance we can make nsCOMPtr<T> extend from nsRefPtr<T>?  It seems like they
have mostly the same purpose (nsRefPtr<T> just has *extra* stuff) and we could
save some lines of code that way (and obscure bugs--copying implementations from
nsCOMPtr save us the headache of repeating history).  The specialized
nsCOMPtr<nsISupports> is the only thing I'm worried about there.  Some kind of
funkiness is going on extending from nsCOMPtr_base.  Anyhow, I'll give this a
shot.  At the least, I think nsRefPtr method implementations should look like
nsCOMPtr's do, whether that means changing nsRefPtr or changing nsCOMPtr :)

Finally, should we comment out everything else in nsAutoPtr.h (nsAutoPtr<T>,
nsAutoArrayPtr<T> and nsMemoryAutoPtr<T>) or is this a "include one, include
all" thing?  I think until we need them we should #if 0 out the other classes.

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Messing with nsCOMPtr that much is scary and, in my opinion, unnecessary.

Comment 14

[Scott Collins]

hmmm, why write our own if we can use the pointers from boost?

Comment 15

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

i might have a look at finishing this. One bug i found was that nsRefPtr needs
to "NS_IF_ADDREF" rather then "NS_ADDREF", i.e. it needs to nullcheck before
calling Addref/Release

Comment 16

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

all begin_assignment needs to exit() the current pointer before returning.

Comment 17

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: fixes a few random things

This fixes:
* comment 15 and comment 16
* make nsRefPtr return nsDeriviedSafe to protect against addreffing and
  releasing an nsRefPtr
* fixes a typo in nsMemoryAutoPtr::operator=
* removes the implementations of all operator& since they should not be called


There are a few things i'm pondering:
1. Scott raises a good question on if we should be doing our own implementation

   at all. There are two things i don't like with boost
   a) They keep the refcount outside the object. Their pointer therefor bigger
      then 4 bytes, and they have an extra heap-allocated object for each
      refcounted object to keep the count.
   b) AFAICT there is no way to get an equivalent to nsMemoryAutoPtr with
boost.
      How big of a loss this is I don't know.
   If we can live with these things, or if it would be worth using boost for
the
   other pointertypes, I don't know.
   
2. Should we really have |operator=| and |operator T*| on the autoptrs. On one
   hand we do on nsRefPtr and nsCOMPtr, but it seems easier to forget to call 
   .release() on an autoptr and crash. Maybe it's the |operator T*| that is
   dangerous, if the only way to get the pointer was through .get() and
   .release() then people might be less likly to accidently get the pointer
   without properly managing the ownership.
   If we decide not to have |operator=| we should probably not have
   |getter_Transfers| either.

3. |explicit| on the ctors seems like a nice thing to have. If we have an
   operator=(T*) then that should be enough to get |nsAutoPtr<T> ptr = foo;| to

   compile. This is of course related to the previous point.

Comment 18

[David Bradley]

I believe there was a very good reason std::auto_ptr didn't provide an opertor
=(T*); Unfortunately I don't remember the reason, just that there was one.

And there's issues to deal with protecting const pointers. And some of the
solutions may not be supported across the wide variety platforms we work on.

Also, if we're going to do something different than what std::auto_ptr does, you
might think about changing the name.

As far as the reference counted pointers go. A friend of mine created a template
implementation that allowed the user to decide how the reference counting system
was implemented. This allowed us to handle both intrusive and passive ref
counting mechanisms. It could be a little edgy on templates for our uses, but I
don't think it would be. In addition to the type of pointer, we had an
additional template parameter which was the class to implement the ref counting.
This essentially would allow all ref counting code to be based of the same base
template, whether it was XPCOM or regular pointer, or some other system. I'd
have to sit down and remember the full implementation. It's been like 4 years ago.

Comment 19

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

a couple of articles on other smart-pointers:
http://www.gotw.ca/publications/using_auto_ptr_effectively.htm
http://www.cuj.com/experts/2007/hyslop.htm

Comment 20

[Brian Ryner (not reading)]

I need this for some deCOMtamination work I'm doing.  Any chance we can come to
a decision on this and get nsRefPtr turned on?

Comment 21

[David Bradley]

1. Comment #14 and Comment #17 couldn't we use the stuff that fits our needs? I
don't know about licensing issues and such, though.
2. Why is this in XPCOM? What does it have to do with XPCOM?

Comment 22

[David Bradley]

Skimming I noticed the assignment operators for most if not all the class are
not self assignment safe. They also don't take into account you could be
assigning the same pointer value but using two different pointer class
instances. In both these cases you'll end up with class holding pointers to dead
objects.

So I think there's some work to be done even if we use the interfaces provided.
(it still makes me nervous to allow conventions that other implementations have
disallowed to avoid accidental misuse, such as non-explicit constructors and
assignment operators taking pointer types)

Comment 23

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: a version of nsAutoPtr.h based on nsCOMPtr

Since bryner wants to be able to use something quickly, here's a version of
nsAutoPtr.h based on nsCOMPtr, which is something that we know works and meets
our portability requirements.

Comment 24

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch to tests to go along with attachment 111746

This is a patch to TestAutoPtr.  It compiles with the above patch.

However, a bunch of the tests fail, mostly related to casting to base classes
(perhaps only dealing with equality).  I still haven't investigated why.

Comment 25

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: a version of nsAutoPtr.h based on nsCOMPtr

This one passes the equality tests, at least with gcc 3.2.1.

I think I'd like to check in these changes (to nsAutoPtr and to the tests) so
we can test this on multiple platforms.  Does that seem reasonable?

Comment 26

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Also, what do you think about the name |forget()|?  People didn't like the use
of |release()| as in the standard auto_ptr since we use |Release()| for
something else.  |drop()| was also suggested above, I think, but I just happened
to think of |forget()| and like it.

Comment 27

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Also, we should probably port the equality fixes to nsCOMPtr (although I
actually worry such comparisons may not compile cross-platform).

Comment 28

[Brian Ryner (not reading)]

Visual C++ doesn't quite like this:

../../dist/include/xpcom\nsAutoPtr.h(1059) : error C2440: 'reinterpret_cast' : 
cannot convert from 'const class TestRefObject *' to 'class 
nsDerivedSafe<class TestRefObject const > *'
        ../../dist/include/xpcom\nsAutoPtr.h(1058) : while compiling class-
template member function 'class nsDerivedSafe<class TestRefObject const > 
*__thiscall
 nsRefPtr<class TestRefObject const >::get(void) const'
make[1]: *** [TestAutoPtr.obj] Error 2

Comment 29

[Brian Ryner (not reading)]

Attachment: patch against attachment 111884 to fix msvc bustage

This fixes get() for me.

Comment 30

[David Bradley]

Just a note, it looks like nsCOMPtr may go the with swap instead of a 'forget'
type function. Thought I'd mention in case you want to keep the two in sync or
have other ideas.

Comment 31

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: remove operator= and add reset()

This patch removes the potentially harmful operator= and introduces a
reset-method instead. It allows

myAutoPtr.reset();
myAutoPtr.reset(myRawPtr);
myAutoPtr.reset(myOtherAutoPtr);

It also removes getter_Transfers() since it is pretty much as dangerous as
operator=. I'm a little unsure about removing it though feel free to speak up
if you want it to stick around. The new way of doing the same is

Foo* rawptr
obj->GetFoo(&rawptr);
nsAutoPtr<Foo> ptr(rawptr);

or

nsAutoPtr<Foo> ptr;
...
Foo* rawptr
obj->GetFoo(&rawptr);
ptr.reset(rawptr);

I also made all ctors explicit. (should the empty ctor be explicit?)

Comment 32

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Does |TestAutoPtr| still work with this patch?

Comment 33

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: remove operator= and add reset() v2

This includes neccesary changes to make TestAutoPtr.cpp build on windows. It
also adds some more tests.

There are two things that i find strange:

1. The following compiles even though the ctor taking a raw-pointer is
explicit:

   nsAutoPtr<TestObject> TestObjectFactory()
   {
       return new TestObject;
   }

   Not that i see a great risk in this, i just found it a bit strange and
thought
   it'd be best to get tested on all platforms

2. I had to make the ctor taking an nsAutoPtr be *not* explicit. Otherwise I
   could not get functions like:
   void Sink(nsAutoPtr<TestObject> aObject);
   to compile, no matter how i tried to call it. I kept getting a compileerror
   saying "cannot convert parameter 1 from 'class nsAutoPtr<TestObject>' to
   'class nsAutoPtr<TestObject>'. No copy constructor supplied"
   nsAutoPtr<TestObject>"

Comment 34

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

one thing i thought about if we really really want |getter_Transfers| is to
force a syntax like

nsAutoPtr<Foo> foo;
...
getFoo(getter_Transfers(foo.reset());

that way it should be apparent to anyone that whatever foo is pointing to is
deleted. I would prefer removing getter_Transfers entierly though.

Comment 35

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I'm comfortable with removing getter_Transfers entirely.  In fact, now that you
mention it, I'm strongly in favor of removing it entirely.

Comment 36

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: remove operator= and add reset() v3

exactly the same as the above but with an additional test that simulates
out-of-memory

Comment 37

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 113556 [details] [diff] [review]
remove operator= and add reset() v3

cool. Then i propose that we go with this interface.

Comment 38

[David Bradley]

I think adding explicit to the default constructor has no meaning. And why not
just remove the default constructor and use the T * constructor with a default
of null. explicit nsAutoPtr(T * aRawPtr = 0);

Comment 39

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I'm not sure that nsAutoPtr(T * aRawPtr = 0) would work across all compilers.
nsCOMPtr has separate ctors

Comment 40

[Brian Ryner (not reading)]

Attachment: patch to nsDerivedSafe

There's a potential problem on platforms where HAVE_CPP_ACCESS_CHANGING_USING
is not defined.  The fact that nsDerivedSafe declares AddRef and Release as
virtual functions can change the object layout, since they may be non-virtual
on the template class.	There's really no _need_ for them to be marked virtual
on nsDerivedSafe (a call to them won't compile anyway!) so this patch removes
the virtual.

Comment 41

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 114729 [details] [diff] [review]
patch to nsDerivedSafe

There *might* be an issue with this on Windows because of the calling
convention stuff hidden in |NS_IMETHOD_|.  (|virtual| should be implied when
the method is |virtual| on the base class.)  Did you test on Windows?

Comment 42

[Brian Ryner (not reading)]

yeah, i compiled and ran on windows and things appeared to be fine.

Comment 43

[Brian Ryner (not reading)]

Comment on attachment 114729 [details] [diff] [review]
patch to nsDerivedSafe

Scott, what do you think?

Comment 44

[jag (Peter Annema)]

Comment on attachment 114729 [details] [diff] [review]
patch to nsDerivedSafe

r=jag.

Comment 45

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: attempt to fix otaku bustage

Comment 46

[Bradley Baetz (:bbaetz)]

Comment on attachment 115277 [details] [diff] [review]
attempt to fix otaku bustage

r=bbaetz

Comment 47

[Brian Ryner (not reading)]

dbaron, i'll leave it up to you whether this bug should be resolved as fixed now.

Comment 48

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

IMHO we should still check in the interface change in attachment 113556 [details] [diff] [review]. It
makes the interface "safer" and more similar to other smart-pointer
implementations out there.

Bug 185797 (huge rewrite of the XSLT part of transformiix) is very dependant on
nsAutoPtr. If we're going to go with the current interface i'd like to know so
that i can start using it as is. Right now i'm waiting for the patch to be
checked in so that i can start using it.

Comment 49

[Scott Collins]

I don't think I'm the person that put those modifiers on there.  I think that
was a fix for some platform horkage.  My advice is to make sure you try building
on the compilers that use that hunk with your changes.

Comment 50

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 113556 [details] [diff] [review]
remove operator= and add reset() v3

Removing getter_Transfers is a good idea.  I'm not so sure about the operator=
changes -- is the idea to make it so that you can't use |=| syntax?  What about
compiler generated operator=?  Do the same things compile and not compile
across platforms?

Comment 51

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Yes, the idea is that the |=| syntax since it might not be obvious that the old
object will be deleted (though I forgot to make a private non-implemented one).
It's code like this that I want to avoid:

nsAutoPtr<Foo> foo(new Foo);
...
Foo* old = foo;
foo = getFoo();
...
old->useFoo();


It is IMHO not apparant that this will crash due to the first Foo being deleted
when |foo| gets a new value. So the idea is to force

nsAutoPtr<Foo> foo(new Foo);
...
foo.reset(getFoo());

which should make it more apparent that the old value gets deleted. This is the
same reason that I wanted to get rid of getter_Transfers, that it isn't apparent
that the old value will unmercifully be deleted.

Looking at scoped_ptr from boost and auto_ptr from stl it seems like they
instead protect against this be not having an |operator T*()| but instead force
you to call .get(). Though scoped_ptr doesn't have an operator= either.


The same things should compile across all platforms. I added every possible
thing I could think of to TestAutoPtr.cpp so as long as that behaves properly it
should work.

Comment 52

[Peter Van der Beken [:peterv]]

So on OS X I get the following compilation error for
    nsAutoPtr<foo> bar = new foo();

no matching function for call to `nsAutoPtr<T>::nsAutoPtr(nsAutoPtr<T>)'
candidates are: nsAutoPtr<T>::nsAutoPtr(nsAutoPtr<T>&) [with T = foo]
                nsAutoPtr<T>::nsAutoPtr(T*) [with T = foo]
initializing temporary from result of `nsAutoPtr<T>::nsAutoPtr(T*) [with T = foo]'

Comment 53

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

i've started using nsAutoPtr and am more and more convinced that we should get
rid of the operator=. Even in cases where you do want the old value to be
deleted it is pretty unclear that it actually is. For example in functions like

void
txStylesheetCompilerState::popChooseGotoList()
{
    // this will delete the old value
    mChooseGotoList = popObject();
}

i've found that i always end up commenting that the old value is deleted. It
would IMHO look much better as

void
txStylesheetCompilerState::popChooseGotoList()
{
    mChooseGotoList.reset(popObject());
}

David: you mentioned that you were worried that we would have cross-platform
issues with my patch. Could you explain what you suspect would/wouldn't compile
where? I'd happily try out different things on neccesary platforms.

Comment 54

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I'm worried that if you remove operator= then using operator= (but not in the
ctor, like peterv used it) will compile on some platforms and not others. 
Although the ctor case matters to.  We need to make things so that we're as
close as possible to having the same set of things compile across platforms.

Comment 55

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I also find the reset syntax a bit clumsy, especially when dealing with member
variables.  Furthermore, I don't think people have had a problem with nsCOMPtr
and realizing that the object can't be accessed once the nsCOMPtr holds another
value.

Comment 56

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

another thing we should protect against is constructing an nsAutoPtr<A> from an
nsAutoPtr<B> (where B inherits A). For example in constructs like

class B : public A {
};

void sink(nsAutoPtr<A> aItem) {
}


  nsAutoPtr<B> b = new B;
  sink(b);
should either not compile, or it should behave like
  nsAutoPtr<A> a = new A;
  sink(a);
i.e. pass the value to the sink and then loose its value.

Comment 57

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch to remove getter_Transfers

This subsumes part of attachment 113556 [details] [diff] [review], and I'd like to get it in sooner
rather than later.

Comment 58

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 113556 [details] [diff] [review]
remove operator= and add reset() v3

Marking review- pending testing that this doesn't make the set of things that
compile inconsistent between platforms.  Also note that the test functions like

+nsAutoPtr<TestObject> TestObjectFactory()
+{
+    return new TestObject;
+}

won't compile on gcc3, since they give the error (at least without the
nsAutoPtr.h changes, and I expect with them as well):

/builds/trunk/mozilla/xpcom/tests/TestAutoPtr.cpp: In function
`nsAutoPtr<TestObject> TestObjectFactory()':
/builds/trunk/mozilla/xpcom/tests/TestAutoPtr.cpp:57: error: no matching
function for call to `nsAutoPtr<TestObject>::nsAutoPtr(nsAutoPtr<TestObject>)'
../../dist/include/xpcom/nsAutoPtr.h:93: error: candidates are:
nsAutoPtr<T>::nsAutoPtr(nsAutoPtr<T>&) [with T = TestObject]
../../dist/include/xpcom/nsAutoPtr.h:87: error: 	       
nsAutoPtr<T>::nsAutoPtr(T*) [with T = TestObject]
/builds/trunk/mozilla/xpcom/tests/TestAutoPtr.cpp:57: error:   initializing
temporary from result of `nsAutoPtr<T>::nsAutoPtr(T*) [with T = TestObject]'

Comment 59

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch to remove getter_Transfers

This one also removes |begin_assignment| from nsRefPtr as cleanup that should
have happened when I fixed some build bustage before that was caused by having
the non-inlined function in the header.

Comment 60

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 125243 [details] [diff] [review]
patch to remove getter_Transfers

Oh, and I'll also remove |CreateTestObject| from TestAutoPtr.cpp since this
makes it unused, but I won't bother attaching a new patch just for that.

Comment 61

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 125243 [details] [diff] [review]
patch to remove getter_Transfers

Oh, never mind, transformix is using StartAssignment directly.

Comment 62

[Peter Van der Beken [:peterv]]

Can't we just change those two places in Transformiix where we use
StartAssignment to something else?

Comment 63

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Is it needed?  What I wanted to remove would remove the ability to do that kind
of thing entirely in any way other than casting and knowing what the internals
are.  (However, either way, you shouldn't be using |StartAssignment| directly.)

Comment 64

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Yeah, i wasn't sure if using StartAssignment was a good idea. I had a gut
feeling it wasn't. However it performed exactly the functionality that I wanted
so in a weird way it kinda made sense to use it.

What I had was a class-member that is an nsAutoPointer, and I need to get a
pointer to the internal rawpointer in there. Is there a preffered way to do
this, or is that what getter_Transfers is for.

Anyhow, we need a way to get a pointer to the internal pointer, and that
function should probably delete+nullify the internal value. Personally I like
.StartAssignment() better since it better describes what actually happens, but
OTOH getter_Transfers is closer to getter_AddRefs so I guess it makes more sense.

David, i'm not sure what you mean with "casting and knowing what the internals
are", do you mean that I should do |&mFirstInstruction.mRawPtr|?

I'm perfectly fine with using any of
mNextInstrPtr = &mFirstInstruction.mRawPtr;
mNextInstrPtr = getter_Transfers(mFirstInstruction);
or
mNextInstrPtr = mFirstInstruction.StartAssignment();