Skip to content
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

[c++] Make operators new and delete weak symbols by default #747

Merged
merged 2 commits into from Oct 14, 2021

Conversation

ASMfreaK
Copy link
Contributor

This change makes operators new and delete weak symbols by default. See #746

@salkinium salkinium added enhancement 🌈 toolchain ⚙️ ci:hal Triggers the exhaustive HAL compile CI jobs labels Oct 14, 2021
@salkinium
Copy link
Member

Hm, so would it also solve the issue if we left the operator delete functions empty by default? Then the new/malloc would still trigger the linker error message, but not this issue right? And then only if the modm:platform:heap module is added we implement the operator delete as it's now implemented?

@salkinium
Copy link
Member

I mean, an application which is never calling new in the first place cannot call delete and expect that to work?

@chris-durand
Copy link
Member

I mean, an application which is never calling new in the first place cannot call delete and expect that to work?

👍 , sounds reasonable.

@salkinium
Copy link
Member

I'll push a commit to implement this, but we're keeping the weak symbols, they could be useful.

@ASMfreaK
Copy link
Contributor Author

ASMfreaK commented Oct 14, 2021

Hm, so would it also solve the issue if we left the operator delete functions empty by default?

I'd prefer to put modm_assert() there maybe not by default, but behind a macro/lbuild option.

@ASMfreaK
Copy link
Contributor Author

And then only if the modm:platform:heap module is added we implement the operator delete as it's now implemented?

Honestly, this was the behavior I was expecting. If modm:platform:heap is added implement operators, otherwise they are left unimplented.

Signed-off-by: delphi <cpp.create@gmail.com>
@salkinium salkinium added this to the 2021q4 milestone Oct 14, 2021
Copy link
Member

@salkinium salkinium left a comment

Choose a reason for hiding this comment

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

Thanks, always nice to fix another corner case!

@salkinium salkinium added ci:hal Triggers the exhaustive HAL compile CI jobs and removed ci:hal Triggers the exhaustive HAL compile CI jobs labels Oct 14, 2021
@salkinium salkinium merged commit 387a625 into modm-io:develop Oct 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:hal Triggers the exhaustive HAL compile CI jobs enhancement 🌈 toolchain ⚙️
Development

Successfully merging this pull request may close these issues.

None yet

3 participants