774120 - #ifdef fields only used in debug builds

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

Attachment: add #ifdef

Clang warns about unused private fields.

Comment 1

[Terrence Cole [: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.

Comment 2

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

Attachment: 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.

Comment 3

[Terrence Cole [:terrence]]

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.

Comment 4

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/808279a147bf