[clang-tools-extra] b2cb7e8 - [clang-tidy] cppcoreguidelines-virtual-class-destructor: Fix crash when "virtual" keyword is expanded from a macro

[via cfe-commits]

Author: Joachim Priesner
Date: 2022-06-25T15:50:13-06:00
New Revision: b2cb7e81f8978ccce5c5e7b316480b88785ca9cc

URL: https://github.com/llvm/llvm-project/commit/b2cb7e81f8978ccce5c5e7b316480b88785ca9cc
DIFF: https://github.com/llvm/llvm-project/commit/b2cb7e81f8978ccce5c5e7b316480b88785ca9cc.diff

LOG: [clang-tidy] cppcoreguidelines-virtual-class-destructor: Fix crash when "virtual" keyword is expanded from a macro

Check llvm::Optional before dereferencing it.

Compute VirtualEndLoc differently to avoid an assertion failure
in clang::SourceManager::getFileIDLoaded:

Assertion `0 && "Invalid SLocOffset or bad function choice"' failed

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
index e730fb6fa01d..92b38bcfd81d 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
@@ -54,13 +54,17 @@ getVirtualKeywordRange(const CXXDestructorDecl &Destructor,
     return None;
 
   SourceLocation VirtualBeginLoc = Destructor.getBeginLoc();
-  SourceLocation VirtualEndLoc = VirtualBeginLoc.getLocWithOffset(
-      Lexer::MeasureTokenLength(VirtualBeginLoc, SM, LangOpts));
+  SourceLocation VirtualBeginSpellingLoc =
+      SM.getSpellingLoc(Destructor.getBeginLoc());
+  SourceLocation VirtualEndLoc = VirtualBeginSpellingLoc.getLocWithOffset(
+      Lexer::MeasureTokenLength(VirtualBeginSpellingLoc, SM, LangOpts));
 
   /// Range ends with \c StartOfNextToken so that any whitespace after \c
   /// virtual is included.
-  SourceLocation StartOfNextToken =
-      Lexer::findNextToken(VirtualEndLoc, SM, LangOpts)->getLocation();
+  Optional<Token> NextToken = Lexer::findNextToken(VirtualEndLoc, SM, LangOpts);
+  if (!NextToken)
+    return None;
+  SourceLocation StartOfNextToken = NextToken->getLocation();
 
   return CharSourceRange::getCharRange(VirtualBeginLoc, StartOfNextToken);
 }

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/virtual-class-destructor.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/virtual-class-destructor.cpp
index fcf558dcac8e..61e565075b15 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/virtual-class-destructor.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/virtual-class-destructor.cpp
@@ -272,6 +272,7 @@ DerivedFromTemplateNonVirtualBaseStruct2Typedef InstantiationWithPublicNonVirtua
 } // namespace Bugzilla_51912
 
 namespace macro_tests {
+#define MY_VIRTUAL virtual
 #define CONCAT(x, y) x##y
 
 // CHECK-MESSAGES: :[[@LINE+2]]:7: warning: destructor of 'FooBar1' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
@@ -317,8 +318,17 @@ class FooBar5 {
 protected:
   XMACRO(CONCAT(vir, tual), ~CONCAT(Foo, Bar5());) // no-crash, no-fixit
 };
+
+// CHECK-MESSAGES: :[[@LINE+2]]:7: warning: destructor of 'FooBar6' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+1]]:7: note: make it protected and non-virtual
+class FooBar6 {
+protected:
+  MY_VIRTUAL ~FooBar6(); // FIXME: We should have a fixit for this.
+};
+
 #undef XMACRO
 #undef CONCAT
+#undef MY_VIRTUAL
 } // namespace macro_tests
 
 namespace FinalClassCannotBeBaseClass {