Skip to content

Conversation

@al45tair
Copy link
Contributor

Sized deallocation isn't enabled everywhere, so we need to check for it before using it.

Sized deallocation isn't enabled everywhere, so we need to check for it
before using it.
@al45tair al45tair requested a review from compnerd July 29, 2025 16:29
@al45tair al45tair requested a review from rjmccall as a code owner July 29, 2025 16:29
@al45tair
Copy link
Contributor Author

@swift-ci Please smoke test

@al45tair
Copy link
Contributor Author

This should fix build errors I'm seeing, e.g.

/.../swift-project/swift/lib/IRGen/ProtocolInfo.h:237:5: error: no matching function for call to 'operator delete'
  237 |     ::operator delete(ptr, size);
      |     ^~~~~~~~~~~~~~~~~

after #83321 landed.

Copy link
Member

@compnerd compnerd left a comment

Choose a reason for hiding this comment

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

Hmm, I wonder if we should be avoiding the use of the sized allocation similarly.

edit
C++11 treated the two parameter as a placement deletion. C++14 changed that to the sized deletion by default. Clang is the odd compiler that requires -fsized-deallocation flag, MSVC and GCC should enable that by default on C++14 and newer.

@al45tair
Copy link
Contributor Author

Hmm, I wonder if we should be avoiding the use of the sized allocation similarly.

I'm not sure what you mean — operator new always takes a size, doesn't it?

@al45tair
Copy link
Contributor Author

(The changes in this PR fixed my local build.)

@compnerd
Copy link
Member

The issue here was identified by ASAN - we are mismatching the new/delete operation (doing partial frees). I think that this is safe to apply, but I'm wondering if we should be doing something else for the memory handling. Perhaps we should look into bump allocators for the type information rather than using operator new.

@al45tair
Copy link
Contributor Author

Closed in favour of #83399

@al45tair al45tair closed this Jul 31, 2025
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.

3 participants