The default bug view has changed. See this FAQ.

#ifdef fields only used in debug builds

RESOLVED FIXED in mozilla17

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: espindola, Assigned: espindola)

Tracking

unspecified
mozilla17
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Created attachment 642424 [details] [diff] [review]
add #ifdef

Clang warns about unused private fields.
Attachment #642424 - Flags: review?(terrence)
Comment on attachment 642424 [details] [diff] [review]
add #ifdef

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

We only need 1 of these new DEBUG sections if we use DebugOnly.  The only place we cannot safely use DebugOnly is in the AssertRootingUnnecessary case.  Patch accepted once you've applied the nits.

::: js/src/gc/Root.h
@@ +399,2 @@
>          : cx(cx)
> +#endif

Just move cx init down into the DEBUG block in the function proper.

::: js/xpconnect/src/xpcprivate.h
@@ +3909,2 @@
>      jsid mCheck;
> +#endif

Just make mCheck a DebugOnly<jsid>, then we won't need either new DEBUG section.  While you are here, please also make the "jsid old" in the destructor a DebugOnly<jsid> and kill the DEBUG section there as well.
Attachment #642424 - Flags: review?(terrence) → review+
Created attachment 642701 [details] [diff] [review]
updated patch

Using DebugOnly is the the right think in here. We would still have a mCheck member that would be private and unused, which is exactly what the warning is about.

I did implement the other review request.
Assignee: general → respindola
Status: NEW → ASSIGNED
Attachment #642701 - Flags: review?(terrence)
Attachment #642424 - Attachment is obsolete: true
Comment on attachment 642701 [details] [diff] [review]
updated patch

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

I still don't like the change to xpcprivate, but it's not worth quibbling over for something in XPConnect.
Attachment #642701 - Flags: review?(terrence) → review+
https://hg.mozilla.org/mozilla-central/rev/808279a147bf
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in before you can comment on or make changes to this bug.