[clang-tools-extra] 5bd0f0d - [clangd] Check for valid location in ExtractionContext::exprIsValidOutside (#71162)

[via cfe-commits]

Author: Björn Pettersson
Date: 2023-11-11T01:53:18-05:00
New Revision: 5bd0f0d9d635617b335dd0b259e971a605ad6776

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

LOG: [clangd] Check for valid location in ExtractionContext::exprIsValidOutside (#71162)

If the code has a call to an implicitly declared function, an expression
could end up referencing declarations without valid source locations. So
when doing the exprIsValidOutside check we could end up calling
SourceManager::isPointWithin using invalid source locations, and then a
debug build would crash with an assertion failure in
SourceManager::isBeforeInTranslationUnit.

This patch make sure that we deal with the invalid locations (by
considering a ReferencedDecl with invalid location as not being inside
the Scope).

Added: 
    

Modified: 
    clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
    clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp b/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
index 1e4edc6d6b4bb39..79bf962544242bb 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
@@ -176,7 +176,8 @@ bool ExtractionContext::exprIsValidOutside(const clang::Stmt *Scope) const {
   SourceLocation ScopeBegin = Scope->getBeginLoc();
   SourceLocation ScopeEnd = Scope->getEndLoc();
   for (const Decl *ReferencedDecl : ReferencedDecls) {
-    if (SM.isPointWithin(ReferencedDecl->getBeginLoc(), ScopeBegin, ScopeEnd) &&
+    if (ReferencedDecl->getBeginLoc().isValid() &&
+        SM.isPointWithin(ReferencedDecl->getBeginLoc(), ScopeBegin, ScopeEnd) &&
         SM.isPointWithin(ReferencedDecl->getEndLoc(), ScopeBegin, ScopeEnd))
       return false;
   }

diff  --git a/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp
index 3d74a941071f849..eb5b06cfee43166 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp
@@ -64,6 +64,14 @@ TEST_F(ExtractVariableTest, Test) {
       int x = [[1]];
     })cpp";
   EXPECT_AVAILABLE(AvailableC);
+
+  ExtraArgs = {"-xc"};
+  const char *NoCrashCasesC = R"cpp(
+    // error-ok: broken code, but shouldn't crash
+    int x = [[foo()]];
+    )cpp";
+  EXPECT_UNAVAILABLE(NoCrashCasesC);
+
   ExtraArgs = {"-xobjective-c"};
   const char *AvailableObjC = R"cpp(
     __attribute__((objc_root_class))