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

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clangd

Author: Björn Pettersson (bjope)

<details>
<summary>Changes</summary>

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).

---
Full diff: https://github.com/llvm/llvm-project/pull/71162.diff


2 Files Affected:

- (modified) clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp (+2-1) 
- (added) clang-tools-extra/clangd/test/implicit-function.test (+11) 


``````````diff
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/test/implicit-function.test b/clang-tools-extra/clangd/test/implicit-function.test
new file mode 100644
index 000000000000000..85731f358b4ae8e
--- /dev/null
+++ b/clang-tools-extra/clangd/test/implicit-function.test
@@ -0,0 +1,11 @@
+// RUN: cp %s %t.c
+// RUN: not clangd -enable-config=0 -log=verbose -check=%t.c 2>&1 | FileCheck -strict-whitespace %s
+
+// Regression test for a situation when we used to hit an assertion failure
+// due to missing source location (for the implicit function declaration).
+
+// CHECK-DAG: [-Wimplicit-function-declaration] Line {{.*}}: call to undeclared function 'foo'
+// CHECK-DAG: [init_element_not_constant] Line {{.*}}: initializer element is not a compile-time constant
+// CHECK: All checks completed, 2 errors
+
+int x = foo();

``````````

</details>


https://github.com/llvm/llvm-project/pull/71162