[Meta] Convert enums to enum classes in WritingMode.h
Categories
(Core :: Layout, task, P3)
Tracking
()
People
(Reporter: TYLin, Unassigned)
References
(Depends on 1 open bug, Blocks 1 open bug)
Details
(Keywords: meta)
Filed per https://phabricator.services.mozilla.com/D59051#1847027. The enums worth converting are LogicalAxis
, LogicalEdge
, LogicalSide
, etc.
Updated•1 year ago
|
Comment 1•6 months ago
•
|
||
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.]
Reporter | ||
Comment 2•6 months ago
|
||
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•6 months ago
|
||
Converting into a meta bug for split-off bugs for select enums to ease reviewing and management.
Description
•