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

Add C++14 sized delete operators #10707

Merged
merged 1 commit into from
Jun 9, 2019
Merged

Conversation

kjbracey
Copy link
Contributor

Description

Correct C++14 operation of the alloc wrappers requires us to define
custom sized delete operators.

Their presence won't cause any problem for people compiling as C++03 or
C++11.

Pull request type

[X] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

@kjbracey
Copy link
Contributor Author

No test failures seen - it may be that the libraries were actually automatically redirecting from their sized version to our unsized. But suppresses a warning.

@ciarmcom
Copy link
Member

@kjbracey-arm, thank you for your changes.
@ARMmbed/mbed-os-core @ARMmbed/mbed-os-maintainers please review.

Correct C++14 operation of the alloc wrappers requires us to define
custom sized delete operators.

Their presence won't cause any problem for people compiling as C++03 or
C++11.
Copy link
Member

@pan- pan- left a comment

Choose a reason for hiding this comment

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

Thanks @kjbracey-arm for this addition. It is indeed mandatory to define these forms if we define the global version of the delete operator. One thing that can be improved: mark all delete functions as noexcept.

@kjbracey
Copy link
Contributor Author

I've been assuming noexcept does nothing in our build with exceptions disabled, except be our first C++03 breakage. Do you think it will affect code generation?

@pan-
Copy link
Member

pan- commented May 30, 2019

I don't think it will impact code generation however the default signatures are noexpect; my remark was just for consistency (see https://isocpp.org/files/papers/n3778.html).

@kjbracey
Copy link
Contributor Author

I guess I'm just being a bit coy at immediately flinging in non-C++03-compatible code without good reason.

I believe the situation is that we're free to put non-C++03-compatible code in anything targetting 5.14, but this is targetted as a fix for 5.13, so we want it to remain C++03 compatible so people have the option to build as C++03 if they've got an application C++11 incompatibility, for the 5.13 release only.

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 4, 2019

Any review from @ARMmbed/mbed-os-core ?

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 4, 2019

CI started

@mbed-ci
Copy link

mbed-ci commented Jun 4, 2019

Test run: FAILED

Summary: 6 of 7 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_mbed2-build-ARM
  • jenkins-ci/mbed-os-ci_build-IAR
  • jenkins-ci/mbed-os-ci_mbed2-build-GCC_ARM
  • jenkins-ci/mbed-os-ci_build-GCC_ARM
  • jenkins-ci/mbed-os-ci_build-ARM
  • jenkins-ci/mbed-os-ci_mbed2-build-IAR

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 4, 2019

Test run: FAILED

Jenkins restarted, restarting the job now

@mbed-ci
Copy link

mbed-ci commented Jun 4, 2019

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 2
Build artifacts

@0xc0170 0xc0170 merged commit e7bc177 into ARMmbed:master Jun 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants