[PATCH] [clang][frontend] Fix serialization for CXXConstructorDecl (refs llvm#132794)

[koplas]

When retrieving the ExplicitSpecifier from a CXXConstructorDecl, one of its canonical declarations is returned. To correctly write the declaration record the ExplicitSpecifier of the current declaration must be used.

Failing to do so results in a crash during deserialization.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[llvmbot]

@llvm/pr-subscribers-clang-modules

@llvm/pr-subscribers-clang

Author: Paul Schwabauer (koplas)

Changes

When getting the ExplicitSpecifier of the CXXConstructorDecl, one of the canonical declaration is returned. When writing the declaration record with ExplicitSpecifier, the AllocKind of the canonical declaration has to be used.
If this is not done, it will later crash when on deserialization.

TODO: Reduce test.

---

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

2 Files Affected:

• (modified) clang/include/clang/AST/DeclCXX.h (+3-2)
• (added) clang/test/Modules/ComplexExplicitSpecifier.cpp (+45)

diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h
index dbd02ef7f8011..7728669b3bcec 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -2601,10 +2601,11 @@ class CXXConstructorDecl final
   void anchor() override;
 
   size_t numTrailingObjects(OverloadToken<InheritedConstructor>) const {
-    return CXXConstructorDeclBits.IsInheritingConstructor;
+    return getCanonicalDecl()->CXXConstructorDeclBits.IsInheritingConstructor;
   }
   size_t numTrailingObjects(OverloadToken<ExplicitSpecifier>) const {
-    return CXXConstructorDeclBits.HasTrailingExplicitSpecifier;
+    return getCanonicalDecl()
+        ->CXXConstructorDeclBits.HasTrailingExplicitSpecifier;
   }
 
   ExplicitSpecifier getExplicitSpecifierInternal() const {
diff --git a/clang/test/Modules/ComplexExplicitSpecifier.cpp b/clang/test/Modules/ComplexExplicitSpecifier.cpp
new file mode 100644
index 0000000000000..33501ba264cc6
--- /dev/null
+++ b/clang/test/Modules/ComplexExplicitSpecifier.cpp
@@ -0,0 +1,45 @@
+// Tests complex explicit constructor across modules.
+//
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/Foo.cppm \
+// RUN: -o %t/Foo.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface \
+// RUN: -fmodule-file=Foo=%t/Foo.pcm \
+// RUN: %t/Bar.cppm \
+// RUN: -o %t/Bar.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-obj \
+// RUN: -main-file-name Bar.cppm \
+// RUN: -fmodule-file=Foo=%t/Foo.pcm \
+// RUN: -x pcm %t/Bar.pcm \
+// RUN: -o %t/Bar.o
+
+//--- Foo.cppm
+export module Foo;
+
+export {
+template<class T>
+class Foo {
+  public:
+    template<class... Args>
+    explicit (sizeof...(Args) == 1) Foo(Args&&... args);
+};
+}
+
+template<class T>
+template<class... Args>
+inline Foo<T>::Foo(Args&&... args) {}
+
+//--- Bar.cppm
+export module Bar;
+import Foo;
+
+struct Bar {};
+
+void a() {
+  auto foo = Foo<Bar>{};
+}

[cor3ntin]

Thanks for working on this

This change needs a release note.
Please add an entry to clang/docs/ReleaseNotes.rst in the section the most adapted to the change, and referencing any Github issue this change fixes. Thanks!

[cor3ntin]

LGTM, Thanks!
Will you need me to merge that for you?

[koplas]

LGTM, Thanks! Will you need me to merge that for you?

Yes, I don't have any permissions to merge this.

[shafik]

I am probably missing something here but I see you switched to using getExplicitSpecifierInternal() but in your summary you talk about using the correct AllocKind but I just don't see how they related at all.

I see the line before does Record.push_back(D->getTrailingAllocKind());. I looked at ExplicitSpecifier and also don't see how it relates.

Could you please clarify?

[koplas]

@shafik, I updated the commit message based on the feedback of @cor3ntin. I have now updated the title and description to reflect the new commit message, to avoid further confusion.