<div dir="auto"><div><br><div class="gmail_extra"><br><div class="gmail_quote">On May 9, 2017 2:07 PM, "Michael Catanzaro" <<a href="mailto:mcatanzaro@igalia.com">mcatanzaro@igalia.com</a>> wrote:<br type="attribution"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br>
<br>
Consider this function:<br>
<br>
       static WKAutoplayEvent toWKAutoplayEvent(WebCore::Aut<wbr>oplayEvent event)<br>
       {<br>
           switch (event) {<br>
           case WebCore::AutoplayEvent::DidEnd<wbr>MediaPlaybackWithoutUserInterf<wbr>erence:<br>
               return kWKAutoplayEventDidEndMediaPla<wbr>ybackWithoutUserInterference;<br>
           case WebCore::AutoplayEvent::DidPla<wbr>yMediaPreventedFromPlaying:<br>
               return kWKAutoplayEventDidPlayMediaPr<wbr>eventedFromAutoplaying;<br>
           case WebCore::AutoplayEvent::DidPre<wbr>ventMediaFromPlaying:<br>
               return kWKAutoplayEventDidPreventFrom<wbr>Autoplaying;<br>
           case WebCore::AutoplayEvent::UserDi<wbr>dInterfereWithPlayback:<br>
               return kWKAutoplayEventUserDidInterfe<wbr>reWithPlayback;<br>
           case WebCore::AutoplayEvent::UserNe<wbr>verPlayedMediaPreventedFromPla<wbr>ying:<br>
               return kWKAutoplayEventUserNeverPlaye<wbr>dMediaPreventedFromPlaying;<br>
           }<br>
       }<br></blockquote></div></div></div><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto">Out of curiosity, why is using a switch statement better than defining an array to hold the return values?</div><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto"><span style="font-family:sans-serif">Alfonso Guerra</span><br style="font-family:sans-serif"><span style="font-family:sans-serif">Founder/CEO</span><br style="font-family:sans-serif"><span style="font-family:sans-serif">Apokalypse Software Corp</span><br style="font-family:sans-serif"><span style="font-family:sans-serif">@Huperniketes</span><br style="font-family:sans-serif"><span style="font-family:sans-serif">(626) 667-4285</span><br></div><div dir="auto"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
It will trigger this GCC warning:<br>
<br>
[3490/4357] Building CXX object Source...bKit2.dir/UIProcess/A<wbr>PI/C/WKPage.cpp.o<br>
../../Source/WebKit2/UIProcess<wbr>/API/C/WKPage.cpp: In static member function ‘static WKAutoplayEvent WKPageSetPageUIClient(WKPageRe<wbr>f, const WKPageUIClientBase*)::UIClient<wbr>::toWKAutoplayEvent(WebCore::<wbr>AutoplayEvent)’:<br>
../../Source/WebKit2/UIProcess<wbr>/API/C/WKPage.cpp:2277:9: warning: control reaches end of non-void function [-Wreturn-type]<br>
        }<br>
        ^<br>
<br>
Such functions are very common in WebKit. What should be our preferred idiom for suppressing this warning?<br>
<br>
<a href="https://bugs.webkit.org/show_bug.cgi?id=171851" rel="noreferrer" target="_blank">https://bugs.webkit.org/show_b<wbr>ug.cgi?id=171851</a> suggests this approach:<br>
<br>
       static WKAutoplayEvent toWKAutoplayEvent(WebCore::Aut<wbr>oplayEvent event)<br>
       {<br>
           switch (event) {<br>
               // ...<br>
           }<br>
<br>
           ASSERT_NOT_REACHED();<br>
           return 0;<br>
       }<br>
<br>
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.<br>
<br>
I've been putting a RELEASE_ASSERT_NOT_REACHED() in the default case:<br>
<br>
       static WKAutoplayEvent toWKAutoplayEvent(WebCore::Aut<wbr>oplayEvent event)<br>
       {<br>
           switch (event) {<br>
           // ...<br>
           default:<br>
               RELEASE_ASSERT_NOT_REACHED();<br>
           }<br>
       }<br>
<br>
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?<br>
<br>
Opinions?<br>
<br>
Bugs to close once we've decided how to handle this:<br>
<br>
<a href="https://bugs.webkit.org/show_bug.cgi?id=171851" rel="noreferrer" target="_blank">https://bugs.webkit.org/show_b<wbr>ug.cgi?id=171851</a><br>
<a href="https://bugs.webkit.org/show_bug.cgi?id=171866" rel="noreferrer" target="_blank">https://bugs.webkit.org/show_b<wbr>ug.cgi?id=171866</a><br>
<a href="https://bugs.webkit.org/show_bug.cgi?id=171867" rel="noreferrer" target="_blank">https://bugs.webkit.org/show_b<wbr>ug.cgi?id=171867</a><br>
<br>
Michael<br>
<br>
______________________________<wbr>_________________<br>
webkit-dev mailing list<br>
<a href="mailto:webkit-dev@lists.webkit.org" target="_blank">webkit-dev@lists.webkit.org</a><br>
<a href="https://lists.webkit.org/mailman/listinfo/webkit-dev" rel="noreferrer" target="_blank">https://lists.webkit.org/mailm<wbr>an/listinfo/webkit-dev</a><br>
</blockquote></div><br></div></div></div>