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

[ChuanqiXu]

Summary

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. And the original contributor is not
active now. So we can't know his original idea.

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. So this patch
should be good or at least not bad.

clang/include/clang/AST/APValue.h

public:
     friend bool operator!=(LValuePathEntry A, LValuePathEntry B) {
       return A.Value != B.Value;
     }
     friend llvm::hash_code hash_value(LValuePathEntry A) {
       return llvm::hash_value(A.Value);
     }
   };
   class LValuePathSerializationHelper {
-    const void *ElemTy;
+    const void *Ty;
 
   public:
     ArrayRef<LValuePathEntry> Path;
 
     LValuePathSerializationHelper(ArrayRef<LValuePathEntry>, QualType);
     QualType getType();
   };
   struct NoLValuePath {};

clang/include/clang/AST/AbstractBasicReader.h

llvm::FixedPointSemantics readFixedPointSemantics() {
     bool isSaturated = tmp & 0x2;
     bool hasUnsignedPadding = tmp & 0x4;
     return llvm::FixedPointSemantics(width, scale, isSigned, isSaturated,
                                      hasUnsignedPadding);
   }
 
   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>()) {
         unsigned int_ = asImpl().readUInt32();
         Decl *decl = asImpl().template readDeclAs<Decl>();
         if (auto *recordDecl = dyn_cast<CXXRecordDecl>(decl))
           elemTy = getASTContext().getRecordType(recordDecl);
         else
           elemTy = cast<ValueDecl>(decl)->getType();
         path.push_back(
             APValue::LValuePathEntry(APValue::BaseOrMemberType(decl, int_)));
       } else {
         elemTy = getASTContext().getAsArrayType(elemTy)->getElementType();
         path.push_back(
             APValue::LValuePathEntry::ArrayIndex(asImpl().readUInt32()));
       }
     }
-    return APValue::LValuePathSerializationHelper(path, elemTy);
+    return APValue::LValuePathSerializationHelper(path, origTy);
   }
 
   Qualifiers readQualifiers() {
     static_assert(sizeof(Qualifiers().getAsOpaqueValue()) <= sizeof(uint32_t),
                   "update this if the value size changes");
     uint32_t value = asImpl().readUInt32();
     return Qualifiers::fromOpaqueValue(value);
   }

clang/lib/AST/APValue.cpp

 }
 
 void APValue::LValuePathEntry::Profile(llvm::FoldingSetNodeID &ID) const {
   ID.AddInteger(Value);
 }
 
 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 {
   struct LVBase {
     APValue::LValueBase Base;
     CharUnits Offset;
     unsigned PathLength;
     bool IsNullPtr : 1;

clang/test/Modules/pr58716.cppm

+// 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("{}");
+}