Skip to content

Conversation

Michael137
Copy link
Member

@Michael137 Michael137 commented Aug 22, 2025

This patch works around an assertion that we hit in the LambdaExpr constructor when we call it from ASTNodeImporter::VisitLambdaExpr (see #149477). The lambda that we imported doesn't have the NumCaptures field accurately set to the one on the source decl. This is because in MinimalImport mode, we skip importing of lambda definitions:

(To->isLambda() && shouldForceImportDeclContext(Kind))) {

In practice we have seen this assertion occur in our import-std-module test-suite when libc++ headers decide to use lambdas inside inline function bodies (the latest failure being caused by #144602).

To avoid running into this whenever libc++ decides to use lambdas in headers, this patch skips ASTImport of lambdas alltogether. Ideally this would bubble up to the user or log as an error, but we swallow the ASTImportErrors currently. The only way this codepath is hit is when lambdas are used inside functions in defined in the expression evaluator, or when importing AST nodes from Clang modules. Both of these are very niche use-cases (for now), so a workaround seemed appropriate.

@llvmbot llvmbot added the lldb label Aug 22, 2025
@Michael137 Michael137 marked this pull request as draft August 22, 2025 14:49
@llvmbot
Copy link
Member

llvmbot commented Aug 22, 2025

@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)

Changes

Works around #149477


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

2 Files Affected:

  • (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp (+10)
  • (added) lldb/test/Shell/Expr/TestLambdaExprImport.test (+26)
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
index 08e2d0f1b4011..92094c01e2a54 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
@@ -1048,6 +1048,16 @@ ClangASTImporter::MapCompleter::~MapCompleter() = default;
 
 llvm::Expected<Decl *>
 ClangASTImporter::ASTImporterDelegate::ImportImpl(Decl *From) {
+  // FIXME: The Minimal import mode of clang::ASTImporter does not correctly
+  // import Lambda definitions. Work around this for now by not importing
+  // lambdas at all. This is most likely encountered when importing decls from
+  // the `std` module (not from debug-info), where lambdas can be defined in
+  // inline function bodies. Those will be imported by LLDB.
+  if (const auto *CXX = llvm::dyn_cast<clang::CXXRecordDecl>(From))
+    if (CXX->isLambda())
+      return llvm::make_error<ASTImportError>(
+          ASTImportError::UnsupportedConstruct);
+
   if (m_std_handler) {
     std::optional<Decl *> D = m_std_handler->Import(From);
     if (D) {
diff --git a/lldb/test/Shell/Expr/TestLambdaExprImport.test b/lldb/test/Shell/Expr/TestLambdaExprImport.test
new file mode 100644
index 0000000000000..c57ce06453fe2
--- /dev/null
+++ b/lldb/test/Shell/Expr/TestLambdaExprImport.test
@@ -0,0 +1,26 @@
+# Test that we can successfully ASTImport clang::LambdaExpr nodes.
+# Currently this is not supported in MinimalImport mode (which LLDB
+# uses always).
+
+# RUN: split-file %s %t
+# RUN: %clang_host -g -gdwarf %t/main.cpp -o %t.out
+# RUN: %lldb -o "settings set interpreter.stop-command-source-on-error false" \
+# RUN:       -x -b -s %t/commands.input %t.out 2>&1 \
+# RUN:       | FileCheck %s
+
+#--- main.cpp
+
+int main() {
+  __builtin_debugtrap();
+}
+
+#--- commands.input
+
+run
+expression --top-level -- void method(int x) { [x=x] { ; }; }
+target dump typesystem
+
+# CHECK:      expression
+# CHECK:      target dump typesystem
+# CHECK-NOT:  FunctionDecl
+# CHECK-NOT:  LambdaExpr

Copy link
Collaborator

@adrian-prantl adrian-prantl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fit unbreaks the test suite, let's do this for now.

@Michael137 Michael137 enabled auto-merge (squash) August 22, 2025 19:30
@Michael137 Michael137 merged commit 0bbb794 into llvm:main Aug 22, 2025
9 checks passed
@Michael137 Michael137 deleted the lldb/lambda-import-crash branch August 22, 2025 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants