[clang] [NFC] Minor fix to tryEmitAbstract type in EmitCXXNewAllocSize (PR #123433)

Erich Keane via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 21 10:01:03 PST 2025


================
@@ -732,8 +732,8 @@ static llvm::Value *EmitCXXNewAllocSize(CodeGenFunction &CGF,
   // Emit the array size expression.
   // We multiply the size of all dimensions for NumElements.
   // e.g for 'int[2][3]', ElemType is 'int' and NumElements is 6.
-  numElements =
-    ConstantEmitter(CGF).tryEmitAbstract(*e->getArraySize(), e->getType());
+  numElements = ConstantEmitter(CGF).tryEmitAbstract(
----------------
erichkeane wrote:

I THINK there are some 32 bit targets with 32 bit pointers and 64 bit size_t, but I'm not positive.

That said, see here:
https://godbolt.org/z/8s1dj4cac

The AST already does the cast to the correct platform-specific size.  More particularly:

```
|   `-CXXNewExpr <line:4:5, col:14> 'int *' array Function 0x1b516b00 'operator new[]' 'void *(unsigned long)'
|     `-ImplicitCastExpr <col:13> 'unsigned long' <IntegralCast>
|       `-ImplicitCastExpr <col:13> 'short' <LValueToRValue>
|         `-DeclRefExpr <col:13> 'short' lvalue ParmVar 0x1b5162c0 'I' 'short'
```

See the cast to `unsigned long`, or `size_t`, which I've now confirmed is correct, as it is defined to be `size_t` in the standard for `operator new[]`.  THUS, the overload resolution will always get this type 'correct'.  If you've found an exception to that, it should probably be fixed in the AST.

I would expect the `e->getType()` to be more correct here, because it is the result of the AST being formed correctly (which, if operator `new[]` is ever extended either by standard or extension to allow user overrides of non-`size_t` array sizes here, `size_t` would be incorrect).

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


More information about the cfe-commits mailing list