It looks like the motivating enum here was `LogicalSide` per bug 1155772 comment 21. Back when TYLin filed this bug, I think the goal was to appease a g++ build warning (where g++ couldn't figure out that code after a switch statement was unreachable). In the meantime, he added a dummy return statement with MOZ_ASSERT_UNREACHABLE, to appease g++. That assertion is still there: https://searchfox.org/mozilla-central/rev/f9beb753a84aa297713d1565dcd0c5e3c66e4174/layout/generic/WritingModes.h#1296-1297,1311-1312 I think we don't build with g++ in automation anymore; we only use clang++. We do presumably still intend to build successfully with g++, but I don't think we aim to expend effort on maintaining a **build-warning-free** g++ build anymore. So: it's possible we could just remove that dummy code after the switch statement (possibly in a separate bug) and simplify things a bit here.
Bug 1610666 Comment 1 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
It looks like the motivating enum here was `LogicalSide` per bug 1155772 comment 21. Back when TYLin filed this bug, I think the goal was to appease a g++ build warning (where g++ couldn't figure out that code after a switch statement was unreachable). In the meantime, he added a dummy return statement with MOZ_ASSERT_UNREACHABLE, to appease g++. That assertion is still there: https://searchfox.org/mozilla-central/rev/f9beb753a84aa297713d1565dcd0c5e3c66e4174/layout/generic/WritingModes.h#1296-1297,1311-1312 I think we don't build with g++ in automation anymore; we only use clang++. We do presumably still intend to build successfully with g++, but I don't think we aim to expend effort on maintaining a **build-warning-free** g++ build anymore. So: it's possible we could just remove that dummy code after the switch statement (possibly in a separate bug) and simplify things a bit here (which would then removing that motivating-factor for this bug -- but possibly the enum-class-conversion would still be worth doing here, for type safety purposes).
Background: t looks like the motivating enum here was `LogicalSide` per bug 1155772 comment 21. Back when TYLin filed this bug, I think the goal was to appease a g++ build warning (where g++ couldn't figure out that code after a switch statement was unreachable). In the meantime, he added a dummy return statement with MOZ_ASSERT_UNREACHABLE, to appease g++. That assertion is still there: https://searchfox.org/mozilla-central/rev/f9beb753a84aa297713d1565dcd0c5e3c66e4174/layout/generic/WritingModes.h#1296-1297,1311-1312 ~I think we don't build with g++ in automation anymore; we only use clang++. We do presumably still intend to build successfully with g++, but I don't think we aim to expend effort on maintaining a **build-warning-free** g++ build anymore.~ So: it's possible we could just remove that dummy code after the switch statement (possibly in a separate bug) and simplify things a bit here (which would then removing that motivating-factor for this bug -- but possibly the enum-class-conversion would still be worth doing here, for type safety purposes). [Edit: thanks for the correction in comment 2; I've added a ~strikethrough~ here for clarity.]