[all-commits] [llvm/llvm-project] 2ff370: Warn about virtual methods in `final` classes (#13...

[Devon Loehr via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 2ff370f45266b14c2a86e6395042a4574701f2d2
      https://github.com/llvm/llvm-project/commit/2ff370f45266b14c2a86e6395042a4574701f2d2
  Author: Devon Loehr <DKLoehr at users.noreply.github.com>
  Date:   2025-03-17 (Mon, 17 Mar 2025)

  Changed paths:
    M clang/docs/ReleaseNotes.rst
    M clang/include/clang/Basic/DiagnosticGroups.td
    M clang/include/clang/Basic/DiagnosticSemaKinds.td
    M clang/lib/Sema/SemaDeclCXX.cpp
    A clang/test/SemaCXX/unnecessary-virtual-specifier.cpp

  Log Message:
  -----------
  Warn about virtual methods in `final` classes (#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 currently implement the second part of the suggestion, to warn
on `virtual` methods which are never overridden anywhere. Although it's
feasible to do this for things with internal linkage (so we can check at
the end of the TU), it's more complicated to implement and it's not
clear it's worth the effort.

I tested the warning by compiling chromium and clang itself. Chromium
resulted in [277 warnings across 109
files](https://github.com/user-attachments/files/19234889/warnings-chromium.txt),
while clang had [38 warnings across 29
files](https://github.com/user-attachments/files/19234888/warnings-clang.txt).
I inspected a subset of the warning sites manually, and they all seemed
legitimate.

This warning is very easy to fix (just remove the `virtual` specifier)
and I haven't seen any false positives, so it's suitable for
on-by-default. However, I've currently made it off-by-default because it
fires at several places in the repo. I plan to submit a followup PR
fixing those places and enabling the warning by default.



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications