[PATCH] D73020: [Sema] Perform call checking when building CXXNewExpr

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

It would be really useful to provide these extra parameters to `FindAllocationFunctions` too. For example, we could support direct dispatch to a size-class-specific allocation function:

  void *operator new(std::size_t n) __attribute__((enable_if(n == 16, "optimized allocator for 16-byte objects")));



================
Comment at: clang/lib/AST/ExprConstant.cpp:14469
+  if (!E->getType()->isIntegralOrUnscopedEnumerationType() &&
+      !E->getType()->isAlignValT()) {
+    if (Loc)
----------------
This is wrong and may result in user-visible non-conformance; an expression of type `std::align_val_t` should never return `true` from `isIntegerConstantExpr`, because `align_val_t` is a scoped enumeration.

Can we instead change the caller (presumably the checking for the `alloc_align` attribute) to call a more general evaluation function rather than requiring an ICE?


================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:2086-2088
+    // conjure them. For size, we can't do anything better than just passing a
+    // nullptr, but for alignment we can either also pass a nullptr, or actually
+    // materialize the alignment we'll pass into the call.
----------------
If this is not an array new, or it's an array new with a constant array bound, we can pass in a correct size. For the remaining (runtime array bound) cases, please conjure up an `OpaqueValueExpr` as the corresponding argument here rather than working around the `nullptr` argument in `EvaluateWithSubstitution`.


================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:2097-2104
+      auto *AlignmentLiteral = IntegerLiteral::Create(
+          Context,
+          llvm::APInt(Context.getTypeSize(SizeTy),
+                      Alignment / Context.getCharWidth()),
+          SizeTy, SourceLocation());
+      auto *DesiredAlignmnet = ImplicitCastExpr::Create(
+          Context, AlignValT, CK_IntegralCast, AlignmentLiteral,
----------------
Please put these on the stack rather than `ASTContext`-allocating them. Both of these classes already support stack allocation.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73020/new/

https://reviews.llvm.org/D73020