[clang][bytecode] Fix dummy handling for p2280r4

[tbaederr]

This makes some other problems show up like the fact that we didn't suppress diagnostics during __builtin_constant_p evaluation.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

This makes some other problems show up like the fact that we didn't suppress diagnostics during __builtin_constant_p evaluation.

---

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

8 Files Affected:

• (modified) clang/lib/AST/ByteCode/Compiler.cpp (+1-1)
• (modified) clang/lib/AST/ByteCode/Descriptor.cpp (+4-2)
• (modified) clang/lib/AST/ByteCode/Interp.cpp (+9-11)
• (modified) clang/lib/AST/ByteCode/InterpBuiltin.cpp (+3)
• (modified) clang/lib/AST/ByteCode/Pointer.cpp (+22-11)
• (modified) clang/lib/AST/ByteCode/Pointer.h (+1-2)
• (modified) clang/test/AST/ByteCode/cxx2a.cpp (+1-3)
• (modified) clang/test/SemaCXX/constant-expression-p2280r4.cpp (+12-4)

diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index 66ab27bdd13da5..4659d0e00784d9 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -6175,7 +6175,7 @@ bool Compiler<Emitter>::visitDeclRef(const ValueDecl *D, const Expr *E) {
     }
 
     if (D->getType()->isReferenceType())
-      return false; // FIXME: Do we need to emit InvalidDeclRef?
+      return this->emitDummyPtr(D, E);
   }
 
   // In case we need to re-visit a declaration.
