[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

2 files changed, 18 insertions, 4 deletions

diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
index e730fb6..92b38bc 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 fcf558d..61e5650 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 {