- Boost - lists.boost.org

[scope] review
by Julien Blanc 07 Dec '23

07 Dec '23

Hi, This is my review of the proposed boost scope library. * Does this library bring real benefit to C++ developers for real world use-case? Yes. Scope guards are a well-known and widely used feature, nearly every code-base has some implementation of it. The same goes for raii resource wrappers. * Do you have an application for this library? I would consider using it for new projects. It does not, however, offer significant value at the time which would justify migrating from our homebrew scope guard. Worse, it misses some guarantees that makes it less fitted (but arguably it is more generic than what we use now). * Does the API match with current best practices? The API is copied from a TS proposal, for some parts. Most of my griefs against the API also applies to the TS. While I was not fond of it at the start, i think there's a clear value in differentiating scope_success and scope_fail. The first one is allowed to throw, the second is not. This, however, is not a strong requirement, but a general advice. It thus raises the concern about why not simply giving a boolean parameter to the scope_guard handler function, and letting it act accordingly. I suggest making the noexcept-ness of scope_fail executor a strong requirement. scope_success / scope_fail are not default-constructible, although they have a valid empty state. While there's a rationale behind this (it is a bit uncommon to create a scope guard that does nothing), it is a limitation for the sake of nothing. Initializing an empty scope_exit allows scope escaping (useful when initializing the scope guard in the body of an if statement). There will always be the possiblity to do it with an optional, so it just seems that forbidding default construction has no value. To check whether a scope_xxx is active or not, one has to call the active() method. To check whether a unique_resource has a value, one has to call the allocated() method. These are additions compared to the TS. While they may be useful, the names are inconsistent, both are new, also inconsistent with what other nullable types (optional, unique_ptr, variant...) uses. While this is common practice accross the c++ standard, I would not called this *best*. This leads to problems for generic code, and proposals such as this one: https://herbsutter.com/2022/09/25/something-i-implemented-today-is-void/ . In both case, i think at least an operator bool() should be provided, because it's a more widespread and common way to check for emptiness / valid state. scope_success / fails / exit takes an additional parameter (extension over TS), which is the execution condition. scope_success executes when the condition returns false. This is unintuitive, and something i will usually get wrong. While i get the reasoning behind, i think it tells more in favor of a scope_conditional<SuccessHandler, FailureHandler> interface. I'm not sure there's a lot of use-cases for a success_only or fail_only scope guard, especially since you can release in case of success, to avoid the cost of the failure check. About this customizable failure detector, i'm not found of the idea. The default, detection of a current exception, is a sane one. And it allows us to require FailureHandler to be noexcept. That's no longer the case if you allow customizing the failure detector. I'm not sure it's worth the tradeoff, since the failure detection could always be done inside the body of a standard unconditional scope_exit. Finally, i don't get the point of set_active. Sure, it's cheap to implement, but do we really want to encourage dynamically enabling scope guards ? Disabling a scope guard makes a lot of sense, that's what you do to disable a rollback scope_guard upon successful operation. Dynamically enabling it looks more like being hostile to the reader to me. In all cases, it's possible to get this behaviour with a boolean inside the scope guard callback, so this use case is already addressed. There's no need for it to be a first-class citizen. I like the idea of the lighter, unconditional, scope_final, which is missing in the TS. * Is the scope of the library well-defined? unique_resource and scope_exit are quite different beasts. They only have in common being RAII. But that also include unique_lock, unique_ptr, and nobody thought about making them part of the same library. I think it is a mistake to mix them. unique_resource takes three template parameters, the third one being a predicate telling if the resource is valid. I understand the wish to avoid storing an extra information, when there's a possibility to do without by storing an invalid value. But to me, it looks like a more widespread problem already solved, for example by https://github.com/akrzemi1/markable . It would be far better IMHO to get markable in boost, and have unique_resource specialized for markable<X> than replicating most of its functionality into unique_resource. * Is the documentation helpful and clear? The documentation is clear enough, although i could not find some information that i think should be emphasized more (ie, what the library guarantees in terms of pre / post conditions, noexceptness, etc). I don't like the way the reference section subitems are links to the file headers descriptions, and not links to class descriptions. I'd prefer something like: Reference * template< > class scope_exit * template< > class scope_final * ... instead of the headers, for which i usually don't care (i just need to know which header i need to include, and this information is provided in the class description). There are a few examples in the documentation that can be improved. There's always room for improvement, the overall quality is good. * Did you try to use it? What problems or surprises did you encounter? How much time did you spent? I made some small experiments with it. I spent around 6-8 hours reviewing the library and writing this review. Most of the time was spent reading the documentation and checking the behaviour. No problems and no surprises so far. * What is your evaluation of the implementation? The library is big (~2,5k lines of code), given what it offers. Most of this comes from the fact that the library offers a lot of customization points, maybe too much. Others have pointed that it reimplements some standard features, for the sake of not including some headers. I don't like this choice, but I don't know what's boost policy in this regard is. In all cases, a reimplementation with a name is better than nothing. In particular, there are some static_cast<T&&> which would advantagely be replaced by a detail::forward<T>, the latter being far more readable than the former. It's a shame that compact_storage is not provided as a facility: i guess there are several similar implementation across boost. But that's not the library's fault. Outside that, the organisation is clean, and the code largely readable, given that it needs to be compatible with C++11. * What is you final evaluation of the library? My current recommandation is to CONDITIONNALY ACCEPT the library. I think the following should be adressed, but i'd note that several of these comments also applies to the TS: - scope_success and scope_fail should be unified into a single call, taking two callbacks. This will make a slightly bigger object, but if you plan on detecting exceptions then you don't mind the extra cost anyway. And it will allow to evaluate the condition only once, so that may prove a gain in several situations. - scope_success and scope_fail should not take an execution condition. Exception detection should be enough for everyone, or else you should resort to scope_exit. - the failure handler (scope_fail) should be enforced to be noexcept - scope_exit should be default constructible (released by default, equivalent to moved-from state). That does not hold for scope_final, which is not releasable. - the active / released name fiasco should be adressed. At least provide operator bool(). - set_active should be removed. This is encouraging bad practice. - unique_resource should be moved outside the library. I consider that it belongs to a separate library, and i think it has some flaws in its design that should be addressed. Finally, many thanks to Andrey for proposing this library, and for quickly replying to the questions during the review. Regards, Julien

