Fixing problems with the consteval guards

ojhunt · ojhunt · commit 302840dc976f · 2024-12-07T14:43:12.000-08:00
Corentin asked about ODR issues which led to me making more tests,
alas the existing tests all triggered new/delete inside explicitly
consteval contexts (if consteval, template parameters, requires, etc)

In a fully general constexpr function the logic failed, so this
update removes the fast path I was trying to use and just implements
the required semantics in both constant expression evaluators.

It also adds a bunch more tests :D
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
@@ -2521,6 +2521,17 @@ class FunctionDecl : public DeclaratorDecl,
   /// If this function is an allocation/deallocation function that takes
   /// the `std::nothrow_t` tag, return true through IsNothrow,
   bool isReplaceableGlobalAllocationFunction(
+      std::optional<unsigned> *AlignmentParam = nullptr,
+      bool *IsNothrow = nullptr) const {
+    if (isTypeAwareOperatorNewOrDelete())
+      return false;
+    return isConstEvalSafeOrReplaceableGlobalAllocationFunction(AlignmentParam, IsNothrow);
+  }
+
+  /// Determines whether this function is one of the replaceable global
+  /// allocation functions described in isReplaceableGlobalAllocationFunction,
+  /// or is a function that may be treated as such during constant evaluation
+  bool isConstEvalSafeOrReplaceableGlobalAllocationFunction(
       std::optional<unsigned> *AlignmentParam = nullptr,
       bool *IsNothrow = nullptr) const;
 
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
@@ -2146,12 +2146,6 @@ class Sema final : public SemaBase {
            isConstantEvaluatedOverride;
   }
 
-  TypeAwareAllocationMode allocationModeInCurrentContext() const {
-    if (getLangOpts().TypeAwareAllocators && !isConstantEvaluatedContext())
-      return TypeAwareAllocationMode::Yes;
-    return TypeAwareAllocationMode::No;
-  }
-
   SourceLocation getLocationOfStringLiteralByte(const StringLiteral *SL,
                                                 unsigned ByteNo) const;
 
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -3304,7 +3304,7 @@ bool Compiler<Emitter>::VisitCXXNewExpr(const CXXNewExpr *E) {
       // Always invalid.
       return this->emitInvalid(E);
     }
-  } else if (!OperatorNew->isReplaceableGlobalAllocationFunction())
+  } else if (!OperatorNew->isConstEvalSafeOrReplaceableGlobalAllocationFunction())
     return this->emitInvalidNewDeleteExpr(E, E);
 
   const Descriptor *Desc;
@@ -3402,7 +3402,7 @@ bool Compiler<Emitter>::VisitCXXDeleteExpr(const CXXDeleteExpr *E) {
 
   const FunctionDecl *OperatorDelete = E->getOperatorDelete();
 
-  if (!OperatorDelete->isReplaceableGlobalAllocationFunction())
+  if (!OperatorDelete->isConstEvalSafeOrReplaceableGlobalAllocationFunction())
     return this->emitInvalidNewDeleteExpr(E, E);
 
   // Arg must be an lvalue.
@@ -4559,7 +4559,7 @@ bool Compiler<Emitter>::VisitCallExpr(const CallExpr *E) {
 
   const FunctionDecl *FuncDecl = E->getDirectCallee();
   // Calls to replaceable operator new/operator delete.
-  if (FuncDecl && FuncDecl->isReplaceableGlobalAllocationFunction()) {
+  if (FuncDecl && FuncDecl->isConstEvalSafeOrReplaceableGlobalAllocationFunction()) {
     if (FuncDecl->getDeclName().getCXXOverloadedOperator() == OO_New ||
         FuncDecl->getDeclName().getCXXOverloadedOperator() == OO_Array_New) {
       return VisitBuiltinCallExpr(E, Builtin::BI__builtin_operator_new);
diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp
@@ -1063,7 +1063,7 @@ bool Free(InterpState &S, CodePtr OpPC, bool DeleteIsArrayForm,
       if (const FunctionDecl *VirtualDelete =
               getVirtualOperatorDelete(AllocType);
           VirtualDelete &&
-          !VirtualDelete->isReplaceableGlobalAllocationFunction()) {
+          !VirtualDelete->isConstEvalSafeOrReplaceableGlobalAllocationFunction()) {
         S.FFDiag(S.Current->getSource(OpPC),
                  diag::note_constexpr_new_non_replaceable)
             << isa<CXXMethodDecl>(VirtualDelete) << VirtualDelete;
@@ -1498,14 +1498,14 @@ bool InvalidNewDeleteExpr(InterpState &S, CodePtr OpPC, const Expr *E) {
                !OperatorNew->isReservedGlobalPlacementOperator()) {
       S.FFDiag(Loc, diag::note_constexpr_new_placement)
           << /*Unsupported*/ 0 << E->getSourceRange();
-    } else if (!OperatorNew->isReplaceableGlobalAllocationFunction()) {
+    } else if (!OperatorNew->isConstEvalSafeOrReplaceableGlobalAllocationFunction()) {
       S.FFDiag(Loc, diag::note_constexpr_new_non_replaceable)
           << isa<CXXMethodDecl>(OperatorNew) << OperatorNew;
     }
   } else {
     const auto *DeleteExpr = cast<CXXDeleteExpr>(E);
     const FunctionDecl *OperatorDelete = DeleteExpr->getOperatorDelete();
-    if (!OperatorDelete->isReplaceableGlobalAllocationFunction()) {
+    if (!OperatorDelete->isConstEvalSafeOrReplaceableGlobalAllocationFunction()) {
       S.FFDiag(Loc, diag::note_constexpr_new_non_replaceable)
           << isa<CXXMethodDecl>(OperatorDelete) << OperatorDelete;
     }
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
@@ -3373,7 +3373,7 @@ bool FunctionDecl::isReservedGlobalPlacementOperator() const {
   return (proto->getParamType(1).getCanonicalType() == Context.VoidPtrTy);
 }
 
-bool FunctionDecl::isReplaceableGlobalAllocationFunction(
+bool FunctionDecl::isConstEvalSafeOrReplaceableGlobalAllocationFunction(
     std::optional<unsigned> *AlignmentParam, bool *IsNothrow) const {
   if (getDeclName().getNameKind() != DeclarationName::CXXOperatorName)
     return false;
@@ -3390,16 +3390,19 @@ bool FunctionDecl::isReplaceableGlobalAllocationFunction(
   if (!getDeclContext()->getRedeclContext()->isTranslationUnit())
     return false;
 
+  bool IsTypeAware = isTypeAwareOperatorNewOrDelete();
+  unsigned MaxParamCount = IsTypeAware + 4;
   const auto *FPT = getType()->castAs<FunctionProtoType>();
-  if (FPT->getNumParams() == 0 || FPT->getNumParams() > 4 || FPT->isVariadic())
+  if (FPT->getNumParams() == 0 || FPT->getNumParams() > MaxParamCount || FPT->isVariadic())
     return false;
 
+  unsigned MinimumParamCount = IsTypeAware + 1;
   // If this is a single-parameter function, it must be a replaceable global
   // allocation or deallocation function.
-  if (FPT->getNumParams() == 1)
+  if (FPT->getNumParams() == MinimumParamCount)
     return true;
 
-  unsigned Params = 1;
+  unsigned Params = MinimumParamCount;
   QualType Ty = FPT->getParamType(Params);
   const ASTContext &Ctx = getASTContext();
 
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
@@ -8277,7 +8277,7 @@ class ExprEvaluatorBase
           FD = CorrespondingCallOpSpecialization;
         } else
           FD = LambdaCallOp;
-      } else if (FD->isReplaceableGlobalAllocationFunction()) {
+      } else if (FD->isConstEvalSafeOrReplaceableGlobalAllocationFunction()) {
         if (FD->getDeclName().getCXXOverloadedOperator() == OO_New ||
             FD->getDeclName().getCXXOverloadedOperator() == OO_Array_New) {
           LValue Ptr;
@@ -10188,7 +10188,7 @@ bool PointerExprEvaluator::VisitCXXNewExpr(const CXXNewExpr *E) {
     Info.FFDiag(E, diag::note_constexpr_new_placement)
         << /*Unsupported*/ 0 << E->getSourceRange();
     return false;
-  } else if (!OperatorNew->isReplaceableGlobalAllocationFunction()) {
+  } else if (!OperatorNew->isConstEvalSafeOrReplaceableGlobalAllocationFunction()) {
     Info.FFDiag(E, diag::note_constexpr_new_non_replaceable)
         << isa<CXXMethodDecl>(OperatorNew) << OperatorNew;
     return false;
@@ -16256,7 +16256,7 @@ bool VoidExprEvaluator::VisitCXXDeleteExpr(const CXXDeleteExpr *E) {
     return false;
 
   FunctionDecl *OperatorDelete = E->getOperatorDelete();
-  if (!OperatorDelete->isReplaceableGlobalAllocationFunction()) {
+  if (!OperatorDelete->isConstEvalSafeOrReplaceableGlobalAllocationFunction()) {
     Info.FFDiag(E, diag::note_constexpr_new_non_replaceable)
         << isa<CXXMethodDecl>(OperatorDelete) << OperatorDelete;
     return false;
@@ -16300,7 +16300,7 @@ bool VoidExprEvaluator::VisitCXXDeleteExpr(const CXXDeleteExpr *E) {
   if (!E->isArrayForm() && !E->isGlobalDelete()) {
     const FunctionDecl *VirtualDelete = getVirtualOperatorDelete(AllocType);
     if (VirtualDelete &&
-        !VirtualDelete->isReplaceableGlobalAllocationFunction()) {
+        !VirtualDelete->isConstEvalSafeOrReplaceableGlobalAllocationFunction()) {
       Info.FFDiag(E, diag::note_constexpr_new_non_replaceable)
           << isa<CXXMethodDecl>(VirtualDelete) << VirtualDelete;
       return false;
diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp
@@ -1111,7 +1111,7 @@ static bool findDeleteForPromise(Sema &S, SourceLocation Loc, QualType PromiseTy
   // scope of the promise type. If nothing is found, a search is performed in
   // the global scope.
   ImplicitDeallocationParameters IDP = {
-      S.allocationModeInCurrentContext(),
+      typeAwareAllocationModeFromBool(S.getLangOpts().TypeAwareAllocators),
       alignedAllocationModeFromBool(Overaligned), SizedDeallocationMode::Yes};
   if (S.FindDeallocationFunction(Loc, PointeeRD, DeleteName, OperatorDelete,
                                  PromiseType, IDP, /*Diagnose*/ true))
@@ -1422,7 +1422,7 @@ bool CoroutineStmtBuilder::makeNewAndDeleteExpr() {
   // Helper function to indicate whether the last lookup found the aligned
   // allocation function.
   ImplicitAllocationParameters IAP = {
-      S.allocationModeInCurrentContext(),
+      typeAwareAllocationModeFromBool(S.getLangOpts().TypeAwareAllocators),
       alignedAllocationModeFromBool(S.getLangOpts().CoroAlignedAllocation)};
   auto LookupAllocationFunction =
       [&](Sema::AllocationFunctionScope NewScope = Sema::AFS_Both,
@@ -1437,9 +1437,10 @@ bool CoroutineStmtBuilder::makeNewAndDeleteExpr() {
         if (NewScope == Sema::AFS_Both)
           NewScope = PromiseContainsNew ? Sema::AFS_Class : Sema::AFS_Global;
 
-        IAP = {S.allocationModeInCurrentContext(),
-               alignedAllocationModeFromBool(
-                   !ForceNonAligned && S.getLangOpts().CoroAlignedAllocation)};
+        bool ShouldUseAlignedAlloc = !ForceNonAligned && S.getLangOpts().CoroAlignedAllocation;
+        IAP = {
+          typeAwareAllocationModeFromBool(S.getLangOpts().TypeAwareAllocators),
+               alignedAllocationModeFromBool(ShouldUseAlignedAlloc)};
 
         FunctionDecl *UnusedResult = nullptr;
 
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
@@ -14475,10 +14475,11 @@ void Sema::CheckCompleteVariableDeclaration(VarDecl *var) {
         << var;
 
   // Check whether the initializer is sufficiently constant.
-  if ((getLangOpts().CPlusPlus || (getLangOpts().C23 && var->isConstexpr())) &&
-      !type->isDependentType() && Init && !Init->isValueDependent() &&
-      (GlobalStorage || var->isConstexpr() ||
-       var->mightBeUsableInConstantExpressions(Context))) {
+  bool c1 = (getLangOpts().CPlusPlus || (getLangOpts().C23 && var->isConstexpr()));
+  bool c2 = !type->isDependentType() && Init && !Init->isValueDependent();
+  bool c3 = var->isConstexpr();
+  bool c4 = var->mightBeUsableInConstantExpressions(Context);
+  if (c1 && c2 && (GlobalStorage || c3 || c4)) {
     // If this variable might have a constant initializer or might be usable in
     // constant expressions, check whether or not it actually is now.  We can't
     // do this lazily, because the result might depend on things that change
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -9715,7 +9715,8 @@ bool Sema::ShouldDeleteSpecialMember(CXXMethodDecl *MD,
     QualType DeallocType = Context.getRecordType(RD);
     DeclarationName Name =
       Context.DeclarationNames.getCXXOperatorName(OO_Delete);
-    ImplicitDeallocationParameters IDP = {allocationModeInCurrentContext(),
+    ImplicitDeallocationParameters IDP = {
+      typeAwareAllocationModeFromBool(getLangOpts().TypeAwareAllocators),
                                           AlignedAllocationMode::No,
                                           SizedDeallocationMode::No};
     if (FindDeallocationFunction(MD->getLocation(), MD->getParent(), Name,
@@ -16140,7 +16141,7 @@ bool Sema::isTypeAwareOperatorNewOrDelete(const NamedDecl *ND) const {
 FunctionDecl *
 Sema::instantiateTypeAwareUsualDelete(FunctionTemplateDecl *FnTemplateDecl,
                                       QualType DeallocType) {
-  if (!isTypeAwareAllocation(allocationModeInCurrentContext()))
+  if (!getLangOpts().TypeAwareAllocators)
     return nullptr;
 
   TemplateParameterList *TemplateParameters =
@@ -16214,7 +16215,7 @@ Sema::instantiateTypeAwareUsualDelete(FunctionTemplateDecl *FnTemplateDecl,
 }
 
 QualType Sema::instantiateSpecializedTypeIdentity(QualType Subject) {
-  assert(clang::isTypeAwareAllocation(allocationModeInCurrentContext()));
+  assert(getLangOpts().TypeAwareAllocators);
   ClassTemplateDecl *TypeIdentity = getStdTypeIdentity();
   if (!TypeIdentity)
     return QualType();
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
@@ -1754,7 +1754,7 @@ static bool isNonPlacementDeallocationFunction(Sema &S, FunctionDecl *FD) {
     return false;
 
   unsigned UsualParams = 1;
-  if (isTypeAwareAllocation(S.allocationModeInCurrentContext()) &&
+  if (S.getLangOpts().TypeAwareAllocators &&
       UsualParams < FD->getNumParams() && FD->isTypeAwareOperatorNewOrDelete())
     ++UsualParams;
 
@@ -1795,7 +1795,7 @@ namespace {
         FD = InstantiatedDecl;
       }
       unsigned NumBaseParams = 1;
-      if (isTypeAwareAllocation(S.allocationModeInCurrentContext()) &&
+      if (S.getLangOpts().TypeAwareAllocators &&
           FD->isTypeAwareOperatorNewOrDelete()) {
         QualType TypeIdentityTag = FD->getParamDecl(0)->getType();
         QualType ExpectedTypeIdentityTag =
@@ -2425,7 +2425,7 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal,
       AllocType->isDependentType() ? 0 : Context.getTypeAlign(AllocType);
   unsigned NewAlignment = Context.getTargetInfo().getNewAlign();
   ImplicitAllocationParameters IAP = {
-      allocationModeInCurrentContext(),
+      typeAwareAllocationModeFromBool(getLangOpts().TypeAwareAllocators),
       alignedAllocationModeFromBool(getLangOpts().AlignedAllocation &&
                                     Alignment > NewAlignment)};
 
@@ -3514,7 +3514,7 @@ FunctionDecl *Sema::FindUsualDeallocationFunction(
 
   LookupResult FoundDelete(*this, Name, StartLoc, LookupOrdinaryName);
   DeallocLookupMode LookupMode =
-      isTypeAwareAllocation(allocationModeInCurrentContext())
+      getLangOpts().TypeAwareAllocators
           ? DeallocLookupMode::OptionallyTyped
           : DeallocLookupMode::Untyped;
   LookupGlobalDeallocationFunctions(*this, StartLoc, FoundDelete, LookupMode,
@@ -3544,7 +3544,8 @@ FunctionDecl *Sema::FindDeallocationFunctionForDestructor(SourceLocation Loc,
 
   FunctionDecl *OperatorDelete = nullptr;
   QualType DeallocType = Context.getRecordType(RD);
-  ImplicitDeallocationParameters IDP = {allocationModeInCurrentContext(),
+  ImplicitDeallocationParameters IDP = {
+    typeAwareAllocationModeFromBool(getLangOpts().TypeAwareAllocators),
                                         AlignedAllocationMode::No,
                                         SizedDeallocationMode::No};
 
@@ -4017,7 +4018,8 @@ Sema::ActOnCXXDelete(SourceLocation StartLoc, bool UseGlobal,
                                       ArrayForm ? OO_Array_Delete : OO_Delete);
 
     if (PointeeRD) {
-      ImplicitDeallocationParameters IDP = {allocationModeInCurrentContext(),
+      ImplicitDeallocationParameters IDP = {
+        typeAwareAllocationModeFromBool(getLangOpts().TypeAwareAllocators),
                                             AlignedAllocationMode::No,
                                             SizedDeallocationMode::No};
       if (!UseGlobal &&
@@ -4071,7 +4073,7 @@ Sema::ActOnCXXDelete(SourceLocation StartLoc, bool UseGlobal,
 
       // Look for a global declaration.
       ImplicitDeallocationParameters IDP = {
-          allocationModeInCurrentContext(),
+          typeAwareAllocationModeFromBool(getLangOpts().TypeAwareAllocators),
           alignedAllocationModeFromBool(Overaligned),
           sizedDeallocationModeFromBool(CanProvideSize)};
       OperatorDelete =
diff --git a/clang/test/CodeGenCXX/type-aware-new-constexpr.cpp b/clang/test/CodeGenCXX/type-aware-new-constexpr.cpp
@@ -0,0 +1,53 @@
+// RUN: %clang_cc1 %s -triple arm64-apple-macosx -faligned-allocation -fexperimental-cxx-type-aware-allocators -emit-llvm -fcxx-exceptions -fexceptions -std=c++23 -o - | FileCheck %s
+// RUN: %clang_cc1 %s -triple arm64-apple-macosx -faligned-allocation -fexperimental-cxx-type-aware-allocators -emit-llvm -fcxx-exceptions -fexceptions -std=c++23 -fexperimental-new-constant-interpreter -o - | FileCheck %s
+
+using size_t = __SIZE_TYPE__;
+
+namespace std {
+  template <class T> struct type_identity {
+    typedef T type;
+  };
+  enum class align_val_t : size_t {};
+}
+
+
+template <typename T> void *operator new(std::type_identity<T>, size_t);
+template <typename T> void operator delete(std::type_identity<T>, void *);
+struct S {
+  int i = 0;
+  constexpr S() __attribute__((noinline)) {}
+};
+
+ constexpr int doSomething() {
+  S* s = new S;
+  int result = s->i;
+  delete s;
+  return result;
+}
+
+static constexpr int force_doSomething = doSomething();
+template <int N> struct Tag {};
+
+void test1(Tag<force_doSomething>){
+// CHECK-LABEL: define void @_Z5test13TagILi0EE
+}
+
+void test2(Tag<doSomething() + 1>){
+// CHECK-LABEL: define void @_Z5test23TagILi1EE
+}
+
+int main() {
+  // CHECK-LABEL: define noundef i32 @main()
+  return doSomething();
+  // CHECK: call{{.*}}i32 @_Z11doSomethingv()
+}
+
+// CHECK-LABEL: define linkonce_odr noundef i32 @_Z11doSomethingv()
+// CHECK: [[ALLOC:%.*]] = call noundef ptr @_ZnwI1SEPvSt13type_identityIT_Em
+// CHECK: invoke noundef ptr @_ZN1SC1Ev(ptr{{.*}} [[ALLOC]])
+// CHECK-NEXT: to label %[[CONT:.*]] unwind label %[[LPAD:[a-z0-9]+]]
+// CHECK: [[CONT]]:
+// CHECK: call void @_ZdlI1SEvSt13type_identityIT_EPv
+// CHECK: ret
+// CHECK: [[LPAD]]:
+// call void @_ZdlI1SEvSt13type_identityIT_EPv({{.*}} [[ALLOC]])
diff --git a/clang/test/SemaCXX/type-aware-new-constexpr.cpp b/clang/test/SemaCXX/type-aware-new-constexpr.cpp
