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 -fsized-deallocation compile option #9218

Closed

Conversation

Atry
Copy link
Contributor

@Atry Atry commented Sep 21, 2022

Fb-thrift uses C++17's operator delete, which requires -fsized-deallocation when building c++ source code with clang and libstdc++.

operator delete (
cs, nbytes, std::align_val_t{alignof(apache::thrift::ContextStack)});

Test Plan:
Rebase #9129 onto this PR, then there will be no error message about operator delete.

@facebook-github-bot
Copy link
Contributor

@Atry has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

1 similar comment
@facebook-github-bot
Copy link
Contributor

@Atry has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@Atry Atry marked this pull request as ready for review September 21, 2022 20:56
Atry added a commit to Atry/fbthrift that referenced this pull request Sep 22, 2022
Summary:
Fb-thrift uses C++17's `operator delete`, which requires `-fsized-deallocation` when building c++ source code with clang and libstdc++.

https://github.com/facebook/hhvm/blob/9832791642981d582d0f29c6d89dba2c879cc43d/third-party/thrift/src/thrift/lib/cpp/ContextStack.cpp#L229-L230

X-link: facebook/hhvm#9218

Differential Revision: D39707592

Pulled By: Atry

fbshipit-source-id: d2074612e9d0cc62b8b74d9e93079641521e852c
@facebook-github-bot
Copy link
Contributor

@Atry has updated the pull request. You must reimport the pull request before landing.

@facebook-github-bot
Copy link
Contributor

@Atry has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Atry added a commit to Atry/fbthrift that referenced this pull request Sep 22, 2022
Summary:
Fb-thrift uses C++17's `operator delete`, which requires `-fsized-deallocation` when building c++ source code with clang and libstdc++.

https://github.com/facebook/hhvm/blob/9832791642981d582d0f29c6d89dba2c879cc43d/third-party/thrift/src/thrift/lib/cpp/ContextStack.cpp#L229-L230

X-link: facebook/hhvm#9218

Differential Revision: D39707592

Pulled By: Atry

fbshipit-source-id: 8c1a4251e48b8802aab1606f50a972ca945b7da2
@facebook-github-bot
Copy link
Contributor

@Atry has updated the pull request. You must reimport the pull request before landing.

@facebook-github-bot
Copy link
Contributor

@Atry has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Atry added a commit to Atry/fbthrift that referenced this pull request Sep 23, 2022
Summary:
Fb-thrift uses C++17's `operator delete`, which requires `-fsized-deallocation` when building c++ source code with clang and libstdc++.

https://github.com/facebook/hhvm/blob/9832791642981d582d0f29c6d89dba2c879cc43d/third-party/thrift/src/thrift/lib/cpp/ContextStack.cpp#L229-L230

X-link: facebook/hhvm#9218

Reviewed By: vitaut

Differential Revision: D39707592

Pulled By: Atry

fbshipit-source-id: 46e82a2d16ce095ac60b73396a8baa1d54bc57c9
Summary:
X-link: facebook/fbthrift#528

Fb-thrift uses C++17's `operator delete`, which requires `-fsized-deallocation` when building c++ source code with clang and libstdc++.

https://github.com/facebook/hhvm/blob/9832791642981d582d0f29c6d89dba2c879cc43d/third-party/thrift/src/thrift/lib/cpp/ContextStack.cpp#L229-L230

Pull Request resolved: facebook#9218

Test Plan: Rebase facebook#9129 onto this PR, then there will be no error message about `operator delete`.

Reviewed By: vitaut

Differential Revision: D39707592

Pulled By: Atry

fbshipit-source-id: dd1c0ad0ab438a1340b63b16b683e71fd3eba5c7
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D39707592

@Atry Atry force-pushed the add-fsized-deallocation-compile-option branch from 527e88f to 4b90325 Compare September 24, 2022 01:17
@facebook-github-bot
Copy link
Contributor

@Atry has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot pushed a commit to facebook/fbthrift that referenced this pull request Sep 24, 2022
Summary:
Pull Request resolved: #528

Fb-thrift uses C++17's `operator delete`, which requires `-fsized-deallocation` when building c++ source code with clang and libstdc++.

https://github.com/facebook/hhvm/blob/9832791642981d582d0f29c6d89dba2c879cc43d/third-party/thrift/src/thrift/lib/cpp/ContextStack.cpp#L229-L230

X-link: facebook/hhvm#9218

Test Plan: Rebase facebook/hhvm#9129 onto this PR, then there will be no error message about `operator delete`.

Reviewed By: vitaut

Differential Revision: D39707592

Pulled By: Atry

fbshipit-source-id: aeddf5d6b2c8bb8b9d71578fe38f57a1b47f8dc9
@Atry Atry deleted the add-fsized-deallocation-compile-option branch September 24, 2022 02:09
facebook-github-bot pushed a commit that referenced this pull request Sep 27, 2022
Summary:
This PR is an aggregation of the following PRs that fix issues in the HHVM OSS's clang build.

- #9226
- #9223
- #9220
- #9218
- #9217
- #9215
- #9214
- #9213
- #9211

It merges all the above PRs and removes `continue-on-error` for clang build.

Pull Request resolved: #9129

Test Plan: GitHub Action for the clang build should pass.

Reviewed By: alexeyt

Differential Revision: D38631930

Pulled By: Atry

fbshipit-source-id: 4fbd47ee355e760800444b7112af241555a9a8a5
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.

2 participants