[webkit-dev] Idiom for functions with all return values in a switch case

[Alfonso Guerra]

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>