Q_UNUSED is more specific than a simple (void)x , so ASSERT_UNUSED should call Q_UNUSED for Qt builds to make ICC and RVCT happy. ASSERT_UNUSED: -------------- #define ASSERT_UNUSED(variable, assertion) ((void)variable) Q_UNUSED: --------- #if defined(Q_CC_INTEL) && !defined(Q_OS_WIN) || defined(Q_CC_RVCT) template <typename T> inline void qUnused(T &x) { (void)x; } # define Q_UNUSED(x) qUnused(x); #else # define Q_UNUSED(x) (void)x; #endif
Created attachment 65912 [details] proposed patch I didn't test it locally, please let EWS to do it before review.
Comment on attachment 65912 [details] proposed patch r=me Please set cq+ or manually checkin when bots gets green.
Attachment 65912 [details] did not build on qt: Build output: http://queues.webkit.org/results/3815170
Comment on attachment 65912 [details] proposed patch patch is incorrect, fix is coming soon
Created attachment 65915 [details] proposed patch - include added - incorrect paranthesis removed - tested locally :)
What about UNUSED_PARAM? What about non-Qt platforms? I’d like to see both ASSERT_UNUSED and UNUSED_PARAM use the best technique available. Probably they would just do it directly instead of using Q_UNUSED.
Created attachment 65925 [details] proposed patch UNUSED_PARAM use Q_UNUSED too
(In reply to comment #6) > What about UNUSED_PARAM? What about non-Qt platforms? > > I’d like to see both ASSERT_UNUSED and UNUSED_PARAM use the best technique available. Probably they would just do it directly instead of using Q_UNUSED. Do you mean we should copy and use the logic of Q_UNUSED instead of using Q_UNUSED?
Attachment 65925 [details] did not build on mac: Build output: http://queues.webkit.org/results/3870149
(In reply to comment #8) > (In reply to comment #6) > > What about UNUSED_PARAM? What about non-Qt platforms? > > > > I’d like to see both ASSERT_UNUSED and UNUSED_PARAM use the best technique available. Probably they would just do it directly instead of using Q_UNUSED. > > Do you mean we should copy and use the logic of Q_UNUSED instead of using Q_UNUSED? Yes. We’d want to do that if there are any non-Qt ports that also work with the compilers that require the alternate technique. And if we did that, then we could avoid having any Qt-specific code. In the end, I don’t care whether we use Q_UNUSED or not, I just want the macros to work well for everyone.
Created attachment 66023 [details] proposed patch I refactored ASSERT_UNUSED and UNUSED_PARAM to have similar behavior as Qt's Q_UNUSED. If this patch will be landed, we won't need to use Q_UNUSED anymore.
Comment on attachment 66023 [details] proposed patch Formatting isn’t quite right. We normally put the "&" next to the "T". We normally don’t put a space after the keyword "template". And the names of the functions aren't quite right either. But I think we can live with it like this, so r=me
(In reply to comment #12) > (From update of attachment 66023 [details]) > Formatting isn’t quite right. We normally put the "&" next to the "T". We normally don’t put a space after the keyword "template". And the names of the functions aren't quite right either. But I think we can live with it like this, so r=me I fixed style errors ( T& and template<...> ), and then landed it: http://trac.webkit.org/changeset/66489