3 5

[release] Master Branch is closed for ALL changes
by Marshall Clow 07 Dec '23

07 Dec '23

Please do not commit anything to the master branch. Other branches are always fine. The 1.84.0 release is scheduled for next Wednesday, 13-December. — Marshall

1 0

[scope] Scope review starts on November 26th
by Дмитрий Архипов 07 Dec '23

07 Dec '23

Dear Boost community. The peer review of the proposed Boost.Scope will start on 26th of November and continue until December 5th. Boost.Scope is a small library implementing utilities defined in <experimental/scope> from C++ Extensions for Library Fundamentals v3 with a few extensions. Namely, the library contains: * A number of scope guards for various use cases. * A unique resource wrapper that automatically frees the resource on destruction. * Utilities for wrapping POSIX-like file descriptors in the unique resource wrapper. You can find the source code of the library at https://github.com/Lastique/scope and read the documentation at https://lastique.github.io/scope/libs/scope/doc/html/index.html. The library is header-only and thus it is fairly easy to try it out. In addition, the library can be used with Conan and vcpkg (see its README for instructions on how to do that). Finally, there's a single header version suitable for Compiler Explorer (https://raw.githubusercontent.com/Lastique/scope/single-header/include/boos…) As the library is not domain-specific, everyone is very welcome to contribute a review either by sending it to the Boost mailing list, or me personally. In your review please state whether you recommend to reject or accept the library into Boost, and whether you suggest any conditions for acceptance. Thanks in advance for your time and effort! Dmitry Arkhipov, Staff Engineer at The C++ Alliance.

16 83

The standard library is better documented
by Niilo Huovila 07 Dec '23

07 Dec '23