diff --git a/clang/lib/AST/ByteCode/Descriptor.cpp b/clang/lib/AST/ByteCode/Descriptor.cpp
index 1c16c2022dd028..319d1690c1cd07 100644
--- a/clang/lib/AST/ByteCode/Descriptor.cpp
+++ b/clang/lib/AST/ByteCode/Descriptor.cpp
@@ -409,7 +409,8 @@ QualType Descriptor::getElemQualType() const {
   assert(isArray());
   QualType T = getType();
   if (T->isPointerOrReferenceType())
-    return T->getPointeeType();
+    T = T->getPointeeType();
+
   if (const auto *AT = T->getAsArrayTypeUnsafe()) {
     // For primitive arrays, we don't save a QualType at all,
     // just a PrimType. Try to figure out the QualType here.
@@ -424,7 +425,8 @@ QualType Descriptor::getElemQualType() const {
     return CT->getElementType();
   if (const auto *CT = T->getAs<VectorType>())
     return CT->getElementType();
-  llvm_unreachable("Array that's not an array/complex/vector type?");
+
+  return T;
 }
 
 SourceLocation Descriptor::getLocation() const {
diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp
index 40fe7147a18a36..f91820e16fac52 100644
--- a/clang/lib/AST/ByteCode/Interp.cpp
+++ b/clang/lib/AST/ByteCode/Interp.cpp
@@ -68,6 +68,9 @@ static bool diagnoseUnknownDecl(InterpState &S, CodePtr OpPC,
   const SourceInfo &E = S.Current->getSource(OpPC);
 
   if (isa<ParmVarDecl>(D)) {
+    if (D->getType()->isReferenceType())
+      return false;
+
     if (S.getLangOpts().CPlusPlus11) {
       S.FFDiag(E, diag::note_constexpr_function_param_value_unknown) << D;
       S.Note(D->getLocation(), diag::note_declared_at) << D->getSourceRange();
@@ -1287,6 +1290,12 @@ bool Call(InterpState &S, CodePtr OpPC, const Function *Func,
 
     const Pointer &ThisPtr = S.Stk.peek<Pointer>(ThisOffset);
 
+    // C++23 [expr.const]p5.6
+    // an invocation of a virtual function ([class.virtual]) for an object whose
+    // dynamic type is constexpr-unknown;
+    if (ThisPtr.isDummy() && Func->isVirtual())
+      return false;
+
     // If the current function is a lambda static invoker and
     // the function we're about to call is a lambda call operator,
     // skip the CheckInvoke, since the ThisPtr is a null pointer
@@ -1661,17 +1670,6 @@ bool GetTypeidPtr(InterpState &S, CodePtr OpPC, const Type *TypeInfoType) {
   if (!P.isBlockPointer())
     return false;
 
-  if (P.isDummy()) {
-    QualType StarThisType =
-        S.getASTContext().getLValueReferenceType(P.getType());
-    S.FFDiag(S.Current->getSource(OpPC),
-             diag::note_constexpr_polymorphic_unknown_dynamic_type)
-        << AK_TypeId
-        << P.toAPValue(S.getASTContext())
-               .getAsString(S.getASTContext(), StarThisType);
-    return false;
-  }
-
   S.Stk.push<Pointer>(P.getType().getTypePtr(), TypeInfoType);
   return true;
 }
diff --git a/clang/lib/AST/ByteCode/InterpBuiltin.cpp b/clang/lib/AST/ByteCode/InterpBuiltin.cpp
index e657dbd2f9c733..0e586725b5869e 100644
--- a/clang/lib/AST/ByteCode/InterpBuiltin.cpp
+++ b/clang/lib/AST/ByteCode/InterpBuiltin.cpp
@@ -1538,9 +1538,12 @@ static bool interp__builtin_constant_p(InterpState &S, CodePtr OpPC,
   if (ArgType->isIntegralOrEnumerationType() || ArgType->isFloatingType() ||
       ArgType->isAnyComplexType() || ArgType->isPointerType() ||
       ArgType->isNullPtrType()) {
+    auto PrevDiags = S.getEvalStatus().Diag;
+    S.getEvalStatus().Diag = nullptr;
     InterpStack Stk;
     Compiler<EvalEmitter> C(S.Ctx, S.P, S, Stk);
     auto Res = C.interpretExpr(Arg, /*ConvertResultToRValue=*/Arg->isGLValue());
+    S.getEvalStatus().Diag = PrevDiags;
     if (Res.isInvalid()) {
       C.cleanup();
       Stk.clear();
diff --git a/clang/lib/AST/ByteCode/Pointer.cpp b/clang/lib/AST/ByteCode/Pointer.cpp
index ec4756fe4f87dc..3033bd47adf754 100644
--- a/clang/lib/AST/ByteCode/Pointer.cpp
+++ b/clang/lib/AST/ByteCode/Pointer.cpp
@@ -209,6 +209,10 @@ APValue Pointer::toAPValue(const ASTContext &ASTCtx) const {
     return ASTCtx.toCharUnitsFromBits(Layout.getFieldOffset(FieldIndex));
   };
 
+  bool UsePath = true;
+  if (getType()->isLValueReferenceType())
+    UsePath = false;
+
   // Build the path into the object.
   Pointer Ptr = *this;
   while (Ptr.isField() || Ptr.isArrayElement()) {
@@ -217,38 +221,42 @@ APValue Pointer::toAPValue(const ASTContext &ASTCtx) const {
       // An array root may still be an array element itself.
       if (Ptr.isArrayElement()) {
         Ptr = Ptr.expand();
+        const Descriptor *Desc = Ptr.getFieldDesc();
         unsigned Index = Ptr.getIndex();
-        Path.push_back(APValue::LValuePathEntry::ArrayIndex(Index));
-        QualType ElemType = Ptr.getFieldDesc()->getElemQualType();
+        QualType ElemType = Desc->getElemQualType();
         Offset += (Index * ASTCtx.getTypeSizeInChars(ElemType));
+        if (Ptr.getArray().getType()->isArrayType())
+          Path.push_back(APValue::LValuePathEntry::ArrayIndex(Index));
         Ptr = Ptr.getArray();
       } else {
-        Path.push_back(APValue::LValuePathEntry(
-            {Ptr.getFieldDesc()->asDecl(), /*IsVirtual=*/false}));
+        const Descriptor *Desc = Ptr.getFieldDesc();
+        const auto *Dcl = Desc->asDecl();
+        Path.push_back(APValue::LValuePathEntry({Dcl, /*IsVirtual=*/false}));
 
-        if (const auto *FD =
-                dyn_cast_if_present<FieldDecl>(Ptr.getFieldDesc()->asDecl()))
+        if (const auto *FD = dyn_cast_if_present<FieldDecl>(Dcl))
           Offset += getFieldOffset(FD);
 
         Ptr = Ptr.getBase();
       }
     } else if (Ptr.isArrayElement()) {
       Ptr = Ptr.expand();
+      const Descriptor *Desc = Ptr.getFieldDesc();
       unsigned Index;
       if (Ptr.isOnePastEnd())
         Index = Ptr.getArray().getNumElems();
       else
         Index = Ptr.getIndex();
 
-      QualType ElemType = Ptr.getFieldDesc()->getElemQualType();
+      QualType ElemType = Desc->getElemQualType();
       Offset += (Index * ASTCtx.getTypeSizeInChars(ElemType));
-      Path.push_back(APValue::LValuePathEntry::ArrayIndex(Index));
+      if (Ptr.getArray().getType()->isArrayType())
+        Path.push_back(APValue::LValuePathEntry::ArrayIndex(Index));
       Ptr = Ptr.getArray();
     } else {
+      const Descriptor *Desc = Ptr.getFieldDesc();
       bool IsVirtual = false;
 
       // Create a path entry for the field.
-      const Descriptor *Desc = Ptr.getFieldDesc();
       if (const auto *BaseOrMember = Desc->asDecl()) {
         if (const auto *FD = dyn_cast<FieldDecl>(BaseOrMember)) {
           Ptr = Ptr.getBase();
@@ -281,8 +289,11 @@ APValue Pointer::toAPValue(const ASTContext &ASTCtx) const {
   // Just invert the order of the elements.
   std::reverse(Path.begin(), Path.end());
 
-  return APValue(Base, Offset, Path, /*IsOnePastEnd=*/isOnePastEnd(),
-                 /*IsNullPtr=*/false);
+  if (UsePath)
+    return APValue(Base, Offset, Path,
+                   /*IsOnePastEnd=*/!isElementPastEnd() && isOnePastEnd());
+
+  return APValue(Base, Offset, APValue::NoLValuePath());
 }
 
 void Pointer::print(llvm::raw_ostream &OS) const {
diff --git a/clang/lib/AST/ByteCode/Pointer.h b/clang/lib/AST/ByteCode/Pointer.h
index ef03c12e86c100..971b0d5e14cf8c 100644
--- a/clang/lib/AST/ByteCode/Pointer.h
+++ b/clang/lib/AST/ByteCode/Pointer.h
@@ -630,8 +630,7 @@ class Pointer {
     if (isUnknownSizeArray())
       return false;
 
-    return isElementPastEnd() || isPastEnd() ||
-           (getSize() == getOffset() && !isZeroSizeArray());
+    return isPastEnd() || (getSize() == getOffset() && !isZeroSizeArray());
   }
 
   /// Checks if the pointer points past the end of the object.
diff --git a/clang/test/AST/ByteCode/cxx2a.cpp b/clang/test/AST/ByteCode/cxx2a.cpp
index e478a0ddc4c146..7573808c7840f0 100644
--- a/clang/test/AST/ByteCode/cxx2a.cpp
+++ b/clang/test/AST/ByteCode/cxx2a.cpp
@@ -139,9 +139,7 @@ namespace TypeId {
   static_assert(&B2().ti1 == &typeid(B));
   static_assert(&B2().ti2 == &typeid(B2));
   extern B2 extern_b2;
-  static_assert(&typeid(extern_b2) == &typeid(B2)); // expected-error {{constant expression}} \
-                                                    // expected-note{{typeid applied to object 'extern_b2' whose dynamic type is not constant}}
-
+  static_assert(&typeid(extern_b2) == &typeid(B2));
 
   constexpr B2 b2;
   constexpr const B &b1 = b2;
diff --git a/clang/test/SemaCXX/constant-expression-p2280r4.cpp b/clang/test/SemaCXX/constant-expression-p2280r4.cpp
index 0f85c60629eed9..03064538574577 100644
--- a/clang/test/SemaCXX/constant-expression-p2280r4.cpp
+++ b/clang/test/SemaCXX/constant-expression-p2280r4.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++23 -verify %s
+// RUN: %clang_cc1 -std=c++23 -verify %s -fexperimental-new-constant-interpreter -DNEW_INTERP
 
 using size_t = decltype(sizeof(0));
 
@@ -47,11 +48,18 @@ void splash(Swim& swam) {
 }
 
 extern Swim dc;
-extern Swim& trident; // expected-note {{declared here}}
-
 constexpr auto& sandeno   = typeid(dc);         // ok: can only be typeid(Swim)
-constexpr auto& gallagher = typeid(trident);    // expected-error {{constexpr variable 'gallagher' must be initialized by a constant expression}}
-                                                // expected-note@-1 {{initializer of 'trident' is not a constant expression}}
+
+#ifdef NEW_INTERP
+  /// FIXME: This is a difference between the two interpreters. The AST walker will diagnose a non-constant initializer of trident.
+  ///   However, trident doesn't even have an initializer.
+  extern Swim& trident;
+  constexpr auto& gallagher = typeid(trident);    // expected-error {{constexpr variable 'gallagher' must be initialized by a constant expression}}
+#else
+  extern Swim& trident; // expected-note {{declared here}}
+  constexpr auto& gallagher = typeid(trident);    // expected-error {{constexpr variable 'gallagher' must be initialized by a constant expression}}
+                                                  // expected-note@-1 {{initializer of 'trident' is not a constant expression}}
+#endif
 
 namespace explicitThis {
 struct C {