Static analysis fixes in dom/base/

RESOLVED FIXED in Firefox 56

Status

()

Core
General
RESOLVED FIXED
4 months ago
4 months ago

People

(Reporter: janx, Assigned: janx)

Tracking

(Blocks: 2 bugs)

unspecified
mozilla56
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 fixed)

Details

Attachments

(4 attachments)

(Assignee)

Description

4 months ago
There are a few larges files in dom/base/ [0] that present opportunities for quick automated improvements using clang-tidy [1].

[0] For example:
- dom/base/nsContentUtils.cpp
- dom/base/nsDOMWindowUtils.cpp
- dom/base/nsGlobalWindow.cpp

[1] Using auto-fixable rules like:
- modernize-use-nullptr
- readability-else-after-return
- performance-for-range-copy
(Assignee)

Comment 1

4 months ago
Created attachment 8881616 [details] [diff] [review]
Use nullptr where possible in dom/base/ (clang-tidy: modernize-use-nullptr).
(Assignee)

Comment 2

4 months ago
Created attachment 8881617 [details] [diff] [review]
Don't use 'else' after 'break' or 'return' in dom/base/ (clang-tidy: readability-else-after-return).
(Assignee)

Comment 3

4 months ago
Created attachment 8881618 [details] [diff] [review]
Change loop variable copies to const references where possible in dom/base/ (clang-tidy: performance-for-range-copy).
(Assignee)

Comment 4

4 months ago
Created attachment 8881619 [details] [diff] [review]
Annotate methods with 'override' where possible in dom/base/ (clang-tidy: modernize-use-override).
(Assignee)

Comment 5

4 months ago
Comment on attachment 8881619 [details] [diff] [review]
Annotate methods with 'override' where possible in dom/base/ (clang-tidy: modernize-use-override).

Review of attachment 8881619 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/nsContentUtils.cpp
@@ +538,5 @@
>    NS_DECL_NSIOBSERVER
>  
>    void Init();
>    void Shutdown();
> +  void AnnotateHang(HangMonitor::HangAnnotations& aAnnotations) override;

Note: This is a slightly unrelated drive-by fix ('virtual' is unnecessary because 'override' already implies it).
(Assignee)

Comment 6

4 months ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b4b92e2cc6f7
(Assignee)

Comment 7

4 months ago
Comment on attachment 8881616 [details] [diff] [review]
Use nullptr where possible in dom/base/ (clang-tidy: modernize-use-nullptr).

Ehsan, please have a look when convenient.

Also, please indicate if any changes are not worth the effort / shouldn't be pursued.
Attachment #8881616 - Flags: review?(ehsan)
(Assignee)

Updated

4 months ago
Attachment #8881617 - Flags: review?(ehsan)
(Assignee)

Updated

4 months ago
Attachment #8881618 - Flags: review?(ehsan)
(Assignee)

Updated

4 months ago
Attachment #8881619 - Flags: review?(ehsan)

Updated

4 months ago
Attachment #8881616 - Flags: review?(ehsan) → review+

Updated

4 months ago
Attachment #8881617 - Flags: review?(ehsan) → review+

Updated

4 months ago
Attachment #8881618 - Flags: review?(ehsan) → review+

Updated

4 months ago
Attachment #8881619 - Flags: review?(ehsan) → review+
(Assignee)

Comment 8

4 months ago
Thank you Ehsan!

Sheriffs, please land these fixes.
Keywords: checkin-needed

Comment 9

4 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/075df18a0a58
Use nullptr where possible in dom/base/ (clang-tidy: modernize-use-nullptr). r=ehsan
https://hg.mozilla.org/integration/mozilla-inbound/rev/b7021a785963
Don't use 'else' after 'break' or 'return' in dom/base/ (clang-tidy: readability-else-after-return). r=ehsan
https://hg.mozilla.org/integration/mozilla-inbound/rev/ab7c82c4c34c
Change loop variable copies to const references where possible in dom/base/ (clang-tidy: performance-for-range-copy). r=ehsan
https://hg.mozilla.org/integration/mozilla-inbound/rev/7095cd0f91c7
Annotate methods with 'override' where possible in dom/base/ (clang-tidy: modernize-use-override). r=ehsan
Keywords: checkin-needed

Comment 10

4 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/075df18a0a58
https://hg.mozilla.org/mozilla-central/rev/b7021a785963
https://hg.mozilla.org/mozilla-central/rev/ab7c82c4c34c
https://hg.mozilla.org/mozilla-central/rev/7095cd0f91c7
Status: NEW → RESOLVED
Last Resolved: 4 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.