[clang] [C++20][Modules] Fix crash/compiler error due broken AST links (PR #123648)

[Dmitry Polukhin via cfe-commits]

https://github.com/dmpolukhin created https://github.com/llvm/llvm-project/pull/123648

Summary:
This PR fixes bugreport https://github.com/llvm/llvm-project/issues/122493 The root problem is the same as before lambda function and DeclRefExpr references a variable that does not belong to the same module as the enclosing function body. Therefore iteration over the function body doesn’t visit the VarDecl. Before this change RelatedDeclsMap was created only for canonical decl but in reality it has to be done for the definition of the function that does not always match the canonical decl.

Test Plan: check-clang

>From e5cd06ddbc4193f9d5910eba93f0eb309d67063c Mon Sep 17 00:00:00 2001
From: Dmitry Polukhin <dmitry.polukhin at gmail.com>
Date: Mon, 20 Jan 2025 09:03:25 -0800
Subject: [PATCH] [C++20][Modules] Fix crash/compiler error due broken AST
 links
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Summary:
This PR fixes bugreport https://github.com/llvm/llvm-project/issues/122493
The root problem is the same as before lambda function and DeclRefExpr references
a variable that does not belong to the same module as the enclosing function body.
Therefore iteration over the function body doesn’t visit the VarDecl. Before this
change RelatedDeclsMap was created only for canonical decl but in reality it has to
be done for the definition of the function that does not always match the canonical decl.

Test Plan: check-clang
---
 clang/lib/Serialization/ASTWriterDecl.cpp     |  5 +-
 ...ash-instantiated-in-scope-cxx-modules5.cpp | 92 +++++++++++++++++++
 2 files changed, 95 insertions(+), 2 deletions(-)
 create mode 100644 clang/test/Headers/crash-instantiated-in-scope-cxx-modules5.cpp

diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp
index f8ed155ca389d7..371b4b29e85991 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1571,9 +1571,10 @@ void ASTDeclWriter::VisitCXXRecordDecl(CXXRecordDecl *D) {
     } else {
       Record.push_back(0);
     }
-    // For lambdas inside canonical FunctionDecl remember the mapping.
+    // For lambdas inside template functions, remember the mapping to
+    // deserialize them together.
     if (auto FD = llvm::dyn_cast_or_null<FunctionDecl>(D->getDeclContext());
-        FD && FD->isCanonicalDecl()) {
+        FD && FD->isDependentContext() && FD->isThisDeclarationADefinition()) {
       Writer.RelatedDeclsMap[Writer.GetDeclRef(FD)].push_back(
           Writer.GetDeclRef(D));
     }
diff --git a/clang/test/Headers/crash-instantiated-in-scope-cxx-modules5.cpp b/clang/test/Headers/crash-instantiated-in-scope-cxx-modules5.cpp
new file mode 100644
index 00000000000000..ca80a3fd21dd00
--- /dev/null
+++ b/clang/test/Headers/crash-instantiated-in-scope-cxx-modules5.cpp
@@ -0,0 +1,92 @@
+// RUN: rm -fR %t
+// RUN: split-file %s %t
+// RUN: cd %t
+// RUN: %clang_cc1 -verify -std=c++20 -xc++ -emit-module module.cppmap -fmodule-name=mock_resolver -o mock_resolver.pcm
+// RUN: %clang_cc1 -verify -std=c++20 -xc++ -emit-module module.cppmap -fmodule-name=sql_internal -o sql_internal.pcm
+// RUN: %clang_cc1 -verify -std=c++20 -xc++ -fmodule-file=mock_resolver.pcm -fmodule-file=sql_internal.pcm main.cc -o main.o
+
+//--- module.cppmap
+module "mock_resolver" {
+  export *
+  module "mock_resolver.h" {
+    export *
+    header "mock_resolver.h"
+  }
+}
+
+module "sql_internal" {
+  export *
+  module "sql_transform_builder.h" {
+    export *
+    header "sql_transform_builder.h"
+  }
+}
+
+//--- set_bits2.h
+// expected-no-diagnostics
+#pragma once
+
+template <typename T>
+void fwd(const T& x) {}
+
+namespace vox::bitset {
+
+template <typename TFunc>
+void ForEachSetBit2(const TFunc&) {
+  fwd([](int) {
+    const int bit_index_base = 0;
+    (void)[&](int) {
+      int v = bit_index_base;
+    };
+  });
+}
+
+}  // namespace vox::bitset
+
+//--- sql_transform_builder.h
+// expected-no-diagnostics
+#pragma once
+
+#include "set_bits2.h"
+
+class QualifyingSet3 {
+ public:
+  void GetIndexes() const {
+    vox::bitset::ForEachSetBit2([]() {});
+  }
+};
+
+template <typename T>
+void DoTransform() {
+  vox::bitset::ForEachSetBit2([]() {});
+}
+
+//--- mock_resolver.h
+// expected-no-diagnostics
+#pragma once 
+#include "set_bits2.h"
+
+class QualifyingSet2 {
+ public:
+  void GetIndexes() const {
+    vox::bitset::ForEachSetBit2([]() {});
+  }
+};
+
+//--- main.cc
+// expected-no-diagnostics
+#include "sql_transform_builder.h"
+
+template <typename Callable>
+void get(const Callable& fn) {
+  fwd<Callable>(fn);
+}
+
+namespace {
+
+void test() {
+  get([]() {});
+  DoTransform<int>();
+}
+
+} // namespace