[webkit-dev] Idiom for functions with all return values in a switch case
Alfonso Guerra
huperniketes at gmail.com
Tue May 9 12:05:37 PDT 2017
On May 9, 2017 2:07 PM, "Michael Catanzaro" <mcatanzaro at igalia.com> wrote:
Hi,
Consider this function:
static WKAutoplayEvent toWKAutoplayEvent(WebCore::AutoplayEvent
event)
{
switch (event) {
case WebCore::AutoplayEvent::DidEndMediaPlaybackWithoutUserInterf
erence:
return kWKAutoplayEventDidEndMediaPla
ybackWithoutUserInterference;
case WebCore::AutoplayEvent::DidPlayMediaPreventedFromPlaying:
return kWKAutoplayEventDidPlayMediaPreventedFromAutoplaying;
case WebCore::AutoplayEvent::DidPreventMediaFromPlaying:
return kWKAutoplayEventDidPreventFromAutoplaying;
case WebCore::AutoplayEvent::UserDidInterfereWithPlayback:
return kWKAutoplayEventUserDidInterfereWithPlayback;
case WebCore::AutoplayEvent::UserNeverPlayedMediaPreventedFromPla
ying:
return kWKAutoplayEventUserNeverPlaye
dMediaPreventedFromPlaying;
}
}
Out of curiosity, why is using a switch statement better than defining an
array to hold the return values?
Alfonso Guerra
Founder/CEO
Apokalypse Software Corp
@Huperniketes
(626) 667-4285
It will trigger this GCC warning:
[3490/4357] Building CXX object Source...bKit2.dir/UIProcess/A
PI/C/WKPage.cpp.o
../../Source/WebKit2/UIProcess/API/C/WKPage.cpp: In static member function
‘static WKAutoplayEvent WKPageSetPageUIClient(WKPageRef, const
WKPageUIClientBase*)::UIClient::toWKAutoplayEvent(WebCore::AutoplayEvent)’:
../../Source/WebKit2/UIProcess/API/C/WKPage.cpp:2277:9: warning: control
reaches end of non-void function [-Wreturn-type]
}
^
Such functions are very common in WebKit. What should be our preferred
idiom for suppressing this warning?
https://bugs.webkit.org/show_bug.cgi?id=171851 suggests this approach:
static WKAutoplayEvent toWKAutoplayEvent(WebCore::AutoplayEvent
event)
{
switch (event) {
// ...
}
ASSERT_NOT_REACHED();
return 0;
}
That works for the cases in that bug, but it won't work in this case,
because the return value here is an enum, so a cast would be needed.
I've been putting a RELEASE_ASSERT_NOT_REACHED() in the default case:
static WKAutoplayEvent toWKAutoplayEvent(WebCore::AutoplayEvent
event)
{
switch (event) {
// ...
default:
RELEASE_ASSERT_NOT_REACHED();
}
}
Of course, that would work just as well coming after the switch. This is
more general as it works for enums, but RELEASE_ASSERT_NOT_REACHED() is a
bunch of typing. I've seen CRASH() used frequently as well, but what's the
point of having RELEASE_ASSERT_NOT_REACHED() at all if we are using CRASH()
directly?
Opinions?
Bugs to close once we've decided how to handle this:
https://bugs.webkit.org/show_bug.cgi?id=171851
https://bugs.webkit.org/show_bug.cgi?id=171866
https://bugs.webkit.org/show_bug.cgi?id=171867
Michael
_______________________________________________
webkit-dev mailing list
webkit-dev at lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20170509/d7b01ecd/attachment.html>
More information about the webkit-dev
mailing list