Skip to content

Commit 04676c6

Browse files
authored
Revert "Enable unnecessary-virtual-specifier by default" (#134105)
Reverts #133265 This causes the whole libc++ CI to fail, since we're not building against a compiler built from current trunk. Specifically, the CMake changes causes some feature detection to fail, resulting in CMake being unable to configure libc++.
1 parent ee60f7c commit 04676c6

File tree

7 files changed

+9
-16
lines changed

7 files changed

+9
-16
lines changed

clang/include/clang/Basic/DiagnosticGroups.td

+4-3
Original file line numberDiff line numberDiff line change
@@ -377,12 +377,13 @@ def CXX11WarnSuggestOverride : DiagGroup<"suggest-override">;
377377
def WarnUnnecessaryVirtualSpecifier : DiagGroup<"unnecessary-virtual-specifier"> {
378378
code Documentation = [{
379379
Warns when a ``final`` class contains a virtual method (including virtual
380-
destructors) that does not override anything. Since ``final`` classes cannot be
381-
subclassed, their methods cannot be overridden, so there is no point to
382-
introducing new ``virtual`` methods.
380+
destructors). Since ``final`` classes cannot be subclassed, their methods
381+
cannot be overridden, and hence the ``virtual`` specifier is useless.
383382

384383
The warning also detects virtual methods in classes whose destructor is
385384
``final``, for the same reason.
385+
386+
The warning does not fire on virtual methods which are also marked ``override``.
386387
}];
387388
}
388389

clang/include/clang/Basic/DiagnosticSemaKinds.td

+1-1
Original file line numberDiff line numberDiff line change
@@ -2733,7 +2733,7 @@ def note_final_dtor_non_final_class_silence : Note<
27332733
"mark %0 as '%select{final|sealed}1' to silence this warning">;
27342734
def warn_unnecessary_virtual_specifier : Warning<
27352735
"virtual method %0 is inside a 'final' class and can never be overridden">,
2736-
InGroup<WarnUnnecessaryVirtualSpecifier>;
2736+
InGroup<WarnUnnecessaryVirtualSpecifier>, DefaultIgnore;
27372737

27382738
// C++11 attributes
27392739
def err_repeat_attribute : Error<"%0 attribute cannot be repeated">;

clang/test/Analysis/Checkers/WebKit/ref-cntbl-crtp-base-no-virtual-dtor.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %clang_analyze_cc1 -analyzer-checker=webkit.RefCntblBaseVirtualDtor -verify %s -Wno-unnecessary-virtual-specifier
1+
// RUN: %clang_analyze_cc1 -analyzer-checker=webkit.RefCntblBaseVirtualDtor -verify %s
22

33
#include "mock-types.h"
44

clang/test/CXX/class/p2-0x.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ struct C : A<int> { }; // expected-error {{base 'A' is marked 'final'}}
2828

2929
namespace Test4 {
3030

31-
struct A final { virtual void func() = 0; }; // expected-warning {{abstract class is marked 'final'}} expected-note {{unimplemented pure virtual method 'func' in 'A'}} expected-warning {{virtual method 'func' is inside a 'final' class}}}
31+
struct A final { virtual void func() = 0; }; // expected-warning {{abstract class is marked 'final'}} expected-note {{unimplemented pure virtual method 'func' in 'A'}}
3232
struct B { virtual void func() = 0; }; // expected-note {{unimplemented pure virtual method 'func' in 'C'}}
3333

3434
struct C final : B { }; // expected-warning {{abstract class is marked 'final'}}

clang/test/SemaCXX/MicrosoftExtensions.cpp

-1
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,6 @@ struct InheritFromSealed : SealedType {};
470470
class SealedDestructor { // expected-note {{mark 'SealedDestructor' as 'sealed' to silence this warning}}
471471
// expected-warning@+1 {{'sealed' keyword is a Microsoft extension}}
472472
virtual ~SealedDestructor() sealed; // expected-warning {{class with destructor marked 'sealed' cannot be inherited from}}
473-
// expected-warning@-1 {{virtual method '~SealedDestructor' is inside a 'final' class}}
474473
};
475474

476475
// expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}

clang/test/SemaCXX/warn-final-dtor-non-final-class.cpp

+2-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
1-
// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify %s -Wfinal-dtor-non-final-class -Wno-unnecessary-virtual-specifier
2-
// RUN: %clang_cc1 -fsyntax-only -std=c++11 %s -Wfinal-dtor-non-final-class -Wno-unnecessary-virtual-specifier \
3-
// RUN: -fdiagnostics-parseable-fixits 2>&1 | FileCheck %s
1+
// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify %s -Wfinal-dtor-non-final-class
2+
// RUN: %clang_cc1 -fsyntax-only -std=c++11 %s -Wfinal-dtor-non-final-class -fdiagnostics-parseable-fixits 2>&1 | FileCheck %s
43

54
class A {
65
~A();

llvm/cmake/modules/HandleLLVMOptions.cmake

-6
Original file line numberDiff line numberDiff line change
@@ -690,12 +690,6 @@ endif( LLVM_COMPILER_IS_GCC_COMPATIBLE OR CMAKE_CXX_COMPILER_ID MATCHES "XL" )
690690

691691
if (CMAKE_CXX_COMPILER_ID MATCHES "Clang")
692692
append("-Werror=unguarded-availability-new" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
693-
if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 21.0)
694-
# LLVM has a policy of including virtual "anchor" functions to control
695-
# where the vtable is emitted. In `final` classes, these are exactly what
696-
# this warning detects: unnecessary virtual methods.
697-
append("-Wno-unnecessary-virtual-specifier" CMAKE_CXX_FLAGS)
698-
endif()
699693
endif()
700694

701695
if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU" AND LLVM_ENABLE_LTO)

0 commit comments

Comments
 (0)