> On May 9, 2017, at 11:35 AM, Michael Catanzaro <mcatanz...@igalia.com> wrote:
>
> On Tue, May 9, 2017 at 1:13 PM, Maciej Stachowiak <m...@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
_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev
