> 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