Convert LogicalAxis to enum class in WritingMode.h
Categories
(Core :: Layout, task)
Tracking
()
Tracking | Status | |
---|---|---|
firefox126 | --- | fixed |
People
(Reporter: canadahonk, Assigned: siddharthaswarnkar, Mentored)
References
Details
(Keywords: good-first-bug, Whiteboard: [lang=C++] )
Attachments
(1 file)
Split-off bug for the base Logical* enums (LogicalAxis
, LogicalEdge
, LogicalSide
, LogicalCorner
). Probably best to leave LogicalSideBits
for later.
Comment 1•3 months ago
|
||
Let's morph this bug into convert LogicalAxis
to an enum class. https://searchfox.org/mozilla-central/rev/5f0a7ca8968ac5cef8846e1d970ef178b8b76dcc/layout/generic/WritingModes.h#49-52
For other enum class conversion, I've filed individual bugs blocking bug 1610666 for them.
See https://firefox-source-docs.mozilla.org/code-quality/coding-style/coding_style_cpp.html#variable-prefixes for the coding style for enum class.
Updated•3 months ago
|
Assignee | ||
Comment 2•2 months ago
|
||
Hello, it's my first time trying to contribute. I saw a similar bug issue before, but I think it's already being worked on. I think I can fix this bug. Can this issue be assigned to me ?
Comment 3•2 months ago
|
||
Yeah, other similar bugs are good references. If you haven't had a develop environment for Firefox , you can start from https://firefox-source-docs.mozilla.org/contributing/contribution_quickref.html. Once you attach a patch to this bug, you'll be assigned to this bug automatically by a bot.
Assignee | ||
Comment 4•2 months ago
|
||
I have converted all to enum class and also followed the coding convention, but now that I am building, I am getting this error, I tried type casting as uint8_t(aAxis) but then it throws an unidentified identifier error.
0:14.59 F:/mozilla-source/mozilla-unified/obj-x86_64-pc-windows-msvc/dist/include/mozilla/WritingModes.h(346,56): error: use of undeclared identifier 'uint_8'
0:14.59 346 | return mozilla::PhysicalAxis((aWritingModeValue ^ (uint_8(aAxis))) & 0x1);
0:14.59 | ^
0:14.60 1 error generated.
^
But type casting works with this function,
inline LogicalSide MakeLogicalSide(LogicalAxis aAxis, LogicalEdge aEdge) {
return LogicalSide((uint8_t(aAxis) << 1) | aEdge);
}
How do I fix this ?
Assignee | ||
Comment 5•2 months ago
|
||
Really sorry, didn't see my typo there
Assignee | ||
Comment 6•2 months ago
|
||
LogicalAxis is one of the Logical* enums. Converting it from enum to
enum class increases type safety.
Updated•2 months ago
|
Assignee | ||
Comment 7•2 months ago
|
||
I have one last query, should I have been more descriptive in the commit ? Because I also had to update the function return types and at places had to type cast the enum class to uint8_t to match the pre-existing code.
Assignee | ||
Comment 8•2 months ago
|
||
*not function return types, only type casting
Comment 9•2 months ago
|
||
Re comment 7: yeah, it is always welcome to be more descriptive in the commit message, but in this case, type casting is somewhat expected since an enum class cannot convert to an integer automatically. I'll leave the judgement to you whether you'd want to update the commit message.
Updated•2 months ago
|
Comment 10•2 months ago
|
||
Pushed by tlin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/dea770591ed3 Converted LogicalAxis to enum class and type casted where necessary. r=TYLin
Comment 11•2 months ago
|
||
bugherder |
Description
•