Skip to content

[clang][modules] Fix lambda and its enclosing function are not loaded from same module. #142467

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 3, 2025

Conversation

hokein
Copy link
Collaborator

@hokein hokein commented Jun 2, 2025

This is a follow-up fix to #109167.

Previously, we stored a mapping between the enclosing function and the lambda class declaration. When loading the enclosing function, we also loaded the corresponding lambda class declaration. However, loading the lambda class declaration does not guarantee that its call operator (a CXXMethodDecl) is loaded as well.
As a result, if the lambda call operator is later loaded from a different module, we can end up with a DeclRefExpr that refers to a VarDecl from a different module — leading to inconsistencies.

To fix this, we should ensure the lambda call operator itself is loaded.

Fixes #141582

@hokein hokein requested review from dmpolukhin and ChuanqiXu9 June 2, 2025 19:44
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:modules C++20 modules and Clang Header Modules labels Jun 2, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 2, 2025

@llvm/pr-subscribers-clang-modules

@llvm/pr-subscribers-clang

Author: Haojian Wu (hokein)

Changes

This is a follow-up fix to #109167.

Previously, we stored a mapping between the enclosing function and the lambda class declaration. When loading the enclosing function, we also loaded the corresponding lambda class declaration. However, loading the lambda class declaration does not guarantee that its call operator (a CXXMethodDecl) is loaded as well.
As a result, if the lambda call operator is later loaded from a different module, we can end up with a DeclRefExpr that refers to a VarDecl from a different module — leading to inconsistencies.

To fix this, we should ensure the lambda call operator itself is loaded.

Fixes #141582


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

3 Files Affected:

  • (modified) clang/include/clang/Serialization/ASTReader.h (+1-1)
  • (modified) clang/lib/Serialization/ASTWriterDecl.cpp (+1-1)
  • (added) clang/test/Headers/crash-instantiated-in-scope-cxx-modules6.cpp (+149)
diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h
index 57b0266af26bb..9ccef6a3f39eb 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -566,7 +566,7 @@ class ASTReader
   /// modules. It is used for the following cases:
   /// - Lambda inside a template function definition: The main declaration is
   ///   the enclosing function, and the related declarations are the lambda
-  ///   declarations.
+  ///   call operators.
   /// - Friend function defined inside a template CXXRecord declaration: The
   ///   main declaration is the enclosing record, and the related declarations
   ///   are the friend functions.
diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp
index f3d260e8671b2..b16026c4ba898 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1655,7 +1655,7 @@ void ASTDeclWriter::VisitCXXRecordDecl(CXXRecordDecl *D) {
     if (auto *FD = llvm::dyn_cast_or_null<FunctionDecl>(D->getDeclContext());
         FD && isDefinitionInDependentContext(FD)) {
       Writer.RelatedDeclsMap[Writer.GetDeclRef(FD)].push_back(
-          Writer.GetDeclRef(D));
+          Writer.GetDeclRef(D->getLambdaCallOperator()));
     }
   } else {
     Record.push_back(CXXRecNotTemplate);
diff --git a/clang/test/Headers/crash-instantiated-in-scope-cxx-modules6.cpp b/clang/test/Headers/crash-instantiated-in-scope-cxx-modules6.cpp
new file mode 100644
index 0000000000000..077f69ea6598d
--- /dev/null
+++ b/clang/test/Headers/crash-instantiated-in-scope-cxx-modules6.cpp
@@ -0,0 +1,149 @@
+// RUN: rm -fR %t
+// RUN: split-file %s %t
+// RUN: cd %t
+// RUN: %clang_cc1 -std=c++20 -xc++ -emit-module -fmodules foo.cppmap -fmodule-name=foo -fmodule-name=foo -o foo.pcm
+// RUN: %clang_cc1 -std=c++20 -xc++ -emit-module -fmodules bar.cppmap -fmodule-name=bar -o bar.pcm
+// RUN: %clang_cc1 -std=c++20 -xc++ -emit-module -fmodules experiment_context.cppmap -fmodule-name=experiment_context -fmodule-file=foo.pcm -fmodule-file=bar.pcm -o experiment_context.pcm
+// RUN: %clang_cc1 -verify -std=c++20 -xc++ -fmodule-file=experiment_context.pcm experiment_context_test.cc -o experiment_context_test.o
+
+// https://github.com/llvm/llvm-project/issues/141582
+//--- bar.cppmap
+module "bar" {
+  export *
+  header "co.h"
+}
+
+//--- foo.cppmap
+module "foo" {
+  export *
+  header "co.h"
+}
+
+//--- experiment_context.cppmap
+module "experiment_context" {
+  export *
+  header "lazy.h"
+
+  use "foo"
+  use "bar"
+}
+
+//--- experiment_context_test.cc
+// expected-no-diagnostics
+#include "lazy.h"
+
+namespace c9 {
+
+template <typename T>
+void WaitForCoroutine() {
+  MakeCo<T>([]() -> Co<void> {
+    co_return;
+  });
+}
+
+void test() {
+  c9::WaitForCoroutine<void>();
+}
+}
+
+//--- lazy.h
+#pragma once
+
+#include "co.h"
+
+namespace c9 {
+template <typename T, typename F>
+Co<T> MakeCo(F f)
+{
+  co_return co_await f();
+}
+}
+
+inline c9::Co<void> DoNothing() { co_return; }
+
+
+//--- co.h
+#pragma  once
+namespace std {
+
+template <class _Ret, class... _Args>
+struct coroutine_traits {};
+
+template <typename Ret, typename... Args>
+  requires requires { typename Ret::promise_type; }
+struct coroutine_traits<Ret, Args...> {
+  using promise_type = typename Ret::promise_type;
+};
+
+template <typename Promise = void>
+struct coroutine_handle;
+
+template <>
+struct coroutine_handle<void> {};
+
+template <typename Promise = void>
+struct coroutine_handle : coroutine_handle<> {
+  static coroutine_handle from_address(void *addr);
+};
+
+struct suspend_always {
+  bool await_ready() noexcept;
+  void await_suspend(coroutine_handle<>) noexcept;
+  void await_resume() noexcept;
+};
+
+}  // namespace std
+
+namespace c9 {
+
+template <typename T>
+class Co;
+
+namespace internal {
+
+template <typename T>
+class CoroutinePromise {
+ public:
+  template <typename... Args>
+  explicit CoroutinePromise(Args&&... args) {
+    // Ensure that the 'dummy_color' VarDecl referenced by the inner DeclRefExpr
+    // is the same declaration as the one outside the lambda.
+    // This is guaranteed because both CoroutinePromise and the lambda's call operator
+    // (CXXMethodDecl) are loaded from the same module.
+    const int dummy_color = 1;
+    [&]{ (void)dummy_color; }();
+  }
+
+  ~CoroutinePromise();
+  void return_void();
+  auto get_return_object() {
+    return Co<T>();
+  }
+  void unhandled_exception();
+  std::suspend_always  initial_suspend();
+
+  struct result_t {
+    ~result_t();
+    bool await_ready() const noexcept;
+    void await_suspend(std::coroutine_handle<void>) noexcept;
+    void await_resume() const noexcept;
+  };
+
+  template <typename U>
+  result_t await_transform(Co<U> co);
+
+  std::suspend_always final_suspend() noexcept;
+};
+}  // namespace internal
+
+template <typename T>
+class Co {
+ public:
+  using promise_type = internal::CoroutinePromise<void>;
+};
+
+class CoIncomingModuleBase {
+ public:
+    Co<void> CoAfterFinish() { co_return; }
+};
+}  // namespace c9

Copy link
Member

@ChuanqiXu9 ChuanqiXu9 left a comment

Choose a reason for hiding this comment

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

LGTM

@hokein
Copy link
Collaborator Author

hokein commented Jun 3, 2025

Verified the fix in our internal codebase, no new breakages observed. Merging now.

@hokein hokein merged commit 6ca59aa into llvm:main Jun 3, 2025
14 checks passed
@hokein hokein deleted the fix-module-lambda branch June 3, 2025 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:modules C++20 modules and Clang Header Modules clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[clang] Function body and its inner lambda are loaded from different module.
3 participants