ccache clang builds fail everywhere we include XPCMaps.h

RESOLVED FIXED in mozilla26

Status

()

Core
XPConnect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Ehsan, Assigned: njn)

Tracking

Trunk
mozilla26
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
Similar to what was discussed in bug 832788.

../../../../js/xpconnect/src/XPCMaps.h:119:31: error: equality comparison with extraneous parentheses [-Werror,-Wparentheses-equality]
        if (((entry)->keyHash == 0))
             ~~~~~~~~~~~~~~~~~^~~~
../../../../js/xpconnect/src/XPCMaps.h:119:31: note: remove extraneous parentheses around the comparison to silence this warning
        if (((entry)->keyHash == 0))
            ~                 ^   ~
../../../../js/xpconnect/src/XPCMaps.h:119:31: note: use '=' to turn this equality comparison into an assignment
        if (((entry)->keyHash == 0))
                              ^~
                              =
../../../../js/xpconnect/src/XPCMaps.h:189:31: error: equality comparison with extraneous parentheses [-Werror,-Wparentheses-equality]
        if (((entry)->keyHash == 0))
             ~~~~~~~~~~~~~~~~~^~~~
../../../../js/xpconnect/src/XPCMaps.h:189:31: note: remove extraneous parentheses around the comparison to silence this warning
        if (((entry)->keyHash == 0))
            ~                 ^   ~
../../../../js/xpconnect/src/XPCMaps.h:189:31: note: use '=' to turn this equality comparison into an assignment
        if (((entry)->keyHash == 0))
                              ^~
                              =
etc.

Bug 831885 worked around this for the jsdhash API by making that an inline function, but bug 884649 removed jsdhash.h and effectively regressed this workaround.

Can we please use to a different hashtable API there, or back out bug 884649 for now?
(Assignee)

Comment 1

5 years ago
I read bug 832788 but don't understand how this breaks ccache.

Anyway, would doing bug 831885 for pldhash.h work?  That would be far easier.
> I read bug 832788 but don't understand how this breaks ccache.

With ccache, the file being compiled already has macros expanded (because ccache uses the .i as the cache key, then just passes it to the compiler)

clang does not warn on the code pattern above _if_ it's the result of a macro.  But it has no way to know that when ccache is involved because it's just given the .i.

> Anyway, would doing bug 831885 for pldhash.h work? 

I would think so, yes.
If you add CCACHE_CPP2=yes to your environment, ccache will do the right thing here and key on the .cpp, fixing the warnings. At least this works for me for SpiderMonkey builds.
It'll also run cpp twice, which leads to somewhat slower builds.
(Assignee)

Comment 5

5 years ago
Created attachment 786654 [details] [diff] [review]
Convert PL_DHASH_ENTRY_IS_* macros to inline functions.

Ehsan, can you check that this unbreaks ccache?  I don't use it and so don't
know what to look for.
Attachment #786654 - Flags: review?(ehsan)
(Assignee)

Updated

5 years ago
Assignee: nobody → n.nethercote
(Reporter)

Comment 6

5 years ago
Comment on attachment 786654 [details] [diff] [review]
Convert PL_DHASH_ENTRY_IS_* macros to inline functions.

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

This fixes it, but then my build fails because of the same reason using PR_CLIST_IS_EMPTY in nsCacheEntry.cpp :(

This might be an uphill battle, but let's land this patch for now, cause it's not in NSPR!
Attachment #786654 - Flags: review?(ehsan) → review+
And a followup to make nsCacheEntry use LinkedList.  ;)
(Reporter)

Comment 9

5 years ago
(In reply to Boris Zbarsky (:bz) (vacation Aug 10-19) from comment #7)
> And a followup to make nsCacheEntry use LinkedList.  ;)

Bug 902696.
https://hg.mozilla.org/mozilla-central/rev/0dd9682f22da
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.