1698127 - Fix clang static-analysis and formatting issues

Status: RESOLVED FIXED

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

Description

[Ryan Hunt [:rhunt]]

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.

Comment 1

[Ryan Hunt [:rhunt]]

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.

Comment 2

[Ryan Hunt [:rhunt]]

Attachment: Bug 1698127 - Run clang-format on js/src/. r?jandem

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

Comment 3

[Ryan Hunt [:rhunt]]

Attachment: Bug 1698127 - wasm: Use range-based for loops. r?lth

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

Comment 4

[Ryan Hunt [:rhunt]]

Attachment: Bug 1698127 - wasm: Use `using` instead of `typedef`. r?lth

Depends on D108204

Comment 5

[Ryan Hunt [:rhunt]]

Attachment: Bug 1698127 - wasm: Make sure parameter declaration names match parameter definition names. r?lth

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

Comment 6

[Ryan Hunt [:rhunt]]

Attachment: Bug 1698127 - wasm: Transform `if` statements to `return`s when trivially possible. r?lth

Depends on D108206

Comment 7

[Ryan Hunt [:rhunt]]

Attachment: Bug 1698127 - wasm: Use auto* instead of auto in some places. r?lth

Depends on D108207

Comment 8

[Ryan Hunt [:rhunt]]

Attachment: Bug 1698127 - wasm: Apply `const` specifiers to functions when applicable. r?lth

Depends on D108208

Comment 9

[Ryan Hunt [:rhunt]]

Attachment: Bug 1698127 - wasm: Default initialize variables of Nothing. r?lth

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

Comment 10

[Ryan Hunt [:rhunt]]

Attachment: Bug 1698127 - wasm: Add braces to single line `if` statements. r?lth

Depends on D108210

Comment 11

[Ryan Hunt [:rhunt]]

Attachment: Bug 1698127 - wasm: Add default, noexcept specifiers to some constructors. r?lth

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

Comment 12

[Ryan Hunt [:rhunt]]

Attachment: Bug 1698127 - wasm: Use std::forward and std::move when we can. r?lth

Depends on D108212

Comment 13

[Ryan Hunt [:rhunt]]

Attachment: Bug 1698127 - wasm: Explicitly examine result of memcmp(). r?lth

Depends on D108213

Comment 14

[Ryan Hunt [:rhunt]]

Attachment: Bug 1698127 - wasm: Add parentheses around macro terms. r?lth

Depends on D108214

Comment 15

[Pulsebot]

Pushed by [e-mail]:
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

Comment 16

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/ea08486942c8
https://hg.mozilla.org/mozilla-central/rev/d0a0f6f44e87
https://hg.mozilla.org/mozilla-central/rev/67ed32df18ab
https://hg.mozilla.org/mozilla-central/rev/1580c7515537
https://hg.mozilla.org/mozilla-central/rev/807faa5f9eea
https://hg.mozilla.org/mozilla-central/rev/af8ab06489e0
https://hg.mozilla.org/mozilla-central/rev/fb0c175ffba3
https://hg.mozilla.org/mozilla-central/rev/4dc5cd8f68da
https://hg.mozilla.org/mozilla-central/rev/16d47ee5c12f
https://hg.mozilla.org/mozilla-central/rev/8567eb45819a
https://hg.mozilla.org/mozilla-central/rev/4a0c57aceb12
https://hg.mozilla.org/mozilla-central/rev/299625448252