llvm-ci wrote:
LLVM Buildbot has detected a new failure on builder `clangd-ubuntu-tsan`
running on `clangd-ubuntu-clang` while building `clang` at step 2 "checkout".
Full details are available at:
https://lab.llvm.org/buildbot/#/builders/134/builds/15203
Here is the relevant piece of the bui
https://github.com/zmodem closed
https://github.com/llvm/llvm-project/pull/131188
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/DKLoehr updated
https://github.com/llvm/llvm-project/pull/131188
>From fbd474fb5ae3adeaf1644a4d44e916e4d7c66395 Mon Sep 17 00:00:00 2001
From: Devon Loehr
Date: Thu, 13 Mar 2025 17:34:27 +
Subject: [PATCH 1/6] Initial warning commit
---
clang/include/clang/Basic/Diagnos
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wunnecessary-virtual-specifier %s
+
+struct Foo final {
+ Foo() = default;
+ virtual ~Foo() = default; // expected-warning {{virtual
method}}
+ virtual Foo& operator=(Foo& other) = default; // ex
DKLoehr wrote:
Changed to not warn on `virtual...override`. I'm inclined to agree that it
doesn't seem worth the effort to extend this to things that aren't actually
overridden in practice.
https://github.com/llvm/llvm-project/pull/131188
___
cfe-com
DKLoehr wrote:
@zmodem Would you take a look? What do you think about implementing the
suggestion about checking if virtual methods actually get overridden or not?
https://github.com/llvm/llvm-project/pull/131188
___
cfe-commits mailing list
cfe-commi
zmodem wrote:
> What do you think about implementing the suggestion about checking if virtual
> methods actually get overridden or not?
Since we could only do it for classes with internal linkage, I think it's
probably not very valuable.
Warning about virtual functions that are not overriding
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wunnecessary-virtual-specifier %s
+
+struct Foo final {
+ Foo() = default;
+ virtual ~Foo() = default; // expected-warning {{virtual
method}}
+ virtual Foo& operator=(Foo& other) = default; // ex
@@ -2706,6 +2706,9 @@ def warn_final_dtor_non_final_class : Warning<
InGroup;
def note_final_dtor_non_final_class_silence : Note<
"mark %0 as '%select{final|sealed}1' to silence this warning">;
+def warn_unnecessary_virtual_specifier : Warning<
+ "virtual method %0 is insi
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wunnecessary-virtual-specifier %s
+
+struct Foo final {
+ Foo() = default;
+ virtual ~Foo() = default; // expected-warning {{virtual
method}}
+ virtual Foo& operator=(Foo& other) = default; // ex
https://github.com/zmodem commented:
Code and test lgtm, but I think we should consider enabling it by default.
https://github.com/llvm/llvm-project/pull/131188
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mail
https://github.com/zmodem edited
https://github.com/llvm/llvm-project/pull/131188
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -2706,6 +2706,9 @@ def warn_final_dtor_non_final_class : Warning<
InGroup;
def note_final_dtor_non_final_class_silence : Note<
"mark %0 as '%select{final|sealed}1' to silence this warning">;
+def warn_unnecessary_virtual_specifier : Warning<
+ "virtual method %0 is insi
https://github.com/cor3ntin approved this pull request.
LGTM, thanks!
Please give some time for @zmodem to also review
https://github.com/llvm/llvm-project/pull/131188
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-b
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wunnecessary-virtual-specifier %s
+
+struct Foo final {
+ Foo() = default;
+ virtual ~Foo() = default; // expected-warning {{virtual
method}}
+ virtual Foo& operator=(Foo& other) = default; // ex
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wunnecessary-virtual-specifier %s
+
+struct Foo final {
+ Foo() = default;
+ virtual ~Foo() = default; // expected-warning {{virtual
method}}
+ virtual Foo& operator=(Foo& other) = default; // ex
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wunnecessary-virtual-specifier %s
+
+struct Foo final {
+ Foo() = default;
+ virtual ~Foo() = default; // expected-warning {{virtual
method}}
+ virtual Foo& operator=(Foo& other) = default; // ex
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wunnecessary-virtual-specifier %s
+
+struct Foo final {
+ Foo() = default;
+ virtual ~Foo() = default; // expected-warning {{virtual
method}}
+ virtual Foo& operator=(Foo& other) = default; // ex
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wunnecessary-virtual-specifier %s
+
+struct Foo final {
+ Foo() = default;
+ virtual ~Foo() = default; // expected-warning {{virtual
method}}
+ virtual Foo& operator=(Foo& other) = default; // ex
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wunnecessary-virtual-specifier %s
+
+struct Foo final {
+ Foo() = default;
+ virtual ~Foo() = default; // expected-warning {{virtual
method}}
+ virtual Foo& operator=(Foo& other) = default; // ex
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wunnecessary-virtual-specifier %s
+
+struct Foo final {
+ Foo() = default;
+ virtual ~Foo() = default; // expected-warning
{{virtual method}}
+ virtual Foo& operator=(Foo& other) = defau
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wunnecessary-virtual-specifier %s
+
+struct Foo final {
+ Foo() = default;
+ virtual ~Foo() = default; // expected-warning {{virtual
method}}
+ virtual Foo& operator=(Foo& other) = default; // ex
https://github.com/DKLoehr updated
https://github.com/llvm/llvm-project/pull/131188
>From fbd474fb5ae3adeaf1644a4d44e916e4d7c66395 Mon Sep 17 00:00:00 2001
From: Devon Loehr
Date: Thu, 13 Mar 2025 17:34:27 +
Subject: [PATCH 1/5] Initial warning commit
---
clang/include/clang/Basic/Diagnos
https://github.com/DKLoehr updated
https://github.com/llvm/llvm-project/pull/131188
>From fbd474fb5ae3adeaf1644a4d44e916e4d7c66395 Mon Sep 17 00:00:00 2001
From: Devon Loehr
Date: Thu, 13 Mar 2025 17:34:27 +
Subject: [PATCH 1/4] Initial warning commit
---
clang/include/clang/Basic/Diagnos
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wunnecessary-virtual-specifier %s
+
+struct Foo final {
+ Foo() = default;
+ virtual ~Foo() = default; // expected-warning
{{virtual method}}
+ virtual Foo& operator=(Foo& other) = defau
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wunnecessary-virtual-specifier %s
+
+struct Foo final {
+ Foo() = default;
+ virtual ~Foo() = default; // expected-warning
{{virtual method}}
+ virtual Foo& operator=(Foo& other) = defau
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Devon Loehr (DKLoehr)
Changes
There's never any point to adding a `virtual` specifier to methods in a `final`
class, since the class can't be subclassed. This adds a warning when we notice
this happening, as suggested in #131108.
We don'
https://github.com/DKLoehr edited
https://github.com/llvm/llvm-project/pull/131188
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/DKLoehr created
https://github.com/llvm/llvm-project/pull/131188
There's never any point to adding a `virtual` specifier to methods in a `final`
class, since the class can't be subclassed. This adds a warning when we notice
this happening, as suggested in #131108.
We don't
29 matches
Mail list logo
