From 3fa5eff5f2647185652afd3fc831f6666e3b11fb Mon Sep 17 00:00:00 2001 From: Devon Loehr Date: Tue, 6 May 2025 17:28:48 +0000 Subject: [PATCH 1/3] Add unnecessary-virtual-specifier to -Wextra --- clang/docs/ReleaseNotes.rst | 6 +++--- clang/include/clang/Basic/DiagnosticGroups.td | 8 ++++---- llvm/cmake/modules/HandleLLVMOptions.cmake | 5 +++++ 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index a4107bfaf913d..498c4f295343c 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -434,9 +434,9 @@ Improvements to Clang's diagnostics - The ``-Wsign-compare`` warning now treats expressions with bitwise not(~) and minus(-) as signed integers except for the case where the operand is an unsigned integer and throws warning if they are compared with unsigned integers (##18878). -- The ``-Wunnecessary-virtual-specifier`` warning has been added to warn about - methods which are marked as virtual inside a ``final`` class, and hence can - never be overridden. +- The ``-Wunnecessary-virtual-specifier`` warning (included in ``-Wextra``) has + been added to warn about methods which are marked as virtual inside a + ``final`` class, and hence can never be overridden. - Improve the diagnostics for chained comparisons to report actual expressions and operators (#GH129069). diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 1faf8508121f4..bb6e7070b1574 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -421,13 +421,12 @@ def CXX11WarnSuggestOverride : DiagGroup<"suggest-override">; def WarnUnnecessaryVirtualSpecifier : DiagGroup<"unnecessary-virtual-specifier"> { code Documentation = [{ Warns when a ``final`` class contains a virtual method (including virtual -destructors). Since ``final`` classes cannot be subclassed, their methods -cannot be overridden, and hence the ``virtual`` specifier is useless. +destructors) that does not override anything. Since ``final`` classes cannot +be subclassed, their methods cannot be overridden, so there is no point to +introducing new ``virtual`` methods. The warning also detects virtual methods in classes whose destructor is ``final``, for the same reason. - -The warning does not fire on virtual methods which are also marked ``override``. }]; } @@ -1163,6 +1162,7 @@ def Extra : DiagGroup<"extra", [ FUseLdPath, CastFunctionTypeMismatch, InitStringTooLongMissingNonString, + WarnUnnecessaryVirtualSpecifier, ]>; def Most : DiagGroup<"most", [ diff --git a/llvm/cmake/modules/HandleLLVMOptions.cmake b/llvm/cmake/modules/HandleLLVMOptions.cmake index 8b3303fe9f3d2..c427a65ee030c 100644 --- a/llvm/cmake/modules/HandleLLVMOptions.cmake +++ b/llvm/cmake/modules/HandleLLVMOptions.cmake @@ -882,6 +882,11 @@ if (LLVM_ENABLE_WARNINGS AND (LLVM_COMPILER_IS_GCC_COMPATIBLE OR CLANG_CL)) # The LLVM libraries have no stable C++ API, so -Wnoexcept-type is not useful. append("-Wno-noexcept-type" CMAKE_CXX_FLAGS) + # LLVM has a policy of including virtual "anchor" functions to control + # where the vtable is emitted. In `final` classes, these are exactly what + # this warning detects: unnecessary virtual methods. + add_flag_if_supported("-Wno-unnecessary-virtual-specifier" CXX_SUPPORTS_UNNECESSARY_VIRTUAL_FLAG) + if (CMAKE_CXX_COMPILER_ID MATCHES "Clang") append("-Wnon-virtual-dtor" CMAKE_CXX_FLAGS) endif() From 56baa26dbf94a0fd804ce9310a6c9ed68ebf741c Mon Sep 17 00:00:00 2001 From: Devon Loehr Date: Tue, 6 May 2025 19:24:41 +0000 Subject: [PATCH 2/3] Testing commit for CI -- DO NOT MERGE --- libcxx/src/hash.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/libcxx/src/hash.cpp b/libcxx/src/hash.cpp index 41c4eb480a5fc..61ae441bc583e 100644 --- a/libcxx/src/hash.cpp +++ b/libcxx/src/hash.cpp @@ -16,6 +16,7 @@ _LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wtautological-constant-out-of-range-compare") _LIBCPP_BEGIN_NAMESPACE_STD namespace { +// Whitespace change DO NOT MERGE // handle all next_prime(i) for i in [1, 210), special case 0 const unsigned small_primes[] = { From a88e7770eff153491373c38f3517dcf3a4054252 Mon Sep 17 00:00:00 2001 From: Devon Loehr Date: Wed, 7 May 2025 13:49:32 +0000 Subject: [PATCH 3/3] Revert "Testing commit for CI -- DO NOT MERGE" This reverts commit 56baa26dbf94a0fd804ce9310a6c9ed68ebf741c. --- libcxx/src/hash.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/libcxx/src/hash.cpp b/libcxx/src/hash.cpp index 61ae441bc583e..41c4eb480a5fc 100644 --- a/libcxx/src/hash.cpp +++ b/libcxx/src/hash.cpp @@ -16,7 +16,6 @@ _LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wtautological-constant-out-of-range-compare") _LIBCPP_BEGIN_NAMESPACE_STD namespace { -// Whitespace change DO NOT MERGE // handle all next_prime(i) for i in [1, 210), special case 0 const unsigned small_primes[] = {