[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

In D112921#4535716 <https://reviews.llvm.org/D112921#4535716>, @wangpc wrote:

> In D112921#4532378 <https://reviews.llvm.org/D112921#4532378>, @Mordante wrote:
>
>> In D112921#4530916 <https://reviews.llvm.org/D112921#4530916>, @wangpc wrote:
>>
>>> In D112921#4529182 <https://reviews.llvm.org/D112921#4529182>, @Mordante wrote:
>>>
>>>> I noticed some of the CI jobs are still failing with the patch, I didn't look into them.
>>>
>>> I don't think they are related to this patch, so I rebased again. If still failed, I will try to fix them later.
>>
>> I see changes in the libc++ ABI list output
>>
>>   Symbol added: _ZdlPvmSt11align_val_t
>>       {'name': '_ZdlPvmSt11align_val_t', 'type': 'FUNC', 'is_defined': False}
>>   
>>   Symbol added: _ZdlPvm
>>       {'name': '_ZdlPvm', 'type': 'FUNC', 'is_defined': False}
>>   
>>   SYMBOL REMOVED: _ZdlPvSt11align_val_t
>>       {'is_defined': False, 'name': '_ZdlPvSt11align_val_t', 'type': 'FUNC'}
>>   
>>   Summary
>>       Added:   2
>>       Removed: 1
>>       Changed: 0
>>
>> There seems to be small change in the symbol name. What does the `m` in the added symbol mean?
>>
>>   _ZdlPvmSt11align_val_t - added
>>   _ZdlPvSt11align_val_t  - remove
>>
>> There is also a new symbol `_ZdlPvm` added.
>
> `m` means `unsigned long` in mangled name (5.1.5.2 Builtin types <https://itanium-cxx-abi.github.io/cxx-abi/abi.html#mangling-builtin>), which is the `size` of sized deallocation.
>
>   $ c++filt _ZdlPvmSt11align_val_t _ZdlPvm
>   operator delete(void*, unsigned long, std::align_val_t)
>   operator delete(void*, unsigned long)
>
> I am not familiar  with libcxx, can you please help me to fix these tests? I hope we can catch up with the release of LLVM 17.

Looking at the ABI list we currently have

  _ZdlPv -> operator delete(void*)                                   // this one is kept
  _ZdlPvSt11align_val_t -> operator delete(void*, std::align_val_t)  // this one is removed

You add the overloads

  operator delete(void*, unsigned long, std::align_val_t)
  operator delete(void*, unsigned long)

Looking in the current WP http://eel.is/c++draft/replacement.functions#lib:new,operator the removed overload is still listed there.

Do you know why `operator delete(void*)` and `operator delete(void*, unsigned long)` are both available?



================
Comment at: clang/include/clang/Driver/Options.td:2728
+  PosFlag<SetTrue, [], "Enable C++14 sized global deallocation functions">,
+  NegFlag<SetFalse>, BothFlags<[CC1Option]>>;
 defm aligned_allocation : BoolFOption<"aligned-allocation",
----------------
Can you update the test `test/Lexer/cxx-features.cpp` and remove `-fsized-deallocation` in C++14 and newer? This should no longer be required.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112921