[clang] f9d7619 - [ASTContext] Profile Dependently-sized array types that do not have a specified number

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2024-05-07T10:59:34+08:00
New Revision: f9d76197ff0099502cf001abe3f5310c5bc4532d

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

LOG: [ASTContext] Profile Dependently-sized array types that do not have a specified number
of elements

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

The root reason for the issue is that we always generate the
dependently-sized array types which don't specify a number of elements.

The original comment says:

> We do no canonicalization here at all, which is okay
> because they can't be used in most locations.

But now we find the locations.

Added: 
    clang/test/Modules/pr91105.cppm

Modified: 
    clang/lib/AST/ASTContext.cpp
    clang/lib/AST/Type.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 5f96e86f803a80..91e7a5f67a93d3 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -3797,33 +3797,33 @@ QualType ASTContext::getDependentSizedArrayType(QualType elementType,
           numElements->isValueDependent()) &&
          "Size must be type- or value-dependent!");
 
+  SplitQualType canonElementType = getCanonicalType(elementType).split();
+
+  void *insertPos = nullptr;
+  llvm::FoldingSetNodeID ID;
+  DependentSizedArrayType::Profile(
+      ID, *this, numElements ? QualType(canonElementType.Ty, 0) : elementType,
+      ASM, elementTypeQuals, numElements);
+
+  // Look for an existing type with these properties.
+  DependentSizedArrayType *canonTy =
+    DependentSizedArrayTypes.FindNodeOrInsertPos(ID, insertPos);
+
   // Dependently-sized array types that do not have a specified number
   // of elements will have their sizes deduced from a dependent
-  // initializer.  We do no canonicalization here at all, which is okay
-  // because they can't be used in most locations.
+  // initializer.
   if (!numElements) {
+    if (canonTy)
+      return QualType(canonTy, 0);
+
     auto *newType = new (*this, alignof(DependentSizedArrayType))
         DependentSizedArrayType(elementType, QualType(), numElements, ASM,
                                 elementTypeQuals, brackets);
+    DependentSizedArrayTypes.InsertNode(newType, insertPos);
     Types.push_back(newType);
     return QualType(newType, 0);
   }
 
-  // Otherwise, we actually build a new type every time, but we
-  // also build a canonical type.
-
-  SplitQualType canonElementType = getCanonicalType(elementType).split();
-
-  void *insertPos = nullptr;
-  llvm::FoldingSetNodeID ID;
-  DependentSizedArrayType::Profile(ID, *this,
-                                   QualType(canonElementType.Ty, 0),
-                                   ASM, elementTypeQuals, numElements);
-
-  // Look for an existing type with these properties.
-  DependentSizedArrayType *canonTy =
-    DependentSizedArrayTypes.FindNodeOrInsertPos(ID, insertPos);
-
   // If we don't have one, build one.
   if (!canonTy) {
     canonTy = new (*this, alignof(DependentSizedArrayType))

diff  --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index 2385c5e02cb269..e31741cd44240d 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -256,7 +256,8 @@ void DependentSizedArrayType::Profile(llvm::FoldingSetNodeID &ID,
   ID.AddPointer(ET.getAsOpaquePtr());
   ID.AddInteger(llvm::to_underlying(SizeMod));
   ID.AddInteger(TypeQuals);
-  E->Profile(ID, Context, true);
+  if (E)
+    E->Profile(ID, Context, true);
 }
 
 DependentVectorType::DependentVectorType(QualType ElementType,

diff  --git a/clang/test/Modules/pr91105.cppm b/clang/test/Modules/pr91105.cppm
new file mode 100644
index 00000000000000..0873962c3773ca
--- /dev/null
+++ b/clang/test/Modules/pr91105.cppm
@@ -0,0 +1,47 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/bar.cppm -emit-module-interface -o %t/bar.pcm
+// RUN: %clang_cc1 -std=c++20 %t/foo.cc -fmodule-file=bar=%t/bar.pcm -fsyntax-only -verify
+//
+// RUN: %clang_cc1 -std=c++20 -fskip-odr-check-in-gmf %t/bar.cppm -emit-module-interface \
+// RUN:     -o %t/bar.pcm
+// RUN: %clang_cc1 -std=c++20 -fskip-odr-check-in-gmf %t/foo.cc \
+// RUN:     -fmodule-file=bar=%t/bar.pcm -fsyntax-only -verify
+//
+// RUN: %clang_cc1 -std=c++20 %t/bar.cppm -emit-reduced-module-interface -o %t/bar.pcm
+// RUN: %clang_cc1 -std=c++20 %t/foo.cc -fmodule-file=bar=%t/bar.pcm -fsyntax-only -verify
+//
+// RUN: %clang_cc1 -std=c++20 -fskip-odr-check-in-gmf %t/bar.cppm -emit-reduced-module-interface \
+// RUN:     -o %t/bar.pcm
+// RUN: %clang_cc1 -std=c++20 -fskip-odr-check-in-gmf %t/foo.cc \
+// RUN:     -fmodule-file=bar=%t/bar.pcm -fsyntax-only -verify
+
+//--- h.hpp
+#pragma once
+
+struct T {
+    constexpr T(const char *) {}
+};
+template <char... c>
+struct t {
+    inline constexpr operator T() const { return {s}; }
+
+private:
+    inline static constexpr char s[]{c..., '\0'};
+};
+
+//--- bar.cppm
+module;
+#include "h.hpp"
+export module bar;
+export inline constexpr auto k = t<'k'>{};
+
+//--- foo.cc
+// expected-no-diagnostics
+#include "h.hpp"
+import bar;
+void f() {
+  T x = k;
+}