Closed Bug 900908 Opened 11 years ago Closed 10 years ago

Use varadic macros in nsIClassInfoImpl.h and nsISupportsImpl.h

Categories

(Core :: XPCOM, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: glandium, Assigned: poiru)

References

Details

Attachments

(4 files, 2 obsolete files)

      No description provided.
Depends on: 900903
Version: 24 Branch → Trunk
Assigning myself.

I have patches for this that work fine on all our compilers (see https://tbpl.mozilla.org/?tree=Try&rev=fbf2c396891f), but I'll wait for bug 989460 to land before attaching them here.
Assignee: mh+mozilla → birunthan
Depends on: 989460
No longer depends on: 900903
Comment on attachment 8411997 [details] [diff] [review]
Part 1: Add variadic variants of numbered macros in nsISupportsImpl.h

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

This is excellent.

::: xpcom/glue/nsISupportsImpl.h
@@ +826,5 @@
>      NS_INTERFACE_TABLE_ENTRY(_class, nsISupports)                             \
>    NS_INTERFACE_TABLE_END
>  
> +#define NS_INTERFACE_TABLE(aClass, ...)                                       \
> +  MOZ_STATIC_ASSERT_VALID_ARG_COUNT(__VA_ARGS__);                             \

This correctly complains about the zero-argument case, yes?  After reading bug 989460, it looks like it does, but there's enough macrology in there that I'd like to double-check.
Attachment #8411997 - Flags: review?(nfroyd) → review+
Attachment #8411998 - Flags: review?(nfroyd) → review+
(In reply to Nathan Froyd (:froydnj) from comment #5)
> This correctly complains about the zero-argument case, yes?

Yep.
Try push: https://tbpl.mozilla.org/?tree=Try&rev=629e2f114268
Status: NEW → ASSIGNED
Keywords: checkin-needed
Whiteboard: [leave open]
(Still slightly confusing that you need to use NS_IMPL_ISUPPORTS_INHERITED0)
Comment on attachment 8412865 [details] [diff] [review]
Part 3: Change uses of numbered macros in nsIClassInfoImpl.h/nsISupportsImpl.h to the variadic variants

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

r/rs=me.  You can imagine that I didn't examine all of these individually. :)
Attachment #8412865 - Flags: review?(nfroyd) → review+
Attachment #8412866 - Flags: review?(nfroyd) → review+
Attachment #8411997 - Flags: checkin+
Attachment #8411998 - Flags: checkin+
Keywords: checkin-needed
Whiteboard: [leave open]
Attachment #8413344 - Flags: checkin+
Attachment #8413345 - Flags: checkin+
https://hg.mozilla.org/mozilla-central/rev/6c1c7e45c902
https://hg.mozilla.org/mozilla-central/rev/bb248db2c54c
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
I would appreciate it, if, in the future, these kinds of mass changes were announced on newsgroups before they occurred.
(In reply to Joshua Cranmer [:jcranmer] from comment #18)
> I would appreciate it, if, in the future, these kinds of mass changes were
> announced on newsgroups before they occurred.

Sorry! I made a post now: https://groups.google.com/d/msg/mozilla.dev.platform/AXtW93QU6d8/91F-wdCvhq4J
You need to log in before you can comment on or make changes to this bug.