By better I mean cppreference.com. There's a page for pretty much everything in the standard library. Related pages are linked. The preconditions and behavior of functions are specified. Consequentially I come to Boost only when the standard library doesn't do enough. If you are going to compete with the standard library instead of just supplementing it, you need outstanding documentation.

14 26

Master branch is now open for bug fixes
by Marshall Clow 06 Dec '23

06 Dec '23

The master branch is now open for bug fixes and documentation changes. Other changes by release manager permission, as described in https://github.com/boostorg/wiki/wiki/Releases%3A-Beta-Merge-Policy The next deadline: On December 6th, master closes for all changes. As always, the calendar is at https://www.boost.org/development/ — Marshall

10 23

[release] Master Branch will close for the 1.84.0 release TOMORROW
by Marshall Clow 05 Dec '23

05 Dec '23

Don’t forget your release notes, too! — Marshall

1 0

[scope] Formal review - David Sankel
by David Sankel 05 Dec '23

05 Dec '23

- What is your evaluation of the design? I'm overall happy with the design. I like the API provided and think it is useful in many scenarios. The one question I have is why unique_resource::allocated instead of a bool conversion. This is the one aspect of the API that makes me hesitate using unique_resource for things like POSIX file descriptors as .allocated() would be quite foreign for users. - What is your evaluation of the implementation? I gave the implementation only a quick glance, but it looked fine to me. - What is your evaluation of the documentation? The documentation was good. I'd suggest the front page focuses on usage examples instead of a Boost.ScopeExit comparison though. Most people reading this page won't know about ScopeExit and are trying to evaluate whether not this library will solve their problem. - What is your evaluation of the potential usefulness of the library? I think this has wide applicability. - Did you try to use the library? With what compiler? Did you have any problems? I did not attempt to use it. - How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? I spent about an hour working through the documentation. - Are you knowledgeable about the problem domain? Yes And finally, every review should answer this question: - Do you think the library should be accepted as a Boost library? Be sure to say this explicitly so that your other comments don't obscure your overall opinion. Yes, I do.

1 0

Proposal for Inclusion of Async.MQTT5 Library in Boost
by Ivica Siladic 05 Dec '23

05 Dec '23

Dear Boost Developers, we are writing to introduce a new C++ Async.MQTT5 library that we believe would be a valuable addition to Boost. MQTT has become the de-facto standard for IoT communication, with billions of IoT devices relying on it to publish or retrieve data from the cloud. Despite its widespread use, integrating MQTT into a C++ embedded application remains a complex and error-prone task. This challenge primarily arises from the absence of a higher-level library that offers a straightforward interface for message publishing and receiving while abstracting MQTT protocol intricacies and effectively managing various corner cases inherent in MQTT implementation. The primary objective behind developing Async.MQTT5 was to address these challenges comprehensively, allowing developers to focus on the messages they need to send without concerns about underlying network connectivity issues. Async.MQTT5 is a C++20 client implementation of the MQTT 5.0 protocol, leveraging Boost Asio and Boost Spirit. It fully adheres to the Asio asynchronous model, providing comprehensive support for all forms of asynchronous completion tokens, per-operation cancellation, and custom allocators. The library is designed to work seamlessly with any ordered network transport, typically TCP, SSL/TLS over TCP, or WebSockets (secured and unsecured). Our goal with Async.MQTT5 is to offer developers a simple and clear C++ interface for sending and receiving messages using the MQTT 5.0 protocol. Emphasizing code readability was a key design principle of Async.MQTT5. For instance, tedious protocol message parsing/composing is implemented declaratively (partially utilizing Boost Spirit). Additionally, Asio composed operations are implemented using a novel, clear, and highly efficient "chaining functional operators" approach. It is worth noting that the library is already written following Boost style guidelines, and the documentation is crafted and formatted in line with Boost conventions. You can find the library at https://github.com/mireo/async-mqtt5, and the documentation is available at https://spacetime.mireo.com/async-mqtt5/. We highly value your opinion and would appreciate any feedback you can provide on the Async.MQTT5 library. Thank you for your time and consideration. Best regards, Ivica Siladic Bruno Iljazovic Korina Simicevic

9 15

