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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
