[clang] Fix assertion failure in constexpr union deserialization

[alexfh]

This commit fixes #140130

[llvmbot]

@llvm/pr-subscribers-clang-modules

Author: Alexander Kornienko (alexfh)

Changes

This commit fixes #140130

---

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

2 Files Affected:

• (modified) clang/include/clang/AST/PropertiesBase.td (+1-1)
• (added) clang/test/Modules/pr140130.cpp (+33)

diff --git a/clang/include/clang/AST/PropertiesBase.td b/clang/include/clang/AST/PropertiesBase.td
index 33336d57b6298..f3f0039ffe33e 100644
--- a/clang/include/clang/AST/PropertiesBase.td
+++ b/clang/include/clang/AST/PropertiesBase.td
@@ -414,7 +414,7 @@ let Class = PropertyTypeCase<APValue, "Union"> in {
     let Read = [{ node.getUnionValue() }];
   }
   def : Creator<[{
-    return APValue(cast<clang::FieldDecl>(fieldDecl), std::move(value));
+    return APValue(cast_if_present<clang::FieldDecl>(fieldDecl), std::move(value));
   }]>;
 }
 let Class = PropertyTypeCase<APValue, "AddrLabelDiff"> in {
diff --git a/clang/test/Modules/pr140130.cpp b/clang/test/Modules/pr140130.cpp
new file mode 100644
index 0000000000000..da26a005b04f8
--- /dev/null
+++ b/clang/test/Modules/pr140130.cpp
@@ -0,0 +1,33 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: cd %t
+// RUN: %clang_cc1 -iquote . -fmodules -fno-cxx-modules -emit-module \
+// RUN:   -std=c++20 -fmodule-name=c -xc++ c.cppmap -o c.pcm
+// RUN: %clang_cc1 -iquote . -fmodules -fno-cxx-modules -emit-module \
+// RUN:   -std=c++20 -fmodule-name=a -fmodule-map-file=a.cppmap \
+// RUN:   -fmodule-file=c.pcm -xc++ a.cppmap -o a.pcm
+
+//--- a.cppmap
+module "a" {
+ header "a.h"
+}
+//--- a.h
+#include "b.h"
+//--- b.h
+#ifndef _B_H_
+#define _B_H_
+struct B {
+  consteval B() {}
+  union {
+    int a;
+  };
+};
+constexpr B b;
+#endif
+//--- c.cppmap
+module "c" {
+header "c.h"
+}
+//--- c.h
+#include "b.h"

[llvmbot]

@llvm/pr-subscribers-clang

Author: Alexander Kornienko (alexfh)

Changes

This commit fixes #140130

---

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

2 Files Affected:

• (modified) clang/include/clang/AST/PropertiesBase.td (+1-1)
• (added) clang/test/Modules/pr140130.cpp (+33)

diff --git a/clang/include/clang/AST/PropertiesBase.td b/clang/include/clang/AST/PropertiesBase.td
index 33336d57b6298..f3f0039ffe33e 100644
--- a/clang/include/clang/AST/PropertiesBase.td
+++ b/clang/include/clang/AST/PropertiesBase.td
@@ -414,7 +414,7 @@ let Class = PropertyTypeCase<APValue, "Union"> in {
     let Read = [{ node.getUnionValue() }];
   }
   def : Creator<[{
-    return APValue(cast<clang::FieldDecl>(fieldDecl), std::move(value));
+    return APValue(cast_if_present<clang::FieldDecl>(fieldDecl), std::move(value));
   }]>;
 }
 let Class = PropertyTypeCase<APValue, "AddrLabelDiff"> in {
diff --git a/clang/test/Modules/pr140130.cpp b/clang/test/Modules/pr140130.cpp
new file mode 100644
index 0000000000000..da26a005b04f8
--- /dev/null
+++ b/clang/test/Modules/pr140130.cpp
@@ -0,0 +1,33 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: cd %t
+// RUN: %clang_cc1 -iquote . -fmodules -fno-cxx-modules -emit-module \
+// RUN:   -std=c++20 -fmodule-name=c -xc++ c.cppmap -o c.pcm
+// RUN: %clang_cc1 -iquote . -fmodules -fno-cxx-modules -emit-module \
+// RUN:   -std=c++20 -fmodule-name=a -fmodule-map-file=a.cppmap \
+// RUN:   -fmodule-file=c.pcm -xc++ a.cppmap -o a.pcm
+
+//--- a.cppmap
+module "a" {
+ header "a.h"
+}
+//--- a.h
+#include "b.h"
+//--- b.h
+#ifndef _B_H_
+#define _B_H_
+struct B {
+  consteval B() {}
+  union {
+    int a;
+  };
+};
+constexpr B b;
+#endif
+//--- c.cppmap
+module "c" {
+header "c.h"
+}
+//--- c.h
+#include "b.h"

[hokein]

Thanks for the fix.

I think we need a release note in clang/docs/ReleaseNotes.rst

[cor3ntin]

Agreed, this needs a release note

[alexfh]

I think we need a release note in clang/docs/ReleaseNotes.rst

Which aspect would you like to cover in the release notes? The fix of the mismatch between AST writer and AST reader or something else? Do you have a specific wording in mind?

[mizvekov]

LGTM

Something like:

• Fixes serialization of constexpr structs containing unions. (#GH140130)

[alexfh]

• Fixes serialization of constexpr structs containing unions. (#GH140130)

Thanks for the suggestion, added to the release notes.