[scope] A few questions
by Julien Blanc 05 Dec '23

05 Dec '23

Hi, I have a few questions regarding the scope library currently being reviewed: * scope_exit is not default constructible. But it can be released and is movable, so a default constructed scope_exit state makes sense (this is the same as a moved-from scope_exit). I find this a rather strange design choice. What is the rationale behind this? There's even a section on the documentation on how to circumvent this: https://lastique.github.io/scope/libs/scope/doc/html/scope/scope_guards.htm… using cleanup_func_t = std::function< void() >; // Create an inactive scope guard first, since the cleanup function is not set yet boost::scope::scope_exit< cleanup_func_t > cleanup(cleanup_func_t(), false); Why can't i just write? boost::scope::scope_exit< std::function< void() > > cleanup; * the documentation says little about memory allocation. I finally found the answer i was looking for in the boost.ScopeExit comparison: scope_xxx objects do not do any heap allocations themselves. Is that a guarantee of the library? Such a guarantee is needed in some code i work with, i need to know if it is a feature of the library that i can rely upon, and not just an optimization that could go away in a further release. * unique_fd is a resource wrapper for files. But on all major systems, closing a file can fail and return an error. It may well be that the file is not completly written correctly, and this is reported not in the write call, but in the final close call. unique_fd silently drops the return value of close. I'm not sure we should encourage this behaviour. Another issue raises when we look at the example code: void write_string(std::string const& filename, std::string const& str) { // Create a unique resource for a file boost::scope::unique_resource< int, fd_deleter > file(open(filename.c_str(), O_CREAT | O_WRONLY)); if (file.get() < 0) { int err = errno; throw std::system_error(err, std::generic_category(), "Failed to open file " + filename); } // Use it to write data to the file const char* p = str.data(); std::size_t size_left = str.size(); while (size_left > 0) { ssize_t written = write(file.get(), p, size_left); if (written < 0) { int err = errno; if (err == EINTR) continue; throw std::system_error(err, std::generic_category(), "Failed to write data"); } p += written; size_left -= written; } } In case of a failure, what we got is an half-baked file, which we know nothing about. A proper write_string function should at least try to do its cleanup and delete the file in case of error. But that defeats the purpose of unique_resource... In my experience, automatically closing file handles is not error handling. But it has its use cases, maybe it would be better demonstrated by an example like: bool compare_files(std::string const& filename1, std::string const& filename2) { boost::scope::unique_resource< int, fd_deleter > file1(open(filename.c_str(), O_CREAT | O_WRONLY)); if (file1.get() < 0) { int err = errno; throw std::system_error(err, std::generic_category(), "Failed to open file " + filename1); } boost::scope::unique_resource< int, fd_deleter > file2(open(filename.c_str(), O_CREAT | O_WRONLY)); if (file2.get() < 0) { int err = errno; throw std::system_error(err, std::generic_category(), "Failed to open file " + filename2); } // do the comparison } Regards, Julien

2 7

[scope] Mr. Docs rendered reference
by Krystian Stasiowski 05 Dec '23

05 Dec '23

Hi everyone, My name is Krystian, and I'm a Staff Engineer at The C++ Alliance. I’ve been working on Mr. Docs, a clang based C++ documentation generator intended to serve as an alternative to Doxygen. Although Mr. Docs is not yet ready for release, I noticed that there was an issue with rendering the exception specification of destructors due to a bug in Boostbook. Thus, I would like to present Mr. Docs as a potential future alternative, and have generated a multi-page reference for Boost.Scope to demonstrate its capabilities: https://mrdocs.com/demos/develop/boost-scope/multi/adoc-asciidoc/boost/scop… The output was created using the built-in Asciidoc generator and converted to HTML with asciidoctor. Since the Asciidoc generator uses handlebars templates to format the output, customizing the rendered documentation is a matter of modifying the templates. Unlike Doxygen, Mr. Docs operates directly on the clang AST. This means that support for new/obscure language features is only limited by the extent of clang’s conformance to the latest C++ standard. Any feedback is greatly appreciated. 🙂 Best, Krystian Stasiowski

5 6