- Boost - lists.preview.boost.org

Re: [boost] [scope] Scope review starts on November 26th
by Janko Dedic 02 Dec '23

02 Dec '23

Thanks for proposing this library. Scope guards are a natural and much needed addition to Boost. The following is my review of the proposed Boost.Scope. The first thing I looked for in this library was a convenience SCOPE_EXIT { ... }; macro that would provide the functionality in the form of a control flow construct. I've managed to quickly find it, but it wasn't exactly what I was expecting. BOOST_SCOPE_FINAL [] { std::cout << "Hello world!" << std::endl; }; I found it rather odd that the capture list is not included in the macro. I looked a bit further into the documentation to find this: BOOST_SCOPE_FINAL std::ref(cleanup_func); Is this use case the reason why the capture list is required? It seems like a very rare one that shouldn't pessimize the interface for 99% of use cases. Performance shouldn't be a concern because the compiler sees all this code and inlines it anyway. In my view, this just makes the interface worse and less pleasant to use. scope_success doesn't seem terribly useful. I have yet to see a legitimate use case of this type. I haven't found a motivating example in the documentation. There should either be one, or this type should be removed from the library. There is no BOOST_SCOPE_FAIL macro. Why is it missing? I found the scope_fail example pretty bad: class collection { std::set< std::shared_ptr< object > > objects; public: void add_object(std::shared_ptr< object > const& obj) { // Create a deactivated scope guard initially std::set< std::shared_ptr< object > >::iterator it; boost::scope::scope_fail rollback_guard{[&, this] { objects.erase(it); }, false}; bool inserted; std::tie(it, inserted) = objects.insert(obj); if (inserted) { // Activate rollback guard rollback_guard.set_active(true); } obj->on_added_to_collection(*this); } }; It could be simply rewritten to not use set_active (which should be discouraged, because it makes code less declarative and harder to follow): class collection { std::set< std::shared_ptr< object > > objects; public: void add_object(std::shared_ptr< object > const& obj) { // Create a deactivated scope guard initially std::set< std::shared_ptr< object > >::iterator it; bool inserted; std::tie(it, inserted) = objects.insert(obj); if (inserted) { boost::scope::scope_fail rollback_guard{[&, this] { objects.erase(it); }}; obj->on_added_to_collection(*this); } } }; Could be made even cleaner by using an early return: if (!inserted) return; All destructor declarations in the documentation lack noexcept specification, which by default means that they're noexcept, but they're actually not if you look at the Throws section, or the implementation. Also, the question presents itself: why are these destructors allowed to throw? I feel like it is common knowledge today that destructors should not throw, and this is heavily frowned upon. I can see the want to "be generic" here, but I don't see why a library should entertain broken code. I also noticed that certain scope guards are move constructible. Why? I cannot imagine myself ever approving of someone moving a scope guard on code review. Scope guards are to be used exclusively as local variables and don't need to be moved. This just opens up room for misuse. I don't see any compelling use cases for scope_exit::set_active in the documentation. The only example I find motivating is this one: void push_back(int x, std::vector<int>& vec1, std::vector<int>& vec2) { vec1.push_back(x); // Revert vec1 modification on failure boost::scope::scope_exit rollback_guard{[&] { vec1.pop_back(); }}; vec2.push_back(x); // Commit vec1 modification rollback_guard.set_active(false); } This could as well have used rollback_guard.release();. This matches my personal experience: only dismiss/release/cancel is necessary for real use cases ("commiting" something and thus disabling the "rollback" action). absl::Cleanup for example, supports only Cancel(). There is value in preventing users from shooting themselves in the foot. I don't like the name scope_exit::release personally. I feel like "dismiss" or "cancel" would be much better. I know that std::experimental::scope_exit calls it "release", but we shouldn't appeal to false authority. I did not like the locked_write_string example. Reading this code locally: // Lock the file while (flock(fd, LOCK_EX) < 0) { err = errno; if (err != EINTR) return; } it's hard to get what's happening, why we're setting err and how big of an impact that has on function behavior. A better way to write this code would be to simply extract this piece into its own function: err = errno; if (err != EINTR) return; together with the ec = std::error_code(err, std::generic_category()), which doesn't require a scope guard. fd_deleter and unique_fd provide some nice handling for EINTR, which wasn't required but I think it's nice to have and it's good that it's a part of this library! I think make_unique_resource_checked approaches the problem from the wrong direction. This code: // Create a unique resource for a file auto file = boost::scope::make_unique_resource_checked( open(filename.c_str(), O_CREAT | O_WRONLY) -1, fd_deleter()); if (!file.allocated()) { int err = errno; throw std::system_error(err, std::generic_category(), "Failed to open file " + filename); } could be this: // Create a unique resource for a file int file = open(filename.c_str(), O_CREAT | O_WRONLY); boost::scope::scope_final close_file(fd_deleter{}); if (file != -1) { int err = errno; throw std::system_error(err, std::generic_category(), "Failed to open file " + filename); } This has the advantage of being able to use `file` directly for the rest of the code instead of `file.get()`. I think a similar utility would be more useful in form of a class template, for example: resource<HANDLE, INVALID_HANDLE_VALUE, ::CloseHandle> windows_handle; resource<SDL_Window*, nullptr, ::SDL_DestroyWindow> sdl_window_handle; This would be very useful for concisely implementing RAII wrappers over C handles (you don't have to manually write destructors, move constructors and move assignment operators). For local use (within a function), I feel like a local handle + scope guard is just more ergonomic than the whole make_unique_resource_checked thing. My overall impression is that this library is trying to be too generic when it's not really necessary. There's definitely useful stuff here that I want to see included, but also a lot of stuff that is too much. I don't personally see the need for any scope guard beyond scope_exit with a release. That covers all the use cases I've ran into very nicely. scope_fail can be more error-prone since the "commit" is not explicit, so you may end up doing rollback even when it's unnecessary. Using the modified example from the documentation: void push_back(int x, std::vector<int>& vec1, std::vector<int>& vec2) { vec1.push_back(x); // Revert vec1 modification on failure boost::scope::scope_fail rollback_guard{[&] { vec1.pop_back(); }}; vec2.push_back(x); do_something_else(); // if this throws, rollback_guard runs } If you use scope_exit with an explicit release, it's clearer IMO: void push_back(int x, std::vector<int>& vec1, std::vector<int>& vec2) { vec1.push_back(x); // Revert vec1 modification on failure boost::scope::scope_exit rollback_guard{[&] { vec1.pop_back(); }}; vec2.push_back(x); rollback_guard.release(); // commit do_something_else(); // if this throws, rollback_guard won't run } The documentation states that "exception_checker is incompatible with C++20 coroutines", which is true in general, but it doesn't have to be (depending on the coroutine library implementation). However, I bring this up because using scope_exit+release is an entirely viable alternative for coroutines. scope_exit+release will work whether you use exceptions, error codes, std::expected, coroutines or all of them combined. This is the beauty of it, which makes me feel like scope_fail is unnecessary. I vote to ACCEPT this library with the following CONDITIONS: - BOOST_SCOPE_FINAL includes the lambda capture list OR another macro that does is provided; - noexcept specification on destructors is properly documented; - move constructors on all scope guard types are deleted; - a motivating example for scope_success is provided, or the type is removed. These are my conditions, but I'm hoping the full review is taken as a recommendation in improving the library and the documentation. Best regards, Janko Dedic

3 18

[scope] The comparison with Boost.ScopeExit is unfair
by Andrzej Krzemienski 02 Dec '23

02 Dec '23

Hi All, This is not a review yet. I am just looking at the front page of the docs, and I think the comparison with Boost.ScopeGuard is unfair. You are comparing C++03 Boost.ScopeGuard with C++11 Boost.Scope. Try comparing it with C++03 Boost.Scope. Otherwise compare C++11 to C++11, and there Boost.ScopeGuard has a nicer syntax: int compute() { // Reset variables on return BOOST_SCOPE_EXIT(this_) { this_->x = 0; this_->y = 0; }; return x + y; } Regarding the example with exceptions, nobody would use Boost.ScopeGuard like this. A robust usage is in Boost.ScopeGuard's docs: void world::add_person(person const& a_person) { bool commit = false; persons_.push_back(a_person); // (1) direct action // Following block is executed when the enclosing scope exits. BOOST_SCOPE_EXIT(&commit, &persons_) { if(!commit) persons_.pop_back(); // (2) rollback action }; // ... // (3) other operations commit = true; // (4) disable rollback actions } Which is longer, but usable in coroutines. Whereas Boost.Scope's solution is not usable in coroutines. Only after these changes are applied, I think is the comparison fair. Regards, &rzej;

2 5

Boost Culture
by Vinnie Falco 02 Dec '23

02 Dec '23

It occurs to me that newcomers to the mailing list will not have any understanding of Boost culture or its development process. For example there is a recurring theme where a new person will ask about "what documentation toolchain does Boost use." The answer of course is that every library has considerable autonomy to define how it wants to generate its documents. The only requirement is that the final result is rendered as HTML. Another example is the idea that there is a "CEO of Boost." Or rather that there are particular individuals who have some sort of decision-making authority for the entire libraries. Or another example, why we use mailing lists over dedicated forum software. We might consider putting together a document which centralizes all of the collected wisdom and knowledge about Boost culture including the history of its evolution and why some things are the way that they are. This would facilitate the entry of newcomers to places like the mailing list which assume that participants already know a lot about Boost culture. This could become a chapter in the new Contributor Guide: https://www.preview.boost.org/doc/contributor-guide/index.html Thanks

4 3

Dead Link
by Brian Salehi 01 Dec '23

01 Dec '23

There is a dead link on page [Machine Learning](https://www.preview.boost.org/doc/user-guide/task-machine-learnin… refering to Boost.uBLAS library. I guess the typo is an extra s letter after /doc which seems to point to the correct page when removed. Regards, Brian

1 0

[scope] Scope review starts on November 26th
by Claudio DeSouza 28 Nov '23

28 Nov '23

It would be helpful if the cmake build was fixed to not require some specific custom setup that was used by the author during development. The cmake list file seems to check for the existence of a cmake test file, to decide whether or not to build the tests. These files seem to have been moved outside reach of the setup in place, under test/test_cmake. Also, it would be better to get rid of this: add_subdirectory(../../../.. "${CMAKE_CURRENT_BINARY_DIR}/boost_superproject") Ideally, these cmake tests should assume scope is a submodule, or a subfolder, of the boost super project. Claudio.

4 3

[release] Boost 1.84.0 beta 1 released
by Marshall Clow 24 Nov '23

24 Nov '23

Boost release 1.84.0 beta 1 is now available at: https://boostorg.jfrog.io/artifactory/main/beta/1.84.0.beta1/source/ The SHA256 checksums are: 224d595cc666c19b91214c5e9cced3988d08063146dfad9f616c481d52626f27 boost_1_84_0_b1.zip de9b35e232e4b1769f4d800ec01255a9c2546639be43e45b16ce6d109c088e99 boost_1_84_0_b1.tar.gz f8a9f21580961536a95fb7fcf9deebde45d7e0e94ddd81402f1e37ae2c590410 boost_1_84_0_b1.tar.bz2 3e4765c41974c763b7d4a67a432569bf75d3367d7a6b72c4ac9458e5004c59dc boost_1_84_0_b1.7z Release notes (still need updates): https://www.boost.org/users/history/version_1_84_0.html Please try the beta and report any problems you encounter. Thanks to everyone who contributed to the release. -- Boost Release Team

3 3

cobalt build is failing
by Alain O' Miniussi 24 Nov '23

24 Nov '23

Hi, The colbalt build fails on master, starting with this message: [...]/boost/libs/cobalt/test/main.cpp:10:21: error: too few arguments to function call, expected 2, have 1 boost::cobalt::main co_main(int argc, char *argv[]) ^~~~~~~ I can provide more details if needed, but first I'd like to know if this is expected. Thanks ---- Alain Miniussi DSI, Pôles Calcul et Genie Log. Observatoire de la Côte d'Azur Tél. : +33609650665

2 2

[1.84][beast] Permission to merge to master
by Mohammad Nejati [ashtum] 17 Nov '23

17 Nov '23

Hi, I’d like to merge this to master: https://github.com/boostorg/beast/compare/master...develop Fixes a narrowing conversion error reported in this issue: https://github.com/boostorg/beast/issues/2766 Thanks, Mohammad Nejati

2 1

[release] Boost 1.84.0 Beta 1 Release Candidate 1 is available
by Marshall Clow 17 Nov '23

17 Nov '23

Available at: <https://boostorg.jfrog.io/artifactory/main/beta/1.84.0.beta1/source/> The SHA256 checksums are as follows: f8a9f21580961536a95fb7fcf9deebde45d7e0e94ddd81402f1e37ae2c590410 boost_1_84_0_b1_rc1.tar.bz2 224d595cc666c19b91214c5e9cced3988d08063146dfad9f616c481d52626f27 boost_1_84_0_b1_rc1.zip 3e4765c41974c763b7d4a67a432569bf75d3367d7a6b72c4ac9458e5004c59dc boost_1_84_0_b1_rc1.7z de9b35e232e4b1769f4d800ec01255a9c2546639be43e45b16ce6d109c088e99 boost_1_84_0_b1_rc1.tar.gz As always, the release managers would appreciate it if you download the candidate of your choice and give building it a try. Please report both success and failure, and anything else that is noteworthy. -- The Release Managers

11 28

Do we still want develop branch?
by Andrey Semashev 16 Nov '23

16 Nov '23

Hi, The recent case of a commit in develop that was left unmerged to master made me wondering if people still find the develop branch useful. Historically (since the SVN days), we used a separate branch for active development, which we then merged to the stable branch that was used for creating Boost releases. This allowed to test libraries together before changes propagate to the stable branch. This was useful back when the official test matrix was the primary gauge of the Boost "health", so when something broke, it was rather visible, even if the breakage manifested in downstream libraries. With the adoption of git and CI, the relevance of the test matrix has diminished, and, presumably, people have shifted towards monitoring their libraries' CI. Which means, people are less likely to notice that something broke downstream, unless the downstream developer notices and reports the problem. Although I have been that downstream developer myself quite a few times, I have to admit that this role of the develop branch of "an integration testing field" is not well filled in the current workflow. Noticing the breakage is mostly luck (i.e. a matter of me making a commit to my library and noticing the CI failure caused by an upstream dependency) rather than a rule. Additionally, I have been asked on a few occasions to avoid development directly in the develop branch and use separate feature branches instead. That is, do your commits in a feature branch, let the CI pass, then merge to develop, let the CI pass again, then merge to master. Apparently, some people are using this workflow by default to avoid breakage in develop, which means develop is no longer the branch where the actual development happens. (This workflow actually makes develop meaningless because your CI over the feature branch already tests your changes against develop of the rest of Boost. Which means after merging to develop you're running the same CI again, and might as well just merge to master straight away.) Then there's the recurring problem that led to this post - people forget to merge to master from time to time. Or simply don't have time to merge. Or the library is no longer actively maintained. Or the commit is left out of master for any other reason, which means users don't receive their bug fixes in a release. Personally, I do try to remember the PRs I make during the release cycle and look through them just before the release to make sure they are merged to master. Some of you probably received gentle merge reminders from me. I'm not sure if anyone else is doing this, but I can say this is an active mental effort on my part, which I'd rather not have to make. And even then I can't be sure I remember all PRs or have the time to cycle though them before the release. Which brings me to my question. Do people still find the develop branch useful? Maybe we'd be better off just dropping it and performing all development in feature branches before merging straight to master? Also, if you do find develop useful, what do you think about creating a GitHub Action to automatically merge develop to master once the CI on develop succeeds? Maybe there are other ideas how to avoid commits unmerged to master? Thanks.

8 51