Skip to content

[core] Disable R__SIZEDDELETE definition for macOS 26 beta #19010

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 16, 2025

Conversation

devajithvs
Copy link
Contributor

@devajithvs devajithvs commented Jun 11, 2025

MacOS 26.0 beta fails to build without this

This along with #18235 should fix MacOs 26.0 builds

This snippet fails to compile with the latest beta (unless we pass -fsized-deallocation):

#include <new>
int main() {
    void* ptr = nullptr;
    size_t size = 10;
    ::operator delete(ptr, size);
    return 0;
}

This Pull request:

Changes or fixes:

Checklist:

  • tested changes locally
  • updated the docs (if necessary)

This PR fixes #

@devajithvs devajithvs self-assigned this Jun 11, 2025
@devajithvs devajithvs marked this pull request as draft June 11, 2025 11:53
@devajithvs devajithvs changed the title [core] Restore macOS condition for R__SIZEDDELETE definition [core] Disable R__SIZEDDELETE definition for macOS 26 beta Jun 11, 2025
@devajithvs devajithvs requested a review from hahnjo June 11, 2025 13:03
@devajithvs devajithvs marked this pull request as ready for review June 11, 2025 13:03
Copy link
Member

@hahnjo hahnjo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you try just

#ifdef __cpp_sized_deallocation
#   define R__SIZEDDELETE
#endif

As far as I understand llvm/llvm-project@ef804d8, libc++ will now also honor the feature flag so my change from commit b5ad658 can be simplified even more.

Copy link

github-actions bot commented Jun 11, 2025

Test Results

    18 files      18 suites   3d 8h 44m 18s ⏱️
 2 811 tests  2 811 ✅ 0 💤 0 ❌
49 162 runs  49 162 ✅ 0 💤 0 ❌

Results for commit 7d50deb.

♻️ This comment has been updated with latest results.

@devajithvs
Copy link
Contributor Author

Can you try just

#ifdef __cpp_sized_deallocation
#   define R__SIZEDDELETE
#endif

As far as I understand llvm/llvm-project@ef804d8, libc++ will now also honor feature flag so my change from commit b5ad658 can be simplified even more.

Thanks Jonas!

@hahnjo
Copy link
Member

hahnjo commented Jun 12, 2025

Looks like we need a new reference file for macOS... Though it's curious that the functions are still universally there on Linux, I thought that -fsized-deallocation is not the default

libc++ now honors the feature flag `__cpp_sized_deallocation`

Ref: llvm/llvm-project@ef804d8
Copy link
Member

@hahnjo hahnjo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. If needed, we can tweak choosing the reference file in the future, for example if some configuration on Linux doesn't enable __cpp_sized_deallocation either

@devajithvs devajithvs merged commit 5f62a1b into root-project:master Jun 16, 2025
23 of 24 checks passed
@hahnjo
Copy link
Member

hahnjo commented Jun 16, 2025

I'm seeing a local failure of roottest-root-meta-runMemberComments with Clang 18. I'm proposing to globally enable sized deallocation in #19053.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants