[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