[PATCH] D113558: [clang-tidy] Fix cppcoreguidelines-virtual-base-class-destructor in macros

Whisperity via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 15 00:56:55 PST 2021


whisperity accepted this revision.
whisperity added a comment.
This revision is now accepted and ready to land.

The fix is concise, thank you for observing and untangling the crash!

As the check was originally introduced in the ongoing release, we can go without a release notes entry.



================
Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp:288
+protected:
+  virtual CONCAT(~Foo, Bar2()); // no-fixit
+};
----------------
It is strange that there is no fix-it here even though the keyword appears as a single token ...[1]


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp:288
+protected:
+  virtual CONCAT(~Foo, Bar2()); // no-fixit
+};
----------------
whisperity wrote:
> It is strange that there is no fix-it here even though the keyword appears as a single token ...[1]
Maybe a FIXME could be added for this case, just to register that we indeed realised something is //strange// here, but I'm not convinced neither for nor against. The purpose of the patch is to get rid of the crash, after all.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp:299
+protected:
+  CONCAT(vir, tual) ~FooBar3();
+};
----------------
[1]... whereas here the check generates the fixit which removes the entire macro substitution!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113558/new/

https://reviews.llvm.org/D113558



More information about the cfe-commits mailing list