WorryFree Computers   »   [go: up one dir, main page]

Closed Bug 1825386 Opened 1 year ago Closed 2 months ago

Convert LogicalAxis to enum class in WritingMode.h

Categories

(Core :: Layout, task)

task

Tracking

()

RESOLVED FIXED
126 Branch
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.

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.

Mentor: aethanyc
Summary: Convert base Logical* enums to enum classes in WritingMode.h → Convert LogicalAxis to enum class in WritingMode.h
Whiteboard: [lang=C++]

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 ?

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.

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 ?

Really sorry, didn't see my typo there

LogicalAxis is one of the Logical* enums. Converting it from enum to
enum class increases type safety.

Assignee: nobody → siddharthaswarnkar
Status: NEW → ASSIGNED

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.

*not function return types, only type casting

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.

Attachment #9393968 - Attachment description: Bug 1825386 - Convert LogicalAxis to enum class in WritingMode.h. r=TYLin → Bug 1825386 - Converted LogicalAxis to enum class and type casted where necessary. r=TYLin
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
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → 126 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: