1097861 - change CycleCollectionNoteChild so that it doesn't require a default argument

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Nathan Froyd [:froydnj]]

I'm trying to trim some fat from nsRefPtr.h in bug 1095541 and I'm running into a problem with CycleCollectionNoteChild.  Depending on the #include ordering that we get prior to nsRefPtr.h's inclusion, we get different errors about CycleCollectionNoteChild's default argument.  This is only a problem because nsRefPtr.h attempts to forward-declare CycleCollectionNoteChild.

Annoyingly, this only happens on try, and not with the compiler I have on my local machine.

I think there are two different ways to address this:

1. Add a separate |template<typename T> void CycleCollectionNoteChild(callbackType&, T*, const char*)| overload that forwards to the "real" implementation of CycleCollectionNoteChild; or
2. Simply drop the default argument, and require people to pass 0 by default everywhere.  Judging from grep, the only time code makes manual CycleCollectionNoteChild calls is when something tricky is being done (and some of the manual calls look like they could benefit from cycle collection macros); the vast majority of the time, the calls to CycleCollectionNoteChild are hidden behind macros or template specializations.  I don't think it'd be too much of a burden to require an explicit 0 in those cases.

Any preferences?

Comment 1

[Andrew McCreight [:mccr8]]

I guess I prefer 1.  It doesn't seem like it would be much of a pain, and having random 0's floating around does not seem great.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

I think I'd prefer 1 too.

Comment 3

[Nathan Froyd [:froydnj]]

Attachment: make CycleCollectionNoteChild more easily forward-declarable

Forward declaring functions with default arguments is difficult.  If you try to say:

template<typename T>
inline void
CycleCollectionNoteChild(nsCycleCollectionTraversalCallback& aCallback,
                         T* aChild, const char* aName, uint32_t aFlags);

and then later have:

template<typename T>
inline void
CycleCollectionNoteChild(nsCycleCollectionTraversalCallback& aCallback,
                         T* aChild, const char* aName, uint32_t aFlags = 0);
{
  ...
}

the compiler complains that default arguments cannot be added to a
function template that has already been declared.  If you attempt to
mollify the compiler by declaring instead:

template<typename T>
inline void
CycleCollectionNoteChild(nsCycleCollectionTraversalCallback& aCallback,
                         T* aChild, const char* aName, uint32_t aFlags = 0);

the compiler then complains about redefining the default argument (!)
when an actual definition is found.

To circumvent this, manually implement "default" arguments by providing
a three-argument form of CycleCollectionNoteChild, which simply forwards
to the four-argument version.

Comment 4

[Andrew McCreight [:mccr8]]

Comment on attachment 8521684 [details] [diff] [review]
make CycleCollectionNoteChild more easily forward-declarable

Review of attachment 8521684 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with T* fixed

::: xpcom/glue/nsCycleCollectionNoteChild.h
@@ +92,5 @@
>  
> +template<typename T>
> +inline void
> +CycleCollectionNoteChild(nsCycleCollectionTraversalCallback& aCallback,
> +                         T aChild, const char* aName)

T* not T

Comment 5

[Ekanan Ketunuti]

https://hg.mozilla.org/mozilla-central/rev/3afd39495f53