Closed Bug 1097861 Opened 7 years ago Closed 7 years ago

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

Categories

(Core :: XPCOM, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: froydnj, Assigned: froydnj)

Details

Attachments

(1 file)

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?
Flags: needinfo?(continuation)
Flags: needinfo?(bugs)
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.
Flags: needinfo?(continuation)
I think I'd prefer 1 too.
Flags: needinfo?(bugs)
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.
Attachment #8521684 - Flags: review?(continuation)
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
Attachment #8521684 - Flags: review?(continuation) → review+
https://hg.mozilla.org/mozilla-central/rev/3afd39495f53
Assignee: nobody → nfroyd
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.