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

[Balázs Benics via Phabricator via cfe-commits]

steakhal updated this revision to Diff 387189.
steakhal marked 2 inline comments as done.
steakhal added a comment.

Add fixme comment for:

  class FooBar2 {
  protected:
    virtual CONCAT(~Foo, Bar2()); // FIXME: We should have a fixit for this.
  };


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

https://reviews.llvm.org/D113558

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
@@ -270,3 +270,53 @@
 DerivedFromTemplateNonVirtualBaseStruct2Typedef InstantiationWithPublicNonVirtualBaseStruct2;
 
 } // namespace Bugzilla_51912
+
+namespace macro_tests {
+#define CONCAT(x, y) x##y
+
+// CHECK-MESSAGES: :[[@LINE+2]]:7: warning: destructor of 'FooBar1' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+1]]:7: note: make it protected and non-virtual
+class FooBar1 {
+protected:
+  CONCAT(vir, tual) CONCAT(~Foo, Bar1()); // no-fixit
+};
+
+// CHECK-MESSAGES: :[[@LINE+2]]:7: warning: destructor of 'FooBar2' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+1]]:7: note: make it protected and non-virtual
+class FooBar2 {
+protected:
+  virtual CONCAT(~Foo, Bar2()); // FIXME: We should have a fixit for this.
+};
+
+// CHECK-MESSAGES: :[[@LINE+6]]:7: warning: destructor of 'FooBar3' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+5]]:7: note: make it protected and non-virtual
+// CHECK-FIXES:      class FooBar3 {
+// CHECK-FIXES-NEXT: protected:
+// CHECK-FIXES-NEXT:   ~FooBar3();
+// CHECK-FIXES-NEXT: };
+class FooBar3 {
+protected:
+  CONCAT(vir, tual) ~FooBar3();
+};
+
+// CHECK-MESSAGES: :[[@LINE+6]]:7: warning: destructor of 'FooBar4' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+5]]:7: note: make it protected and non-virtual
+// CHECK-FIXES:      class FooBar4 {
+// CHECK-FIXES-NEXT: protected:
+// CHECK-FIXES-NEXT:   ~CONCAT(Foo, Bar4());
+// CHECK-FIXES-NEXT: };
+class FooBar4 {
+protected:
+  CONCAT(vir, tual) ~CONCAT(Foo, Bar4());
+};
+
+// CHECK-MESSAGES: :[[@LINE+3]]:7: warning: destructor of 'FooBar5' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+2]]:7: note: make it protected and non-virtual
+#define XMACRO(COLUMN1, COLUMN2) COLUMN1 COLUMN2
+class FooBar5 {
+protected:
+  XMACRO(CONCAT(vir, tual), ~CONCAT(Foo, Bar5());) // no-crash, no-fixit
+};
+#undef XMACRO
+#undef CONCAT
+} // namespace macro_tests
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
@@ -46,9 +46,12 @@
       this);
 }
 
-static CharSourceRange
+static Optional<CharSourceRange>
 getVirtualKeywordRange(const CXXDestructorDecl &Destructor,
                        const SourceManager &SM, const LangOptions &LangOpts) {
+  if (Destructor.getLocation().isMacroID())
+    return None;
+
   SourceLocation VirtualBeginLoc = Destructor.getBeginLoc();
   SourceLocation VirtualEndLoc = VirtualBeginLoc.getLocWithOffset(
       Lexer::MeasureTokenLength(VirtualBeginLoc, SM, LangOpts));
@@ -190,8 +193,10 @@
       Fix = FixItHint::CreateInsertion(Destructor->getLocation(), "virtual ");
     } else if (Destructor->getAccess() == AccessSpecifier::AS_protected) {
       ProtectedAndVirtual = true;
-      Fix = FixItHint::CreateRemoval(getVirtualKeywordRange(
-          *Destructor, *Result.SourceManager, Result.Context->getLangOpts()));
+      if (const auto MaybeRange =
+              getVirtualKeywordRange(*Destructor, *Result.SourceManager,
+                                     Result.Context->getLangOpts()))
+        Fix = FixItHint::CreateRemoval(*MaybeRange);
     }
   } else {
     Fix = generateUserDeclaredDestructor(*MatchedClassOrStruct,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D113558.387189.patch
Type: text/x-patch
Size: 3958 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20211115/72599010/attachment-0001.bin>