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

RESOLVED FIXED in mozilla36

Status

()

Core
XPCOM
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: froydnj, Assigned: froydnj)

Tracking

unspecified
mozilla36
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
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)

Comment 2

3 years ago
I think I'd prefer 1 too.
Flags: needinfo?(bugs)
(Assignee)

Comment 3

3 years ago
Created attachment 8521684 [details] [diff] [review]
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.
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+

Comment 5

3 years ago
https://hg.mozilla.org/mozilla-central/rev/3afd39495f53
Assignee: nobody → nfroyd
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.