-
Notifications
You must be signed in to change notification settings - Fork 26
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
cleanup: initialize vars in the EventLoop constructor in the correct order #143
Conversation
…order This fixes: warning: field 'm_context' will be initialized after field 'm_task_set' [-Wreorder-ctor]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code review ACK f52d08c. Looks good, thanks for the fix!
Again will probably merge this Monday in case anyone else wants to review, but can merge sooner if that would be helpful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK f52d08c, tested on Ubuntu 24.04:
$ cmake -B build -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_CXX_FLAGS=-Wreorder-ctor
$ cmake --build build
Bring in a few cmake changes to improve cmake support bitcoin-core/libmultiprocess#140 build: don't clobber user/superproject c++ version bitcoin-core/libmultiprocess#142 build: add option for external mpgen binary bitcoin-core/libmultiprocess#143 cleanup: initialize vars in the EventLoop constructor in the correct order bitcoin-core/libmultiprocess#146 cmake: Suppress compiler warnings from capnproto headers bitcoin-core/libmultiprocess#147 cmake: EXTERNAL_MPGEN cleanups
Bring in a few cmake changes to improve cmake support bitcoin-core/libmultiprocess#140 build: don't clobber user/superproject c++ version bitcoin-core/libmultiprocess#142 build: add option for external mpgen binary bitcoin-core/libmultiprocess#143 cleanup: initialize vars in the EventLoop constructor in the correct order bitcoin-core/libmultiprocess#146 cmake: Suppress compiler warnings from capnproto headers bitcoin-core/libmultiprocess#147 cmake: EXTERNAL_MPGEN cleanups bitcoin-core/libmultiprocess#148 util: fix -Wpessimizing-move warning
Bring in a few cmake changes to improve cmake support bitcoin-core/libmultiprocess#140 build: don't clobber user/superproject c++ version bitcoin-core/libmultiprocess#142 build: add option for external mpgen binary bitcoin-core/libmultiprocess#143 cleanup: initialize vars in the EventLoop constructor in the correct order bitcoin-core/libmultiprocess#146 cmake: Suppress compiler warnings from capnproto headers bitcoin-core/libmultiprocess#147 cmake: EXTERNAL_MPGEN cleanups bitcoin-core/libmultiprocess#148 util: fix -Wpessimizing-move warning bitcoin-core/libmultiprocess#145 CTest: Module must be included at the top level bitcoin-core/libmultiprocess#149 Avoid `-Wundef` compiler warnings bitcoin-core/libmultiprocess#152 refactor: Fix compiler and clang-tidy warnings
Bring in a few cmake changes to improve cmake support and fix compile/lint/tidy warnings. bitcoin-core/libmultiprocess#140 build: don't clobber user/superproject c++ version bitcoin-core/libmultiprocess#142 build: add option for external mpgen binary bitcoin-core/libmultiprocess#143 cleanup: initialize vars in the EventLoop constructor in the correct order bitcoin-core/libmultiprocess#146 cmake: Suppress compiler warnings from capnproto headers bitcoin-core/libmultiprocess#147 cmake: EXTERNAL_MPGEN cleanups bitcoin-core/libmultiprocess#148 util: fix -Wpessimizing-move warning bitcoin-core/libmultiprocess#145 CTest: Module must be included at the top level bitcoin-core/libmultiprocess#149 Avoid `-Wundef` compiler warnings bitcoin-core/libmultiprocess#152 refactor: Fix compiler and clang-tidy warnings bitcoin-core/libmultiprocess#155 scripted-diff: s/Libmultiprocess_EXTERNAL_MPGEN/MPGEN_EXECUTABLE/g bitcoin-core/libmultiprocess#156 refactor: Remove locale-dependent function calls
Bring in a few cmake changes to improve cmake support and fix compile/lint/tidy warnings. bitcoin-core/libmultiprocess#140 build: don't clobber user/superproject c++ version bitcoin-core/libmultiprocess#142 build: add option for external mpgen binary bitcoin-core/libmultiprocess#143 cleanup: initialize vars in the EventLoop constructor in the correct order bitcoin-core/libmultiprocess#146 cmake: Suppress compiler warnings from capnproto headers bitcoin-core/libmultiprocess#147 cmake: EXTERNAL_MPGEN cleanups bitcoin-core/libmultiprocess#148 util: fix -Wpessimizing-move warning bitcoin-core/libmultiprocess#145 CTest: Module must be included at the top level bitcoin-core/libmultiprocess#149 Avoid `-Wundef` compiler warnings bitcoin-core/libmultiprocess#152 refactor: Fix compiler and clang-tidy warnings bitcoin-core/libmultiprocess#155 scripted-diff: s/Libmultiprocess_EXTERNAL_MPGEN/MPGEN_EXECUTABLE/g bitcoin-core/libmultiprocess#156 refactor: Remove locale-dependent function calls bitcoin-core/libmultiprocess#157 refactor: Avoid using std::format
Bring in a few cmake changes to improve cmake support and fix compile/lint/tidy warnings. bitcoin-core/libmultiprocess#140 build: don't clobber user/superproject c++ version bitcoin-core/libmultiprocess#142 build: add option for external mpgen binary bitcoin-core/libmultiprocess#143 cleanup: initialize vars in the EventLoop constructor in the correct order bitcoin-core/libmultiprocess#146 cmake: Suppress compiler warnings from capnproto headers bitcoin-core/libmultiprocess#147 cmake: EXTERNAL_MPGEN cleanups bitcoin-core/libmultiprocess#148 util: fix -Wpessimizing-move warning bitcoin-core/libmultiprocess#145 CTest: Module must be included at the top level bitcoin-core/libmultiprocess#149 Avoid `-Wundef` compiler warnings bitcoin-core/libmultiprocess#152 refactor: Fix compiler and clang-tidy warnings bitcoin-core/libmultiprocess#155 scripted-diff: s/Libmultiprocess_EXTERNAL_MPGEN/MPGEN_EXECUTABLE/g bitcoin-core/libmultiprocess#156 refactor: Remove locale-dependent function calls bitcoin-core/libmultiprocess#157 refactor: Avoid using std::format bitcoin-core/libmultiprocess#159 bugfix: Do not lock EventLoop::mutex after EventLoop is done
Bring in a few cmake changes to improve cmake support and fix compile/lint/tidy warnings. bitcoin-core/libmultiprocess#140 build: don't clobber user/superproject c++ version bitcoin-core/libmultiprocess#142 build: add option for external mpgen binary bitcoin-core/libmultiprocess#143 cleanup: initialize vars in the EventLoop constructor in the correct order bitcoin-core/libmultiprocess#146 cmake: Suppress compiler warnings from capnproto headers bitcoin-core/libmultiprocess#147 cmake: EXTERNAL_MPGEN cleanups bitcoin-core/libmultiprocess#148 util: fix -Wpessimizing-move warning bitcoin-core/libmultiprocess#145 CTest: Module must be included at the top level bitcoin-core/libmultiprocess#149 Avoid `-Wundef` compiler warnings bitcoin-core/libmultiprocess#152 refactor: Fix compiler and clang-tidy warnings bitcoin-core/libmultiprocess#155 scripted-diff: s/Libmultiprocess_EXTERNAL_MPGEN/MPGEN_EXECUTABLE/g bitcoin-core/libmultiprocess#156 refactor: Remove locale-dependent function calls bitcoin-core/libmultiprocess#157 refactor: Avoid using std::format bitcoin-core/libmultiprocess#159 bugfix: Do not lock EventLoop::mutex after EventLoop is done bitcoin-core/libmultiprocess#161 cmake: Avoid including CTest if not top level project
Bring in a few cmake changes to improve cmake support and fix compile/lint/tidy warnings. bitcoin-core/libmultiprocess#140 build: don't clobber user/superproject c++ version bitcoin-core/libmultiprocess#142 build: add option for external mpgen binary bitcoin-core/libmultiprocess#143 cleanup: initialize vars in the EventLoop constructor in the correct order bitcoin-core/libmultiprocess#146 cmake: Suppress compiler warnings from capnproto headers bitcoin-core/libmultiprocess#147 cmake: EXTERNAL_MPGEN cleanups bitcoin-core/libmultiprocess#148 util: fix -Wpessimizing-move warning bitcoin-core/libmultiprocess#145 CTest: Module must be included at the top level bitcoin-core/libmultiprocess#149 Avoid `-Wundef` compiler warnings bitcoin-core/libmultiprocess#152 refactor: Fix compiler and clang-tidy warnings bitcoin-core/libmultiprocess#155 scripted-diff: s/Libmultiprocess_EXTERNAL_MPGEN/MPGEN_EXECUTABLE/g bitcoin-core/libmultiprocess#156 refactor: Remove locale-dependent function calls bitcoin-core/libmultiprocess#157 refactor: Avoid using std::format bitcoin-core/libmultiprocess#159 bugfix: Do not lock EventLoop::mutex after EventLoop is done bitcoin-core/libmultiprocess#161 cmake: Avoid including CTest if not top level project
Bump libmultiprocess library to include bugfix bitcoin-core/libmultiprocess#159 which should fix intermittent CI failure reported bitcoin#31921 This change is bumping the libmultiprocess version instead of cherry picking the bugfix. It could cherry-pick the bugfix instead, but there are reasons to prefer just bumping the version: - Bugfix might interact with earlier PRs, and the latest version is better tested with testing done in many CI configurations in bitcoin#30975 and bitcoin#31802 - Even though we are in feature freeze for a release, the MULTIPROCESS=1 option is currently not enabled for release, so this PR only affect CI builds and local builds, not the release build. bitcoin-core/libmultiprocess#140 build: don't clobber user/superproject c++ version bitcoin-core/libmultiprocess#142 build: add option for external mpgen binary bitcoin-core/libmultiprocess#143 cleanup: initialize vars in the EventLoop constructor in the correct order bitcoin-core/libmultiprocess#146 cmake: Suppress compiler warnings from capnproto headers bitcoin-core/libmultiprocess#147 cmake: EXTERNAL_MPGEN cleanups bitcoin-core/libmultiprocess#148 util: fix -Wpessimizing-move warning bitcoin-core/libmultiprocess#145 CTest: Module must be included at the top level bitcoin-core/libmultiprocess#149 Avoid `-Wundef` compiler warnings bitcoin-core/libmultiprocess#152 refactor: Fix compiler and clang-tidy warnings bitcoin-core/libmultiprocess#155 scripted-diff: s/Libmultiprocess_EXTERNAL_MPGEN/MPGEN_EXECUTABLE/g bitcoin-core/libmultiprocess#156 refactor: Remove locale-dependent function calls bitcoin-core/libmultiprocess#157 refactor: Avoid using std::format bitcoin-core/libmultiprocess#159 bugfix: Do not lock EventLoop::mutex after EventLoop is done bitcoin-core/libmultiprocess#161 cmake: Avoid including CTest if not top level project bitcoin-core/libmultiprocess#164 Bump minimum required cmake to 3.12
Bump libmultiprocess library to include bugfix bitcoin-core/libmultiprocess#159 which should fix intermittent CI failures reported in bitcoin#31921 This change is bumping the libmultiprocess version instead of cherry picking the bugfix. It could cherry-pick the bugfix instead, but there are reasons to prefer bumping the version: - Bugfix might interact with earlier PRs, and the latest version is better tested with testing done in many CI configurations in bitcoin#30975 and bitcoin#31802 - Even though we are in feature freeze for a release, the MULTIPROCESS=1 option is currently not enabled for release, so this PR only affect CI builds and local builds, not the release build. This update brings in the following changes: bitcoin-core/libmultiprocess#140 build: don't clobber user/superproject c++ version bitcoin-core/libmultiprocess#142 build: add option for external mpgen binary bitcoin-core/libmultiprocess#143 cleanup: initialize vars in the EventLoop constructor in the correct order bitcoin-core/libmultiprocess#146 cmake: Suppress compiler warnings from capnproto headers bitcoin-core/libmultiprocess#147 cmake: EXTERNAL_MPGEN cleanups bitcoin-core/libmultiprocess#148 util: fix -Wpessimizing-move warning bitcoin-core/libmultiprocess#145 CTest: Module must be included at the top level bitcoin-core/libmultiprocess#149 Avoid `-Wundef` compiler warnings bitcoin-core/libmultiprocess#152 refactor: Fix compiler and clang-tidy warnings bitcoin-core/libmultiprocess#155 scripted-diff: s/Libmultiprocess_EXTERNAL_MPGEN/MPGEN_EXECUTABLE/g bitcoin-core/libmultiprocess#156 refactor: Remove locale-dependent function calls bitcoin-core/libmultiprocess#157 refactor: Avoid using std::format bitcoin-core/libmultiprocess#159 bugfix: Do not lock EventLoop::mutex after EventLoop is done bitcoin-core/libmultiprocess#161 cmake: Avoid including CTest if not top level project bitcoin-core/libmultiprocess#164 Bump minimum required cmake to 3.12
Bump libmultiprocess library to include bugfix bitcoin-core/libmultiprocess#159 which should fix intermittent CI failures reported in bitcoin#31921 This change is bumping the libmultiprocess version instead of cherry picking the bugfix. It could cherry-pick the bugfix instead, but there are reasons to prefer bumping the version: - Bugfix might interact with earlier PRs, and the latest version is better tested with testing done in many CI configurations in bitcoin#30975 and bitcoin#31802 - Even though we are in feature freeze for a release, the MULTIPROCESS=1 option is currently not enabled for release, so this PR only affect CI builds and local builds, not the release build. This update brings in the following changes: bitcoin-core/libmultiprocess#140 build: don't clobber user/superproject c++ version bitcoin-core/libmultiprocess#142 build: add option for external mpgen binary bitcoin-core/libmultiprocess#143 cleanup: initialize vars in the EventLoop constructor in the correct order bitcoin-core/libmultiprocess#146 cmake: Suppress compiler warnings from capnproto headers bitcoin-core/libmultiprocess#147 cmake: EXTERNAL_MPGEN cleanups bitcoin-core/libmultiprocess#148 util: fix -Wpessimizing-move warning bitcoin-core/libmultiprocess#145 CTest: Module must be included at the top level bitcoin-core/libmultiprocess#149 Avoid `-Wundef` compiler warnings bitcoin-core/libmultiprocess#152 refactor: Fix compiler and clang-tidy warnings bitcoin-core/libmultiprocess#155 scripted-diff: s/Libmultiprocess_EXTERNAL_MPGEN/MPGEN_EXECUTABLE/g bitcoin-core/libmultiprocess#156 refactor: Remove locale-dependent function calls bitcoin-core/libmultiprocess#157 refactor: Avoid using std::format bitcoin-core/libmultiprocess#159 bugfix: Do not lock EventLoop::mutex after EventLoop is done bitcoin-core/libmultiprocess#161 cmake: Avoid including CTest if not top level project bitcoin-core/libmultiprocess#164 Bump minimum required cmake to 3.12
Bump libmultiprocess library to include bugfix bitcoin-core/libmultiprocess#159 which should fix intermittent CI failures reported in #31921 This change is bumping the libmultiprocess version instead of cherry picking the bugfix. It could cherry-pick the bugfix instead, but there are reasons to prefer bumping the version: - Bugfix might interact with earlier PRs, and the latest version is better tested with testing done in many CI configurations in #30975 and #31802 - Even though we are in feature freeze for a release, the MULTIPROCESS=1 option is currently not enabled for release, so this PR only affect CI builds and local builds, not the release build. This update brings in the following changes: bitcoin-core/libmultiprocess#140 build: don't clobber user/superproject c++ version bitcoin-core/libmultiprocess#142 build: add option for external mpgen binary bitcoin-core/libmultiprocess#143 cleanup: initialize vars in the EventLoop constructor in the correct order bitcoin-core/libmultiprocess#146 cmake: Suppress compiler warnings from capnproto headers bitcoin-core/libmultiprocess#147 cmake: EXTERNAL_MPGEN cleanups bitcoin-core/libmultiprocess#148 util: fix -Wpessimizing-move warning bitcoin-core/libmultiprocess#145 CTest: Module must be included at the top level bitcoin-core/libmultiprocess#149 Avoid `-Wundef` compiler warnings bitcoin-core/libmultiprocess#152 refactor: Fix compiler and clang-tidy warnings bitcoin-core/libmultiprocess#155 scripted-diff: s/Libmultiprocess_EXTERNAL_MPGEN/MPGEN_EXECUTABLE/g bitcoin-core/libmultiprocess#156 refactor: Remove locale-dependent function calls bitcoin-core/libmultiprocess#157 refactor: Avoid using std::format bitcoin-core/libmultiprocess#159 bugfix: Do not lock EventLoop::mutex after EventLoop is done bitcoin-core/libmultiprocess#161 cmake: Avoid including CTest if not top level project bitcoin-core/libmultiprocess#164 Bump minimum required cmake to 3.12
01f7715 depends: Update libmultiprocess library to fix CI failure (Ryan Ofsky) Pull request description: Bump libmultiprocess library to include bugfix bitcoin-core/libmultiprocess#159 which should fix intermittent CI failures reported in #31921 This change is bumping the libmultiprocess version instead of cherry picking the bugfix. It could cherry-pick the bugfix instead, but there are reasons to prefer bumping the version: - Bugfix might interact with earlier PRs, and the latest version is better tested with testing done in many CI configurations in [#30975](#30975) and [#31802](#31802) - Even though we are in feature freeze for a release, the MULTIPROCESS=1 option is currently not enabled for release, so this PR only affect CI builds and local builds, not the release build. This update brings in the following changes: bitcoin-core/libmultiprocess#140 build: don't clobber user/superproject c++ version bitcoin-core/libmultiprocess#142 build: add option for external mpgen binary bitcoin-core/libmultiprocess#143 cleanup: initialize vars in the EventLoop constructor in the correct order bitcoin-core/libmultiprocess#146 cmake: Suppress compiler warnings from capnproto headers bitcoin-core/libmultiprocess#147 cmake: EXTERNAL_MPGEN cleanups bitcoin-core/libmultiprocess#148 util: fix -Wpessimizing-move warning bitcoin-core/libmultiprocess#145 CTest: Module must be included at the top level bitcoin-core/libmultiprocess#149 Avoid `-Wundef` compiler warnings bitcoin-core/libmultiprocess#152 refactor: Fix compiler and clang-tidy warnings bitcoin-core/libmultiprocess#155 scripted-diff: s/Libmultiprocess_EXTERNAL_MPGEN/MPGEN_EXECUTABLE/g bitcoin-core/libmultiprocess#156 refactor: Remove locale-dependent function calls bitcoin-core/libmultiprocess#157 refactor: Avoid using std::format bitcoin-core/libmultiprocess#159 bugfix: Do not lock EventLoop::mutex after EventLoop is done bitcoin-core/libmultiprocess#161 cmake: Avoid including CTest if not top level project bitcoin-core/libmultiprocess#164 Bump minimum required cmake to 3.12 --- This PR is part of the [process separation project](#28722). ACKs for top commit: fanquake: ACK 01f7715 Tree-SHA512: a6a795e4d4e13e9d35c9346f3c83d5da817f1452bdc4a9412aeadc4c652ad6e5047f4c77756570594a70ec9095cc786772a0469e306dc19bb5a508fd515c37f4
This legitimate warning pops up when built with clang 20 and Core's flags:
warning: field 'm_context' will be initialized after field 'm_task_set' [-Wreorder-ctor]
It looks like there are no dependency issues here and the reordering is safe, but please let me know if I'm wrong.