[clang] [clang][bytecode] Don't call checkLiteralType() in visitInitializer() (PR #109530)

[Timm Baeder via cfe-commits]

https://github.com/tbaederr created https://github.com/llvm/llvm-project/pull/109530

We were calling checkLiteralType() too many time and rejecting some things we shouldn't. Add The calls manually when handling MaterializeTemporaryExprs. Maybe we should call it in other places as well, but adding more calls is easier than removing them from a generic code path.

>From e2af94d18fb2496dab65ede49662b70106253a14 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Sat, 21 Sep 2024 15:45:05 +0200
Subject: [PATCH] [clang][bytecode] Don't call checkLiteralType() in
 visitInitializer()

We were calling checkLiteralType() too many time and rejecting
some things we shouldn't. Add The calls manually when handling
MaterializeTemporaryExprs. Maybe we should call it in other places as
well, but adding more calls is easier than removing them from a generic
code path.
---
 clang/lib/AST/ByteCode/Compiler.cpp |  9 +++++---
 clang/lib/AST/ByteCode/Interp.cpp   | 31 ++++++++++++++++++++++++++
 clang/lib/AST/ByteCode/Interp.h     | 34 +----------------------------
 clang/test/AST/ByteCode/cxx17.cpp   | 12 ++++++++++
 4 files changed, 50 insertions(+), 36 deletions(-)

diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index 7e0775a51aee61..fe35ca9eb27675 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -2459,6 +2459,8 @@ bool Compiler<Emitter>::VisitMaterializeTemporaryExpr(
       return this->emitGetPtrGlobal(*GlobalIndex, E);
     }
 
+    if (!this->checkLiteralType(SubExpr))
+      return false;
     // Non-primitive values.
     if (!this->emitGetPtrGlobal(*GlobalIndex, E))
       return false;
