C++: turn on -Weverything (#4104)

### What

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested [demo.rerun.io](https://demo.rerun.io/pr/4104) (if
applicable)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/4104)
- [Docs
preview](https://rerun.io/preview/d084501dddbe035eb3022b1b7afd80cfbdc2fb24/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/d084501dddbe035eb3022b1b7afd80cfbdc2fb24/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://ref.rerun.io/dev/bench/)
- [Wasm size tracking](https://ref.rerun.io/dev/sizes/)

Author: emilk

.clang-format

@@ -19,6 +19,7 @@ DerivePointerAlignment: false
 EmptyLineBeforeAccessModifier: LogicalBlock
 IndentWidth: 4
 IndentWrappedFunctionNames: true
+InsertBraces: true
 InsertTrailingCommas: Wrapped
 MaxEmptyLinesToKeep: 1
 NamespaceIndentation: All

.github/workflows/documentation.yaml

@@ -10,7 +10,7 @@ permissions:
   contents: read
 
 jobs:
-  # BEWARE(streetsidesoftware/cspell-action#187): a misconfigured cspell will _not_ have a non-zero exit code...
+  # BEWARE(streetsidesoftware/cspell-action#187): a misconfigured cspell will _not_ have a non-zero exit code…
   spellcheck:
     name: Spellcheck
     runs-on: ubuntu-latest
@@ -34,7 +34,7 @@ jobs:
           paths: "**/*.c, **/*.cpp, **/*.fbs, **/*.h, **/*.hpp, **/*.js, **/*.md, **/*.py, **/*.rs, **/*.sh, **/*.toml, **/*.txt, **/*.wgsl, **/*.yml"
 
           # Avoid crates.io rate-limiting, skip changelog PR links (so many), and skip speculative links
-          # TODO(##4085): https://rerun-io.github.io/rerun/dev/bench/ often 404:s for unknown reasons
+          # TODO(#4085): https://rerun-io.github.io/rerun/dev/bench/ often 404:s for unknown reasons
           linksToSkip: "https://crates.io/crates/.*, https://github.com/rerun-io/rerun/pull/.*, .*?speculative-link, https://rerun-io.github.io/rerun/dev/bench/"
           retry: true
           retryErrors: true

CMakeLists.txt

@@ -30,18 +30,40 @@ function(set_default_warning_settings target)
             -Wpointer-arith
             -Wshadow
             -Wswitch-enum
+            -Wunreachable-code
             -Wvla
         )
 
         if(CMAKE_CXX_COMPILER_ID MATCHES "Clang") # match both "Clang" and "AppleClang"
             target_compile_options(${target} PRIVATE
                 -Wc++17-compat-pedantic
                 -Wc++20-compat-pedantic
-                -Wpre-c2x-compat-pedantic
                 -Wc99-extensions
+                -Weverything
                 -Wgnu
                 -Wnon-gcc
+                -Wpre-c2x-compat-pedantic
                 -Wshadow-all
+
+                # Turn off some warning that -Weverything turns on:
+                -Wno-c++98-compat
+                -Wno-c++98-compat-pedantic
+                -Wno-covered-switch-default # We often add a `default:` case out of paranoia
+                -Wno-ctad-maybe-unsupported
+                -Wno-disabled-macro-expansion
+                -Wno-documentation
+                -Wno-documentation-unknown-command
+                -Wno-double-promotion # float->double is nothing to whine about
+                -Wno-exit-time-destructors
+                -Wno-float-equal # comparing floats is fine
+                -Wno-global-constructors
+                -Wno-missing-prototypes
+                -Wno-padded
+                -Wno-reserved-id-macro
+                -Wno-reserved-identifier
+                -Wno-unreachable-code-break # TODO(emilk): remove this exception - we only need this because of codegen
+                -Wno-unreachable-code-return # TODO(emilk): remove this exception - we only need this because of codegen
+                -Wno-unused-macros
             )
         endif()
 

examples/cpp/clock/main.cpp

@@ -50,5 +50,5 @@ int main() {
         log_hand(rec, "seconds", step, (step % 60) / 60.0f, LENGTH_S, WIDTH_S, 0);
         log_hand(rec, "minutes", step, (step % 3600) / 3600.0f, LENGTH_M, WIDTH_M, 128);
         log_hand(rec, "hours", step, (step % 43200) / 43200.0f, LENGTH_H, WIDTH_H, 255);
-    };
+    }
 }

pixi.toml

