[clang] [clang] Forbid reinterpret_cast of function pointers in constexpr. (PR #150557)

[Timm Baeder via cfe-commits]

================
@@ -3,8 +3,8 @@
 // RUN: %clang_cc1 -x c -fsyntax-only %s -verify=c -std=c11 -fexperimental-new-constant-interpreter
 // RUN: %clang_cc1 -x c -fsyntax-only %s -pedantic -verify=c-pedantic -std=c11 -fexperimental-new-constant-interpreter
 //
-// RUN: %clang_cc1 -x c++ -fsyntax-only %s -verify=cxx
-// RUN: %clang_cc1 -x c++ -fsyntax-only %s -pedantic -verify=cxx-pedantic
+// RUN: %clang_cc1 -x c++ -fsyntax-only %s -verify=cxx-nointerpreter
+// RUN: %clang_cc1 -x c++ -fsyntax-only %s -pedantic -verify=cxx-pedantic,cxx-nointerpreter
----------------
tbaederr wrote:

Thanks!
I quickly looked into this and seems like there were some small problems in classify() and us not handling `CK_LValueBitCast` which made this more complex than it had to be. I came up with this patch that tries to get by without the additional opcode:
```diff
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index e760055a8d23..b692e4cb4388 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -472,6 +472,7 @@ bool Compiler<Emitter>::VisitCastExpr(const CastExpr *CE) {
   case CK_CPointerToObjCPointerCast:
     return this->delegate(SubExpr);
 
+  case CK_LValueBitCast:
   case CK_BitCast: {
     // Reject bitcasts to atomic types.
     if (CE->getType()->isAtomicType()) {
@@ -492,13 +493,12 @@ bool Compiler<Emitter>::VisitCastExpr(const CastExpr *CE) {
     assert(isPtrType(*FromT));
     assert(isPtrType(*ToT));
     if (FromT == ToT) {
-      if (CE->getType()->isVoidPointerType())
+      if (CE->getType()->isVoidPointerType() &&
+          !SubExpr->getType()->isFunctionPointerType())
         return this->delegate(SubExpr);
 
       if (!this->visit(SubExpr))
         return false;
-      if (CE->getType()->isFunctionPointerType())
-        return true;
       if (FromT == PT_Ptr)
         return this->emitPtrPtrCast(SubExprTy->isVoidPointerType(), CE);
       return true;
diff --git a/clang/lib/AST/ByteCode/Context.cpp b/clang/lib/AST/ByteCode/Context.cpp
index aaeb52e0fa44..63baa7038ade 100644
--- a/clang/lib/AST/ByteCode/Context.cpp
+++ b/clang/lib/AST/ByteCode/Context.cpp
@@ -351,7 +351,7 @@ OptPrimType Context::classify(QualType T) const {
       return PT_Float;
   }
 
-  if (T->isPointerOrReferenceType())
+  if (T->isPointerOrReferenceType() || T->isFunctionProtoType())
     return PT_Ptr;
 
   if (T->isMemberPointerType())
@@ -376,6 +376,9 @@ OptPrimType Context::classify(QualType T) const {
   if (const auto *DT = dyn_cast<DecltypeType>(T))
     return classify(DT->getUnderlyingType());
 
+  if (const auto *PT = dyn_cast<ParenType>(T))
+    return classify(PT->getInnerType());
+
   if (T->isObjCObjectPointerType() || T->isBlockPointerType())
     return PT_Ptr;
 ```
this seems to have the same effect as your changes (including the additional diagnostic in `functions.cpp` unfortunately). I don't have time to run the entire test suite against this patch today. If it breaks something for you, feel free to use your changes instead.

https://github.com/llvm/llvm-project/pull/150557