@@ -2479,6 +2481,10 @@ bool Compiler<Emitter>::VisitMaterializeTemporaryExpr(
       return false;
     return this->emitGetPtrLocal(LocalIndex, E);
   } else {
+
+    if (!this->checkLiteralType(SubExpr))
+      return false;
+
     const Expr *Inner = E->getSubExpr()->skipRValueSubobjectAdjustments();
     if (std::optional<unsigned> LocalIndex =
             allocateLocal(Inner, E->getExtendingDecl())) {
@@ -3500,9 +3506,6 @@ template <class Emitter>
 bool Compiler<Emitter>::visitInitializer(const Expr *E) {
   assert(!classify(E->getType()));
 
-  if (!this->checkLiteralType(E))
-    return false;
-
   OptionScope<Emitter> Scope(this, /*NewDiscardResult=*/false,
                              /*NewInitializing=*/true);
   return this->Visit(E);
diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp
index 17cf3ccdeb6a94..0c6806ebe02e6d 100644
--- a/clang/lib/AST/ByteCode/Interp.cpp
+++ b/clang/lib/AST/ByteCode/Interp.cpp
@@ -1008,6 +1008,37 @@ void diagnoseEnumValue(InterpState &S, CodePtr OpPC, const EnumDecl *ED,
   }
 }
 
+bool CheckLiteralType(InterpState &S, CodePtr OpPC, const Type *T) {
+  assert(T);
+  assert(!S.getLangOpts().CPlusPlus23);
+
+  // C++1y: A constant initializer for an object o [...] may also invoke
+  // constexpr constructors for o and its subobjects even if those objects
+  // are of non-literal class types.
+  //
+  // C++11 missed this detail for aggregates, so classes like this:
+  //   struct foo_t { union { int i; volatile int j; } u; };
+  // are not (obviously) initializable like so:
+  //   __attribute__((__require_constant_initialization__))
+  //   static const foo_t x = {{0}};
+  // because "i" is a subobject with non-literal initialization (due to the
+  // volatile member of the union). See:
+  //   http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#1677
+  // Therefore, we use the C++1y behavior.
+
+  if (S.Current->getFunction() && S.Current->getFunction()->isConstructor() &&
+      S.Current->getThis().getDeclDesc()->asDecl() == S.EvaluatingDecl) {
+    return true;
+  }
+
+  const Expr *E = S.Current->getExpr(OpPC);
+  if (S.getLangOpts().CPlusPlus11)
+    S.FFDiag(E, diag::note_constexpr_nonliteral) << E->getType();
+  else
+    S.FFDiag(E, diag::note_invalid_subexpr_in_const_expr);
+  return false;
+}
+
 bool CallVar(InterpState &S, CodePtr OpPC, const Function *Func,
              uint32_t VarArgSize) {
   if (Func->hasThisPointer()) {
diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index 92bed32d56f4d5..4aceb83eee0e71 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -158,6 +158,7 @@ bool CallBI(InterpState &S, CodePtr OpPC, const Function *Func,
             const CallExpr *CE, uint32_t BuiltinID);
 bool CallPtr(InterpState &S, CodePtr OpPC, uint32_t ArgSize,
              const CallExpr *CE);
+bool CheckLiteralType(InterpState &S, CodePtr OpPC, const Type *T);
 
 enum class ShiftDir { Left, Right };
 
@@ -2946,39 +2947,6 @@ static inline bool IsConstantContext(InterpState &S, CodePtr OpPC) {
   return true;
 }
 
-inline bool CheckLiteralType(InterpState &S, CodePtr OpPC, const Type *T) {
-  assert(T);
-  assert(!S.getLangOpts().CPlusPlus23);
-
-  // C++1y: A constant initializer for an object o [...] may also invoke
-  // constexpr constructors for o and its subobjects even if those objects
-  // are of non-literal class types.
-  //
-  // C++11 missed this detail for aggregates, so classes like this:
-  //   struct foo_t { union { int i; volatile int j; } u; };
-  // are not (obviously) initializable like so:
-  //   __attribute__((__require_constant_initialization__))
-  //   static const foo_t x = {{0}};
-  // because "i" is a subobject with non-literal initialization (due to the
-  // volatile member of the union). See:
-  //   http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#1677
-  // Therefore, we use the C++1y behavior.
-
-  if (S.EvaluatingDecl)
-    return true;
-
-  if (S.Current->getFunction() && S.Current->getFunction()->isConstructor() &&
-      S.Current->getThis().getDeclDesc()->asDecl() == S.EvaluatingDecl)
-    return true;
-
-  const Expr *E = S.Current->getExpr(OpPC);
-  if (S.getLangOpts().CPlusPlus11)
-    S.FFDiag(E, diag::note_constexpr_nonliteral) << E->getType();
-  else
-    S.FFDiag(E, diag::note_invalid_subexpr_in_const_expr);
-  return false;
-}
-
 //===----------------------------------------------------------------------===//
 // Read opcode arguments
 //===----------------------------------------------------------------------===//
diff --git a/clang/test/AST/ByteCode/cxx17.cpp b/clang/test/AST/ByteCode/cxx17.cpp
index 5e38d1a5887007..e8559d8b9812ac 100644
--- a/clang/test/AST/ByteCode/cxx17.cpp
+++ b/clang/test/AST/ByteCode/cxx17.cpp
@@ -81,6 +81,18 @@ constexpr int b() {
 }
 static_assert(b() == 11);
 
+namespace cwg1872 {
+  template<typename T> struct A : T {
+    constexpr int f() const { return 0; }
+  };
+  struct X {};
+  struct Y { virtual int f() const; };
+  struct Z : virtual X {};
+
+  constexpr int z = A<Z>().f(); // both-error {{must be initialized by a constant expression}} \
+                                // both-note {{non-literal type 'A<Z>' cannot be used in a constant expression}}
+}
+
 /// The diagnostics between the two interpreters used to be different here.
 struct S { int a; };
 constexpr S getS() { // both-error {{constexpr function never produces a constant expression}}