Closed
Bug 1317973
Opened 7 years ago
Closed 7 years ago
Convert some code in widget/ to C++11
Categories
(Developer Infrastructure :: Source Code Analysis, defect)
Developer Infrastructure
Source Code Analysis
Tracking
(firefox53 fixed)
RESOLVED
FIXED
mozilla53
Tracking | Status | |
---|---|---|
firefox53 | --- | fixed |
People
(Reporter: Sylvestre, Assigned: Sylvestre)
References
Details
Attachments
(4 files)
Using the checkers provided by clang-tidy we can refresh the code to make it use the features of C++11 like: - auto variables declarations - default CTORS and DTORS - using new range loop operators
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8811258 -
Flags: review?(vladimir)
Attachment #8811261 -
Flags: review?(vladimir)
Attachment #8811260 -
Flags: review?(vladimir)
Attachment #8811259 -
Flags: review?(vladimir)
Assignee | ||
Updated•7 years ago
|
Summary: Run clang-tidy on widget/ module → Convert some code in widget/ to C++11
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8811258 -
Flags: review?(nical.bugzilla)
Attachment #8811261 -
Flags: review?(vladimir) → review?(nical.bugzilla)
Attachment #8811260 -
Flags: review?(vladimir)
Attachment #8811259 -
Flags: review?(vladimir)
Assignee | ||
Updated•7 years ago
|
Attachment #8811260 -
Flags: review?(tnikkel)
Attachment #8811259 -
Flags: review?(tnikkel)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → sledru
Comment 16•7 years ago
|
||
mozreview-review |
Comment on attachment 8811259 [details] Bug 1317973 - Use auto type specifier for variable declarations to improve code readability and maintainability https://reviewboard.mozilla.org/r/93440/#review96684 Is this the style that we are going with now? I slightly prefer the existing code because the type is explicit and I don't have to guess the type when reading the code. But if there is consensus I won't go against it.
Comment 17•7 years ago
|
||
mozreview-review |
Comment on attachment 8811260 [details] Bug 1317973 - Replace default bodies of special member functions with = default; https://reviewboard.mozilla.org/r/93442/#review96686
Attachment #8811260 -
Flags: review?(tnikkel) → review+
Assignee | ||
Comment 18•7 years ago
|
||
(In reply to Timothy Nikkel (:tnikkel) from comment #16) > Comment on attachment 8811259 [details] > Is this the style that we are going with now? I slightly prefer the existing > code because the type is explicit and I don't have to guess the type when > reading the code. But if there is consensus I won't go against it. We used the same approach as in LLVM: http://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable tldr: when the type is explicitly set on the right part of the expression, we use auto. If it requires more thinking, we don't. And yeah, we did that in various other places but we will start a thread on dev-platform about that to integrate this into the coding style
Comment 19•7 years ago
|
||
mozreview-review |
Comment on attachment 8811259 [details] Bug 1317973 - Use auto type specifier for variable declarations to improve code readability and maintainability https://reviewboard.mozilla.org/r/93440/#review96898 Alright if this is or will be the style.
Attachment #8811259 -
Flags: review?(tnikkel) → review+
Comment 20•7 years ago
|
||
mozreview-review |
Comment on attachment 8811258 [details] Bug 1317973 - Converts for(...; ...; ...) loops to use the new range-based loops in C++11. https://reviewboard.mozilla.org/r/93438/#review97082 ::: widget/gtk/nsWindow.cpp:500 (Diff revision 3) > } > > /* static */ void > nsWindow::ReleaseGlobals() > { > - for (uint32_t i = 0; i < ArrayLength(gCursorCache); ++i) { > + for (auto & i : gCursorCache) { There is a strong cognitive bias towards reading variables named "i" as integer indices. I would prefer loop variables like this one to be named "item" or something like that.
Attachment #8811258 -
Flags: review?(nical.bugzilla) → review+
Comment 21•7 years ago
|
||
mozreview-review |
Comment on attachment 8811261 [details] Bug 1317973 - Replace integer literals which are cast to bool. https://reviewboard.mozilla.org/r/93444/#review97084
Attachment #8811261 -
Flags: review?(nical.bugzilla) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 26•7 years ago
|
||
Pushed by sledru@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/8b1670f7a0b4 Converts for(...; ...; ...) loops to use the new range-based loops in C++11. r=nical https://hg.mozilla.org/integration/autoland/rev/d2bfabe4d015 Replace integer literals which are cast to bool. r=nical https://hg.mozilla.org/integration/autoland/rev/8023e756661a Replace default bodies of special member functions with = default; r=tnikkel https://hg.mozilla.org/integration/autoland/rev/9c229cf88ec2 Use auto type specifier for variable declarations to improve code readability and maintainability r=tnikkel
Comment 27•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/8b1670f7a0b4 https://hg.mozilla.org/mozilla-central/rev/d2bfabe4d015 https://hg.mozilla.org/mozilla-central/rev/8023e756661a https://hg.mozilla.org/mozilla-central/rev/9c229cf88ec2
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Updated•6 years ago
|
Product: Core → Firefox Build System
Updated•2 years ago
|
Product: Firefox Build System → Developer Infrastructure
You need to log in
before you can comment on or make changes to this bug.
Description
•