refactor: Remove spurious virtual from final ~CZMQNotificationInterface #32187

1. 
   maflcko commented at 4:51 pm on April 1, 2025: member

   virtual does not make sense here, because:

   • The class is final, thus the destructor isn’t overridden in a derived class
   • The destructor also isn’t overriding the destructor of the base, clarified in commit 2b3ea39de40bc7754cab558245e4ddac1b261750
   • Clang 21 may warn about this

   0src/zmq/zmqnotificationinterface.h:25:13: error: virtual method '~CZMQNotificationInterface' is inside a 'final' class and can never be overridden [-Werror,-Wunnecessary-virtual-specifier]
   1   25 |     virtual ~CZMQNotificationInterface();
   2      |             ^
   

   Fix all issues by removing it.

2. refactor: Remove spurious virtual from final ~CZMQNotificationInterface fa69c42fdf
3. 
   DrahtBot commented at 4:51 pm on April 1, 2025: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32187.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	janb84, TheCharlatan, davidgumberg

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

4. DrahtBot added the label Refactoring on Apr 1, 2025
5. 
   maflcko commented at 6:36 pm on April 1, 2025: member

   (ci failure can be ignored and is unrelated, see #32184)
6. 
   janb84 commented at 6:41 pm on April 1, 2025: contributor

   ACK fa69c42

   The change is not crucial, but it makes the code more precise and has additional benefits.

   • Code review ✅
   • Run tests ✅
   • More extensive fuzzer run because of CI failures ✅
7. TheCharlatan approved
8. 
   TheCharlatan commented at 7:13 pm on April 1, 2025: contributor

   ACK fa69c42fdf0aeec0546e951bc6132ab630edb9d4
9. 
   davidgumberg commented at 11:26 pm on April 1, 2025: contributor

   crACK https://github.com/bitcoin/bitcoin/commit/fa69c42fdf0aeec0546e951bc6132ab630edb9d4
10. fanquake merged this on Apr 2, 2025
11. fanquake closed this on Apr 2, 2025

    ---

12. maflcko deleted the branch on Apr 2, 2025
13. 
    hebasto commented at 9:20 am on April 2, 2025: member

    • Clang 21 may warn about this

    0src/zmq/zmqnotificationinterface.h:25:13: error: virtual method '~CZMQNotificationInterface' is inside a 'final' class and can never be overridden [-Werror,-Wunnecessary-virtual-specifier]
    1   25 |     virtual ~CZMQNotificationInterface();
    2      |             ^
    

    Does Clang 21 have this warning enabled? Asking because I can’t see warnings in https://github.com/hebasto/bitcoin-core-nightly/actions/workflows/clang.yml.

14. 
    maflcko commented at 9:22 am on April 2, 2025: member

    Does Clang 21 have this warning enabled? Asking because I can’t see warnings in https://github.com/hebasto/bitcoin-core-nightly/actions/workflows/clang.yml.

    It ran 6 hours ago, but this pull was merged 7 hours ago. Thus, if you want to test it with clang-21, you’ll have to revert the pull first.

15. 
    hebasto commented at 9:26 am on April 2, 2025: member

    Does Clang 21 have this warning enabled? Asking because I can’t see warnings in https://github.com/hebasto/bitcoin-core-nightly/actions/workflows/clang.yml.

    It ran 6 hours ago, but this pull was merged 7 hours ago. Thus, if you want to test it with clang-21, you’ll have to revert the pull first.

    This one run @ 74d9598bfbc24c3b7bfe1dad5bf9d988381bf893.

16. 
    maflcko commented at 9:42 am on April 2, 2025: member

    I don’t know, but it is possibly before clang enabled the warning by default? I tested Ubuntu clang version 21.0.0 (++20250331082744+4e8fbc60710e-1~exp1~20250331202915.820)

    My recommendation would be to just test on a fresh install of everything, if you want to test this.

    I can take a look, if it fails to reproduce.

17. 
    hebasto commented at 10:51 am on April 2, 2025: member

    The warning has been reproduced for Ubuntu clang version 21.0.0 (++20250401083335+9e5bfbf77db0-1~exp1~20250401083447.821): https://github.com/hebasto/bitcoin-core-nightly/actions/runs/14217750729/job/39838245769.

    Post-merge ACK fa69c42fdf0aeec0546e951bc6132ab630edb9d4.

Contributors
maflcko DrahtBot janb84 TheCharlatan davidgumberg hebasto

Labels
Refactoring