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•2 years 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•2 years ago
|
| Assignee | ||
Comment 2•2 years 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 years 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 years 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 years ago
|
||
Really sorry, didn't see my typo there
| Assignee | ||
Comment 6•2 years ago
|
||
LogicalAxis is one of the Logical* enums. Converting it from enum to
enum class increases type safety.
Updated•2 years ago
|
| Assignee | ||
Comment 7•2 years 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 years ago
|
||
*not function return types, only type casting
Comment 9•2 years 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 years ago
|
Comment 10•2 years ago
|
||
Comment 11•2 years ago
|
||
| bugherder | ||
Description
•