Skip to content

Conversation

HerrCai0907
Copy link
Contributor

fix: #151716

In #65918, support of incomplete array type is added in TryReferenceListInitialization. It causes the crash in Constant Expr Calculation since it only considers the case where it is ConstantArrayType.

This patch wants to add support for incomplete array type also.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Aug 23, 2025
@llvmbot
Copy link
Member

llvmbot commented Aug 23, 2025

@llvm/pr-subscribers-clang

Author: Congcong Cai (HerrCai0907)

Changes

fix: #151716

In #65918, support of incomplete array type is added in TryReferenceListInitialization. It causes the crash in Constant Expr Calculation since it only considers the case where it is ConstantArrayType.

This patch wants to add support for incomplete array type also.


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

2 Files Affected:

  • (modified) clang/lib/AST/ExprConstant.cpp (+8-4)
  • (modified) clang/test/CodeGenCXX/cxx20-p0388-unbound-ary.cpp (+15)
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 9b934753bcc3c..301fc64f3f84f 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -4030,9 +4030,13 @@ findSubobject(EvalInfo &Info, const Expr *E, const CompleteObject &Obj,
     if (ObjType->isArrayType()) {
       // Next subobject is an array element.
       const ConstantArrayType *CAT = Info.Ctx.getAsConstantArrayType(ObjType);
-      assert(CAT && "vla in literal type?");
+      const IncompleteArrayType *IAT =
+          Info.Ctx.getAsIncompleteArrayType(ObjType);
+      const ArrayType *AT = CAT ? static_cast<const ArrayType *>(CAT)
+                                : static_cast<const ArrayType *>(IAT);
+      assert(AT && "vla in literal type?");
       uint64_t Index = Sub.Entries[I].getAsArrayIndex();
-      if (CAT->getSize().ule(Index)) {
+      if (CAT && CAT->getSize().ule(Index)) {
         // Note, it should not be possible to form a pointer with a valid
         // designator which points more than one past the end of the array.
         if (Info.getLangOpts().CPlusPlus11)
@@ -4043,12 +4047,12 @@ findSubobject(EvalInfo &Info, const Expr *E, const CompleteObject &Obj,
         return handler.failed();
       }
 
-      ObjType = CAT->getElementType();
+      ObjType = AT->getElementType();
 
       if (O->getArrayInitializedElts() > Index)
         O = &O->getArrayInitializedElt(Index);
       else if (!isRead(handler.AccessKind)) {
-        if (!CheckArraySize(Info, CAT, E->getExprLoc()))
+        if (CAT && !CheckArraySize(Info, CAT, E->getExprLoc()))
           return handler.failed();
 
         expandArray(*O, Index);
diff --git a/clang/test/CodeGenCXX/cxx20-p0388-unbound-ary.cpp b/clang/test/CodeGenCXX/cxx20-p0388-unbound-ary.cpp
index a29f4d720c1de..007b47c441b2f 100644
--- a/clang/test/CodeGenCXX/cxx20-p0388-unbound-ary.cpp
+++ b/clang/test/CodeGenCXX/cxx20-p0388-unbound-ary.cpp
@@ -32,4 +32,19 @@ void foo(int a) {
   f({a});
 }
 
+constexpr int gh151716() {
+  int(&&g)[]{0,1,2};
+  return g[2];
+}
+// CHECK-LABEL: @_ZN3One10gh151716_fEv
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   %v = alloca i32, align 4
+// CHECK-NEXT:   call void @llvm.lifetime.start.p0(ptr nonnull %v)
+// CHECK-NEXT:   store volatile i32 2, ptr %v, align 4
+// CHECK-NEXT:   call void @llvm.lifetime.end.p0(ptr nonnull %v)
+// CHECK-NEXT:   ret void
+void gh151716_f() {
+  volatile const int v = gh151716();
+}
+
 } // namespace One

…lation

In llvm#65918, support of incomplete array type is added in TryReferenceListInitialization.
It causes the crash in Constant Expr Calculation since it only considers the case where it is ConstantArrayType.

This patch wants to add support for incomplete array type also.
Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs a changelog entry. LGTM otherwise. Thanks!

@HerrCai0907 HerrCai0907 enabled auto-merge (squash) September 4, 2025 13:59
@HerrCai0907 HerrCai0907 merged commit a3186be into llvm:main Sep 4, 2025
10 checks passed
@HerrCai0907 HerrCai0907 deleted the fix/151716 branch September 4, 2025 15:14
Comment on lines +4037 to +4038
if (const auto *CAT = dyn_cast<ConstantArrayType>(AT);
CAT && CAT->getSize().ule(Index)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks wrong and dangerous. What happens if the index is out of bounds but the type is an IncompleteArrayType?

I think this is fundamentally the wrong fix. The type of an object should never be incomplete; we should be forming a complete array type with a bound at some earlier point, and should not see IncompleteArrayTypes here.

Here's a related example:

constexpr int gh151716() {
  int(&&g)[]{0,1,2};
  int *p = &g[1];
  return *p;
}
constexpr int n = gh151716()

This produces a bogus diagnostic:

<source>:6:15: error: constexpr variable 'n' must be initialized by a constant expression
    6 | constexpr int n = gh151716();
      |               ^   ~~~~~~~~~~
<source>:4:10: note: read of element of array without known bound is not allowed in a constant expression
    4 |   return *p;
      |          ^
<source>:6:19: note: in call to 'gh151716()'
    6 | constexpr int n = gh151716();
      |                   ^~~~~~~~~~

... again because we incorrectly have an incomplete array type here.

Normally, initialization of an object declared with an incomplete array type adjusts the AST so that the type in the AST is the complete array type, with a bound. Presumably that's what we should be doing here too.

Copy link
Contributor Author

@HerrCai0907 HerrCai0907 Sep 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

according to spec, here should be unknown bound array.
But you are right, here we should forbidden to do constant calculation for incomplete array type. But also we cannot simply convert it to complete array type.
I think gcc does correct here https://godbolt.org/z/41fK7hjxW.
Maybe we can return false for incomplete array type?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[clang] Assertion `CAT && "vla in literal type?"' failed.
5 participants