Closed Bug 1698127 Opened 2 months ago Closed 2 months ago

Fix clang static-analysis and formatting issues

Categories

(Core :: Javascript: WebAssembly, task, P3)

task

Tracking

()

RESOLVED FIXED
88 Branch
Tracking Status
firefox88 --- fixed

People

(Reporter: rhunt, Assigned: rhunt)

Details

Attachments

(12 files, 1 obsolete file)

48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review

There are some formatting and static-analysis issues in js/src/wasm.

None of them are significant, but they are noise that appears on every commit when it's being reviewed that touches code near these issues.

The patches are the result of running ./mach static-analysis check --fix --path js/src/wasm, then grouping the changes in themes and filtering out dubious changes.

I'm open to debate on whether we should do all of these changes, the goal is really to lower the amount of preexisting issues when we're reviewing new code.

Run ./mach clang-format --path js/src/ to fix style issues
that have been committed.

Assignee: nobody → rhunt
Status: NEW → ASSIGNED

clang static-analysis prefers to use range-based for loops whenever
it is possible. Some of the replacements are obvious improvements,
while others are a bit murky. Open to discussion on whether these
all make sense.

Depends on D108203

Depends on D108204

The autofix algorithm for this rule appears to use the definition parameter
name when there is a mismatch. This seems reasonable to me from glancing at
the results.

Depends on D108205

clang static-analysis complains if you have a variable that is
not initialized at its definition, even if it's always initialized
before it's first use. I don't think it's worth trying to find
a way to appease this warning at this time.

This commit adds initializers to all instances of Nothing to
remove some of these warnings, as this is a trivial case to
fix.

Depends on D108209

I don't know why clang static-analysis wants noexcept specifiers on
these TypeDef methods, but it's trivially true so I don't see an issue
with it.

Depends on D108211

Depends on D108214

Attachment #9208723 - Attachment description: Bug 1698127 - wasm: Explicitly examine result of memcpy(). r?lth → Bug 1698127 - wasm: Explicitly examine result of memcmp(). r?lth
Severity: -- → N/A
Priority: -- → P3
Attachment #9208712 - Attachment is obsolete: true
Pushed by rhunt@eqrion.net:
https://hg.mozilla.org/integration/autoland/rev/ea08486942c8
wasm: Use range-based for loops. r=lth
https://hg.mozilla.org/integration/autoland/rev/d0a0f6f44e87
wasm: Use `using` instead of `typedef`. r=lth
https://hg.mozilla.org/integration/autoland/rev/67ed32df18ab
wasm: Make sure parameter declaration names match parameter definition names. r=lth
https://hg.mozilla.org/integration/autoland/rev/1580c7515537
wasm: Transform `if` statements to `return`s when trivially possible. r=lth
https://hg.mozilla.org/integration/autoland/rev/807faa5f9eea
wasm: Use auto* instead of auto in some places. r=lth
https://hg.mozilla.org/integration/autoland/rev/af8ab06489e0
wasm: Apply `const` specifiers to functions when applicable. r=lth
https://hg.mozilla.org/integration/autoland/rev/fb0c175ffba3
wasm: Default initialize variables of Nothing. r=lth
https://hg.mozilla.org/integration/autoland/rev/4dc5cd8f68da
wasm: Add braces to single line `if` statements. r=lth
https://hg.mozilla.org/integration/autoland/rev/16d47ee5c12f
wasm: Add default, noexcept specifiers to some constructors. r=lth
https://hg.mozilla.org/integration/autoland/rev/8567eb45819a
wasm: Use std::forward and std::move when we can. r=lth
https://hg.mozilla.org/integration/autoland/rev/4a0c57aceb12
wasm: Explicitly examine result of memcmp(). r=lth
https://hg.mozilla.org/integration/autoland/rev/299625448252
wasm: Add parentheses around macro terms. r=lth
You need to log in before you can comment on or make changes to this bug.