1610666 - [Meta] Convert enums to enum classes in WritingMode.h

Status: RESOLVED FIXED

(Core :: Layout, task, P3)

Description

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Filed per https://phabricator.services.mozilla.com/D59051#1847027. The enums worth converting are LogicalAxis, LogicalEdge, LogicalSide, etc.

Comment 1

[Daniel Holbert [:dholbert]]

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.]

Comment 2

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

We are still building with g++ in automation. For example, the Bb job (build-linux64-base-toolchains) is g++. https://treeherder.mozilla.org/jobs?repo=mozilla-central&selectedTaskRun=UhgcMgWQQdSCObFodOCcRw.0 But it is possible that we have a newer g++ version that is smart enough to figure out that code after a switch statement was indeed unreachable.

Comment 3

[Oliver Medhurst [:canadahonk]]

Converting into a meta bug for split-off bugs for select enums to ease reviewing and management.

Comment 4

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

All the enums in WritingModes.h are converted. Let's call this fixed.

Comment 5

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

There are still some enums in WritingMode class. If the numeric values are not important, they are worth converting to enum class.
https://searchfox.org/mozilla-central/rev/a3d520fca1a1ca29992cb3806245db25e41a8617/layout/generic/WritingModes.h#158-185

Comment 6

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Bug 1892778 fixed comment 5.