Skip to content
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

[Serialization] Fix instantiated default arguments #76473

Closed
wants to merge 1 commit into from

Conversation

hahnjo
Copy link
Member

@hahnjo hahnjo commented Dec 27, 2023

ParmVarDecl::getDefaultArg() strips the outermost FullExpr, such as ExprWithCleanups. This leads to an llvm_unreachable being executed with the added test clang/test/Modules/pr68702.cpp; instead use the more generic VarDecl::getInit() which also returns FullExpr's.

Closes #68702

ParmVarDecl::getDefaultArg() strips the outermost FullExpr, such as
ExprWithCleanups. This leads to an llvm_unreachable being executed
with the added test clang/test/Modules/pr68702.cpp; instead use the
more generic VarDecl::getInit() which also returns FullExpr's.

Closes llvm#68702
@hahnjo hahnjo added c++20 clang:modules C++20 modules and Clang Header Modules labels Dec 27, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Dec 27, 2023

@llvm/pr-subscribers-clang-modules

Author: Jonas Hahnfeld (hahnjo)

Changes

ParmVarDecl::getDefaultArg() strips the outermost FullExpr, such as ExprWithCleanups. This leads to an llvm_unreachable being executed with the added test clang/test/Modules/pr68702.cpp; instead use the more generic VarDecl::getInit() which also returns FullExpr's.

Closes #68702


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

2 Files Affected:

  • (modified) clang/lib/Serialization/ASTWriter.cpp (+4-2)
  • (added) clang/test/Modules/pr68702.cpp (+65)
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 78939bfd533ffa..b9f65dc6d452fc 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -5293,8 +5293,10 @@ void ASTWriter::WriteDeclUpdatesBlocks(RecordDataImpl &OffsetsRecord) {
         break;
 
       case UPD_CXX_INSTANTIATED_DEFAULT_ARGUMENT:
-        Record.AddStmt(const_cast<Expr *>(
-            cast<ParmVarDecl>(Update.getDecl())->getDefaultArg()));
+        // Do not use ParmVarDecl::getDefaultArg(): It strips the outermost
+        // FullExpr, such as ExprWithCleanups.
+        Record.AddStmt(
+            const_cast<Expr *>(cast<ParmVarDecl>(Update.getDecl())->getInit()));
         break;
 
       case UPD_CXX_INSTANTIATED_DEFAULT_MEMBER_INITIALIZER:
diff --git a/clang/test/Modules/pr68702.cpp b/clang/test/Modules/pr68702.cpp
new file mode 100644
index 00000000000000..d32f946910f4fb
--- /dev/null
+++ b/clang/test/Modules/pr68702.cpp
@@ -0,0 +1,65 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -std=c++20 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %t/main.cpp -o %t/main.o
+
+//--- V.h
+#ifndef V_H
+#define V_H
+
+class A {
+public:
+  constexpr A() { }
+  constexpr ~A() { }
+};
+
+template <typename T>
+class V {
+public:
+  V() = default;
+
+  constexpr V(int n, const A& a = A()) {}
+};
+
+#endif
+
+//--- inst1.h
+#include "V.h"
+
+static void inst1() {
+  V<int> v;
+}
+
+//--- inst2.h
+#include "V.h"
+
+static void inst2() {
+  V<int> v(100);
+}
+
+//--- module.modulemap
+module "M" {
+  export *
+  module "V.h" {
+    export *
+    header "V.h"
+  }
+  module "inst1.h" {
+    export *
+    header "inst1.h"
+  }
+}
+
+module "inst2.h" {
+  export *
+  header "inst2.h"
+}
+
+//--- main.cpp
+#include "V.h"
+#include "inst2.h"
+
+static void m() {
+  static V<int> v(100);
+}

@hahnjo hahnjo closed this May 7, 2024
@hahnjo hahnjo deleted the serialize-default-argument branch May 7, 2024 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++20 clang:modules C++20 modules and Clang Header Modules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash with modules and constexpr destructor
2 participants