@@ -98,7 +98,7 @@ cpp-build-and-test-all = { depends_on = ["cpp-build-all", "cpp-test"] }
 [dependencies]
 attrs = ">=23.1.0"
 blackdoc = "0.3.8"
-clang-tools = ">=15,<16"
+clang-tools = ">=15,<16"       # clang-format
 cmake = "3.27.6"
 flatbuffers = ">=23"
 gitignore-parser = ">=0.1.9"

rerun_cpp/src/rerun/arrow.cpp

@@ -7,8 +7,8 @@
 
 namespace rerun {
     Result<std::shared_ptr<arrow::Buffer>> ipc_from_table(const arrow::Table& table) {
-        ARROW_ASSIGN_OR_RAISE(auto output, arrow::io::BufferOutputStream::Create());
-        ARROW_ASSIGN_OR_RAISE(auto writer, arrow::ipc::MakeStreamWriter(output, table.schema()));
+        ARROW_ASSIGN_OR_RAISE(auto output, arrow::io::BufferOutputStream::Create())
+        ARROW_ASSIGN_OR_RAISE(auto writer, arrow::ipc::MakeStreamWriter(output, table.schema()))
         ARROW_RETURN_NOT_OK(writer->WriteTable(table));
         ARROW_RETURN_NOT_OK(writer->Close());
 

rerun_cpp/src/rerun/datatypes/translation_rotation_scale3d.hpp

@@ -28,7 +28,8 @@ namespace rerun {
                 : translation(other.translation),
                   rotation(other.rotation),
                   scale(other.scale),
-                  from_parent(other.from_parent){};
+                  from_parent(other.from_parent) {}
+
             DISABLE_MAYBE_UNINITIALIZED_POP
 
             /// Creates a new 3D transform from translation/rotation/scale.

rerun_cpp/src/rerun/datatypes/translation_rotation_scale3d_ext.cpp

@@ -282,8 +282,8 @@ SCENARIO("RecordingStream can log to file", TEST_TAG) {
     std::string test_rrd0 = std::string(test_path) + "test-file-0.rrd";
     std::string test_rrd1 = std::string(test_path) + "test-file-1.rrd";
 
-    fs::remove(test_rrd0.c_str());
-    fs::remove(test_rrd1.c_str());
+    fs::remove(test_rrd0);
+    fs::remove(test_rrd1);
 
     GIVEN("a new RecordingStream") {
         auto stream0 = std::make_unique<rerun::RecordingStream>("test");
@@ -299,16 +299,16 @@ SCENARIO("RecordingStream can log to file", TEST_TAG) {
         // }
         AND_GIVEN("valid save path " << test_rrd0) {
             AND_GIVEN("a directory already existing at this path") {
-                fs::create_directory(test_rrd0.c_str());
+                fs::create_directory(test_rrd0);
                 THEN("then the save call fails") {
                     CHECK(
-                        stream0->save(test_rrd0.c_str()).code ==
+                        stream0->save(test_rrd0).code ==
                         rerun::ErrorCode::RecordingStreamSaveFailure
                     );
                 }
             }
             THEN("save call returns no error") {
-                REQUIRE(stream0->save(test_rrd0.c_str()).is_ok());
+                REQUIRE(stream0->save(test_rrd0).is_ok());
 
                 THEN("a new file got immediately created") {
                     CHECK(fs::exists(test_rrd0));
@@ -318,7 +318,7 @@ SCENARIO("RecordingStream can log to file", TEST_TAG) {
                     auto stream1 = std::make_unique<rerun::RecordingStream>("test2");
 
                     WHEN("saving that one to a different file " << test_rrd1) {
-                        REQUIRE(stream1->save(test_rrd1.c_str()).is_ok());
+                        REQUIRE(stream1->save(test_rrd1).is_ok());
 
                         WHEN("logging a component to the second stream") {
                             check_logged_error([&] {

rerun_cpp/tests/recording_stream.cpp

@@ -57,7 +57,7 @@ def lint_line(line: str, file_extension: str = "rs", is_in_docstring: bool = Fal
     if "NOLINT" in line:
         return None  # NOLINT ignores the linter
 
-    if file_extension not in ("py", "txt", "yml"):
+    if file_extension not in ("py", "txt", "yaml", "yml"):
         if "Github" in line:
             return "It's 'GitHub', not 'Github'"
 
@@ -706,6 +706,7 @@ def main() -> None:
         "toml",
         "txt",
         "wgsl",
+        "yaml",
         "yml",
     ]