Open Bug 1583717 Opened 2 months ago Updated 2 months ago

Enable -Wempty-init-stmt warnings after we compile with -std=c++17

Categories

(Firefox Build System :: General, task, P3)

Tracking

(Not tracked)

People

(Reporter: cpeterson, Unassigned)

References

(Depends on 1 open bug)

Details

After we compile with -std=c++17, we should enable clang 8.0's new -Wempty-init-stmt flag for empty initializers in if and switch statements in C++17 code.

http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0305r1.html

http://releases.llvm.org/8.0.0/tools/clang/docs/ReleaseNotes.html#major-new-features

-Wempty-init-stmt is a new diagnostic that diagnoses empty init-statements of if, switch, range-based for, unless: the semicolon directly follows a macro that was expanded to nothing or if the semicolon is within the macro itself (both macros from system headers, and normal macros).

void test() {
  if(; // <- warning: init-statement of 'if' is a null statement
     true)
    ;

  switch (; // <- warning: init-statement of 'switch' is a null statement
          x) {
    ...
  }

  for (; // <- warning: init-statement of 'range-based for' is a null statement
       int y : S())
    ;
}

https://clang.llvm.org/docs/DiagnosticsReference.html#wempty-init-stmt

As mozilla-central currently has no C++17 code, we have zero -Wempty-init-stmt warnings. :) However, enabling -Wempty-init-stmt implicitly enables clang 8.0's new -Wextra-semi-stmt flag and we have 4626 of those warnings. :( So this patch to enable -Wempty-init-stmt will also need to suppress -Wextra-semi-stmt warnings like:

# catches empty if/switch/for initialization statements that have no effect
add_gcc_warning('-Wempty-init-stmt', cxx_compiler) # C++17 warning
add_gcc_warning('-Wno-extra-semi-stmt')

(In reply to Chris Peterson [:cpeterson] from comment #0)

As mozilla-central currently has no C++17 code, we have zero -Wempty-init-stmt warnings. :) However, enabling -Wempty-init-stmt implicitly enables clang 8.0's new -Wextra-semi-stmt flag and we have 4626 of those warnings. :(

-Wextra-semi-stmt seems pretty pedantic. :( But possibly not that hard to get rid of?

(In reply to Nathan Froyd [:froydnj] from comment #1)

-Wextra-semi-stmt seems pretty pedantic. :( But possibly not that hard to get rid of?

Yes. Most of these warnings are about macros with unnecessary trailing semicolons. They would be simple to fix, but would touch a lot of files and none of them seem to be actual bugs that would cause problems.

js/src/wasm/WasmBaselineCompile.cpp:10923:36 [-Wextra-semi-stmt] empty expression statement has no effect; remove unnecessary ';' to silence this warning

https://searchfox.org/mozilla-central/rev/f43ae7e1c43a4a940b658381157a6ea6c5a185c1/js/src/wasm/WasmBaselineCompile.cpp#10923

js/src/wasm/WasmBaselineCompile.cpp:10923:36 [-Wextra-semi-stmt] empty expression statement has no effect; remove unnecessary ';' to silence this warning

https://searchfox.org/mozilla-central/rev/f43ae7e1c43a4a940b658381157a6ea6c5a185c1/js/src/wasm/WasmBaselineCompile.cpp#10923

We could argue that in that case, it is a bug in clang...

You need to log in before you can comment on or make changes to this bug.