refactor: disable non-virtual-destructor warnings

-Wnon-virtual-dtor is noisy and we keep disabling it. I never made the mistake it tries to fix. Disable the warning codebase-wide.
quick-lint · Sep 3, 2023 · e0bbda1 · e0bbda1
1 parent 03a8af3
commit e0bbda1
Show file tree

Hide file tree

Showing 6 changed files with 26 additions and 16 deletions.
diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -141,6 +141,7 @@ quick_lint_js_configure_exception_handling()
 quick_lint_js_configure_rtti()
 quick_lint_js_work_around_implicit_link_directories()
 quick_lint_js_enable_dead_code_stripping()
+quick_lint_js_disable_unwanted_warnings()
 
 if (WIN32)
   add_definitions(-D_WIN32_WINNT=0x0602)

diff --git a/cmake/QuickLintJSCompiler.cmake b/cmake/QuickLintJSCompiler.cmake
@@ -252,6 +252,29 @@ function (quick_lint_js_enable_dead_code_stripping)
   quick_lint_js_add_cxx_linker_flag_if_supported(-Wl,--gc-sections QUICK_LINT_JS_HAVE_GC_SECTIONS)
 endfunction ()
 
+function (quick_lint_js_disable_unwanted_warnings)
+  quick_lint_js_get_supported_warning_options(
+    "-Wno-non-virtual-dtor"
+    WARNING_OPTIONS_TO_ADD
+  )
+  if (WARNING_OPTIONS_TO_ADD)
+    # HACK(strager): CMake likes to deduplicate options. This can change
+    # semantics. For example:
+    #
+    #   # NOTE: -Weffc++ implies -Wnon-virtual-dtor
+    #   -Wno-non-virtual-dtor -Weffc++ -Wno-non-virtual-dtor
+    #
+    #   # Deduplication by CMake:
+    #   -Wno-non-virtual-dtor -Weffc++
+    #   # Now -Wnon-virtual-dtor is enabled!
+    #
+    # Prevent deduplication by prefixing options with "SHELL:".
+    foreach (WARNING_OPTION IN LISTS WARNING_OPTIONS_TO_ADD)
+      add_compile_options("SHELL:${WARNING_OPTION}")
+    endforeach ()
+  endif ()
+endfunction ()
+
 function (quick_lint_js_optimize_target_for_code_size TARGET)
   quick_lint_js_target_add_c_cxx_flag_if_supported("${TARGET}" -Oz QUICK_LINT_JS_HAVE_OPTIMIZE_GNU_OZ)
   if (QUICK_LINT_JS_HAVE_OPTIMIZE_GNU_OZ_C OR QUICK_LINT_JS_HAVE_OPTIMIZE_GNU_OZ_CXX)

diff --git a/src/quick-lint-js/cli/main.cpp b/src/quick-lint-js/cli/main.cpp
@@ -56,9 +56,6 @@
 #include <unistd.h>
 #endif
 
-QLJS_WARNING_IGNORE_CLANG("-Wnon-virtual-dtor")
-QLJS_WARNING_IGNORE_GCC("-Wnon-virtual-dtor")
-
 namespace quick_lint_js {
 namespace {
 bool stderr_supports_terminal_escapes() {

diff --git a/src/quick-lint-js/io/event-loop.h b/src/quick-lint-js/io/event-loop.h
@@ -14,7 +14,6 @@
 #include <quick-lint-js/port/char8.h>
 #include <quick-lint-js/port/have.h>
 #include <quick-lint-js/port/span.h>
-#include <quick-lint-js/port/warning.h>
 
 #if QLJS_HAVE_KQUEUE
 struct kevent;
@@ -40,10 +39,6 @@ struct kevent;
 #define QLJS_EVENT_LOOP2_PIPE_WRITE 1
 #endif
 
-QLJS_WARNING_PUSH
-QLJS_WARNING_IGNORE_CLANG("-Wnon-virtual-dtor")
-QLJS_WARNING_IGNORE_GCC("-Wnon-virtual-dtor")
-
 namespace quick_lint_js {
 class Event_Loop_Base;
 
@@ -519,8 +514,6 @@ using Event_Loop =
     ;
 }
 
-QLJS_WARNING_POP
-
 #endif
 
 // quick-lint-js finds bugs in JavaScript programs.

diff --git a/test/test-event-loop.cpp b/test/test-event-loop.cpp
@@ -17,7 +17,6 @@
 #include <quick-lint-js/port/have.h>
 #include <quick-lint-js/port/kqueue.h>
 #include <quick-lint-js/port/thread.h>
-#include <quick-lint-js/port/warning.h>
 #include <quick-lint-js/util/synchronized.h>
 #include <thread>
 #include <vector>
@@ -32,9 +31,6 @@
 #include <quick-lint-js/port/windows-error.h>
 #endif
 
-QLJS_WARNING_IGNORE_CLANG("-Wnon-virtual-dtor")
-QLJS_WARNING_IGNORE_GCC("-Wnon-virtual-dtor")
-
 using ::testing::ElementsAreArray;
 using namespace std::literals::chrono_literals;