[clang] 303f20a - [C++20] [Modules] [Serialization] Deserialize

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2022-12-07T10:52:02+08:00
New Revision: 303f20a2cab4554a10ec225fd853709146f8c51c

URL: https://github.com/llvm/llvm-project/commit/303f20a2cab4554a10ec225fd853709146f8c51c
DIFF: https://github.com/llvm/llvm-project/commit/303f20a2cab4554a10ec225fd853709146f8c51c.diff

LOG: [C++20] [Modules] [Serialization] Deserialize
LValuePathSerializationHelper's type properly

Close https://github.com/llvm/llvm-project/issues/58716.

Tested with libcxx's modules build.

When we read the type of
LValuePathSerializationHelper, we didn't read the correct type. We read
the element type as its name suggests. But the problem here is that it
looks like that both the usage and serialization use its type as the
top level type. So here is the mismatch.

Actually, the type of LValuePathSerializationHelper is never used after
Deserialization without the assertion. So it doesn't matter for the
release users. And this patch shouldn't change the behavior too.

Reviewed By: erichkeane

Differential Revision: https://reviews.llvm.org/D139406

Added: 
    clang/test/Modules/pr58716.cppm

Modified: 
    clang/include/clang/AST/APValue.h
    clang/include/clang/AST/AbstractBasicReader.h
    clang/lib/AST/APValue.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/AST/APValue.h b/clang/include/clang/AST/APValue.h
index 5f4ac02f53c9a..4e22d6c8443c5 100644
--- a/clang/include/clang/AST/APValue.h
+++ b/clang/include/clang/AST/APValue.h
@@ -238,7 +238,7 @@ class APValue {
     }
   };
   class LValuePathSerializationHelper {
-    const void *ElemTy;
+    const void *Ty;
 
   public:
     ArrayRef<LValuePathEntry> Path;

diff  --git a/clang/include/clang/AST/AbstractBasicReader.h b/clang/include/clang/AST/AbstractBasicReader.h
index 3394c68f54ebe..bb589fb337b3e 100644
--- a/clang/include/clang/AST/AbstractBasicReader.h
+++ b/clang/include/clang/AST/AbstractBasicReader.h
@@ -190,7 +190,8 @@ class DataStreamBasicReader : public BasicReaderBase<Impl> {
 
   APValue::LValuePathSerializationHelper readLValuePathSerializationHelper(
       SmallVectorImpl<APValue::LValuePathEntry> &path) {
-    auto elemTy = asImpl().readQualType();
+    auto origTy = asImpl().readQualType();
+    auto elemTy = origTy;
     unsigned pathLength = asImpl().readUInt32();
     for (unsigned i = 0; i < pathLength; ++i) {
       if (elemTy->template getAs<RecordType>()) {
@@ -208,7 +209,7 @@ class DataStreamBasicReader : public BasicReaderBase<Impl> {
             APValue::LValuePathEntry::ArrayIndex(asImpl().readUInt32()));
       }
     }
-    return APValue::LValuePathSerializationHelper(path, elemTy);
+    return APValue::LValuePathSerializationHelper(path, origTy);
   }
 
   Qualifiers readQualifiers() {

diff  --git a/clang/lib/AST/APValue.cpp b/clang/lib/AST/APValue.cpp
index 634423008c2ed..d583718c0eeb0 100644
--- a/clang/lib/AST/APValue.cpp
+++ b/clang/lib/AST/APValue.cpp
@@ -156,10 +156,10 @@ void APValue::LValuePathEntry::Profile(llvm::FoldingSetNodeID &ID) const {
 
 APValue::LValuePathSerializationHelper::LValuePathSerializationHelper(
     ArrayRef<LValuePathEntry> Path, QualType ElemTy)
-    : ElemTy((const void *)ElemTy.getTypePtrOrNull()), Path(Path) {}
+    : Ty((const void *)ElemTy.getTypePtrOrNull()), Path(Path) {}
 
 QualType APValue::LValuePathSerializationHelper::getType() {
-  return QualType::getFromOpaquePtr(ElemTy);
+  return QualType::getFromOpaquePtr(Ty);
 }
 
 namespace {

diff  --git a/clang/test/Modules/pr58716.cppm b/clang/test/Modules/pr58716.cppm
new file mode 100644
index 0000000000000..3f97fca7d5e8a
--- /dev/null
+++ b/clang/test/Modules/pr58716.cppm
@@ -0,0 +1,46 @@
+// Tests that the compiler won't crash due to the consteval constructor.
+//
+// REQUIRES: x86-registered-target
+//
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -std=c++20 -emit-module-interface %t/m.cppm -o %t/m.pcm
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -std=c++20 %t/m.pcm -S -emit-llvm -o - | FileCheck %t/m.cppm
+//
+//--- m.cppm
+module;
+#include "fail.h"
+export module mymodule;
+
+// CHECK: @.str = {{.*}}"{}\00"
+// CHECK: store{{.*}}ptr @.str
+
+//--- fail.h
+namespace std { 
+
+template<class _CharT>
+class basic_string_view {
+public:
+    constexpr basic_string_view(const _CharT* __s)
+        : __data_(__s) {}
+
+private:
+    const   _CharT* __data_;
+};
+
+template <class _CharT>
+struct basic_format_string {
+  template <class _Tp>
+  consteval basic_format_string(const _Tp& __str) : __str_{__str} {
+  }
+
+private:
+  basic_string_view<_CharT> __str_;
+};
+}
+
+auto this_fails() -> void {
+    std::basic_format_string<char> __fmt("{}");
+}