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

[Maciej Stachowiak]

> On May 9, 2017, at 11:35 AM, Michael Catanzaro <mcatanzaro at igalia.com> wrote:
> 
> On Tue, May 9, 2017 at 1:13 PM, Maciej Stachowiak <mjs at apple.com> wrote:
>> I think this second option may suppress the warning when you have forgotten to list one of the enum values, since there is now a default case. I believe that's the reason for the recommended option.
> 
> Ah, good point. Normally we do want a warning when a new enum value has been introduced. That could be avoided by this variant:
> 
>      static WKAutoplayEvent toWKAutoplayEvent(WebCore::AutoplayEvent event)
>      {
>          switch (event) {
>          // ...
>          }
> 
>          RELEASE_ASSERT_NOT_REACHED();
>      }
> 
> That seems nicer than this:
> 
>      static WKAutoplayEvent toWKAutoplayEvent(WebCore::AutoplayEvent event)
>      {
>          switch (event) {
>          // ...
>          }
> 
>          ASSERT_NOT_REACHED();
>          return static_cast<WKAutoplayEvent>(0);
>      }
> 
> Andy suggests returning one of the enumeration values directly, then we can use ASSERT_NOT_REACHED() instead of RELEASE_ASSERT_NOT_REACHED(). That would work too, though it forces me to think about which one to pick.

The RELEASE_ASSERT_NOT_REACHED case puts in more code that we believe will be unreached. I agree the other case is more verbose. If it's likely to come up often, we could make a macro for it, something like ASSERT_RETURN_NOT_REACHED(WKAutoplayEvent) which could do a 0 cast or a default-constructed value, whichever seems more useful. That still seems a bit verbose though.

Regards,
Maciej