Closed
Bug 902062
Opened 11 years ago
Closed 11 years ago
ccache clang builds fail everywhere we include XPCMaps.h
Categories
(Core :: XPConnect, defect)
Tracking
()
RESOLVED
FIXED
mozilla26
People
(Reporter: ehsan.akhgari, Assigned: n.nethercote)
References
Details
Attachments
(1 file)
2.62 KB,
patch
|
ehsan.akhgari
:
review+
|
Details | Diff | Splinter Review |
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•11 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.
Comment 2•11 years ago
|
||
> 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.
Comment 3•11 years ago
|
||
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.
Comment 4•11 years ago
|
||
It'll also run cpp twice, which leads to somewhat slower builds.
Assignee | ||
Comment 5•11 years ago
|
||
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•11 years ago
|
Assignee: nobody → n.nethercote
Reporter | ||
Comment 6•11 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+
Comment 7•11 years ago
|
||
And a followup to make nsCacheEntry use LinkedList. ;)
Assignee | ||
Comment 8•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/0dd9682f22da
Reporter | ||
Comment 9•11 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.
Comment 10•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/0dd9682f22da
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in
before you can comment on or make changes to this bug.
Description
•