- Boost - lists.boost.org

[variant2] Andrzej's review -- documentation
by Andrzej Krzemienski 10 Apr '19

10 Apr '19

Hi Everyone, This is the review of the documentation of variant2. First, the docs assume that the user is already familiar with std::variant or boost::variant, and the introductory part only highlights the difference from std::variant. It does not even mention that variant<X, Y> always stores either X or Y. "Type-safe discriminated union (variant) type," sounds arcane to novices. Notation variant<T…> is also confusing to someone who encounters variant for the first time. There is no initial example. No mention of visit() which is the most important part of the interface. Compare this with Boost.Variant docs: https://www.boost.org/doc/libs/1_69_0/doc/html/variant.html I do not think the library should assume this. Boost libraries are characteristic of documentation that introduces potential users to the problem domain and gives a sort of tutorial that shows how to use the library. I recommend doing the same for variant2. This library does a lot of contortions to avoid the valueless state, but does not describe what the motivation for it is. The docs also do not give a sense of what performance (spacial and runtime) trade offs this library does. For instance, we do not know if the double-buffer scenario th second buffer is within variant's storage or on the heap. Or we do not know about secret moves that the library does internally, which gives surprising behavior to the users that store non-movable types in variant. Consider the following example: ``` struct X {}; struct Guard { explicit Guard(int) {} Guard(Guard&&) = delete; }; variant<X, Guard> v {}; v.emplace<Guard>(1); ``` This works fine with std::variant, but fails to compile with variant2::variant, because the latter requires all the alternative types, at least in one implementaiton of the never-empty trick, to be move-constructible. But this requirement is never listed in either global requirements for alternative types stored in variant, or in the specification of functions `emplace()`. Here's a wandbox example: https://wandbox.org/permlink/hGbRtgzpUrpF0ODy The Reference part of the docs is really good and detailed. Some minor "bug reports" follow. In the specification of functions types `T0` and `Ti` are used, but `T0` or `Ti` are never defined. Wording in the C++ Standard also refers to `T0` and `Ti`, but also provides a definition: In the descriptions that follow, let i be in the range [0, sizeof...(Types)), > and Ti be the ith type in Types. > <http://eel.is/c++draft/variant.ctor#1.sentence-1> > See http://eel.is/c++draft/variant.ctor#1 A similar definition should be put in variant2's documentation. Noexcept specifications use mp_all. this assumes that users of variant2 must be familiar with Boost.mp11. I recommend removing the reference to mp_all and instead use something similar to what the Standard does: http://eel.is/c++draft/variant.ctor#11 Specification of function `emplace<I>()`: It says: > > On exception: If the list of alternatives contains monostate, the > contained value is either unchanged, or monostate{}; > In what situation on exception is the contained value left unchanged? This question applies to the next bullet as well. The specification also doesn't mention that `.emplace<I>()` requires Ti to be move-constructible in some cases. It also says: Throws: Nothing unless the initialization of the new contained value throws. Which sounds strange, because it does throw something. In other places you use "Throws: Any exception thrown by the initialization of the contained value.", which makes more sense. Also, the Standard uses, "Throws: Any exception thrown during the initialization of the contained value. <http://eel.is/c++draft/variant.mod#11.sentence-1>": http://eel.is/c++draft/variant.mod#11 swap(): What does it throw and when? Other functions specify it but this one does not. get<I>(): Specification says, > > If v.index() is I, returns a reference to the object stored in the > variant. Otherwise, throws bad_variant_access. > This means that if I provide an index that is far greater than `sizeof...(T)`, the program should compile fine and throw an exception at run-time. This is not what the implementation does: I get a compile-time error, which is good, but inconsistent with the docs. They need an additional "Requires:" clause. visit(): I think it is underspecified. It is not immediately clear that the first parameter is the visitor and the remaining ones are variants. Parameter names F and V do not convey this clearly. Either use Visitor and Variants... or provide a longer description, even if informal. The docs really should contain an example of `visit()` somewhere. According to the specs, the following should work: ``` void f() {} variant2::visit(f); ``` which is not incorrect, but strange. The std::variant also provides a second overload of `visit` that explicitly defines the return type: http://eel.is/c++draft/variant.visit This overload is not present in variant2. But this fact is not listed in the initial section of the docs that lists the differences between the two libraries. Regards, &rzej;

2 6

[variant2] Comments on documentation
by Bjorn Reese 10 Apr '19

10 Apr '19

The overview states that "variant<T...> is not trivial when all contained types are trivial". Why not? Is this an advantage somehow or an unfortunate limitation? Why is there no tutorial? Why is there no design rationale? Change "variant_npos = -1" into "variant_npos = unspecified" Add a comment about the purpose of valuesless_by_exception(). Without such a comment, the function seems very odd on a "never valueless variant type." The destructor should be marked as noexcept.

4 8

[variant2] Review - Review period extended until April 15
by Michael Caisse 09 Apr '19

09 Apr '19

Hi all - The review period for Boost.Variant2 has bee extended to April 15. Thank you to those who have participated so far. There has been a fair amount of discussion about the merits of never valueless. A core design _feature_ of Variant2 is never valueless. You may have made different trade-offs. Understanding motives makes sense but in this review we are not trying to change this basic design choice. This paper by Peter discusses valueless variants some and may be useful reading: <http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0308r0.html> If you haven't started your review yet, you have time! Reviews ranging from detailed discussions of inner workings to a user-perspective of the library and documentation are all helpful. I personally believe we need more user-based reviews in Boost. So, feeling sheepish about reviewing? Don't! Your review is important in the process. Here is one more motivation if you are on the fence ... Peter's code is a joy to read. The library is extremely clean and the code is accessible. There might be all kinds of things you learn from reading the source. Thank you again community for staying involved. We get great libraries when people ask questions, review, and provide input to everything from the docs to the design. Happy coding! michael -- Michael Caisse Ciere Consulting ciere.com

1 0

GSOC 2019 proposal
by Siddhartha Sen 09 Apr '19

09 Apr '19

Once the proposal is ready, where do I officially submit the pdf?

2 1

Re: [boost] [release] Boost 1.70.0 Release Candidate 1 available
by Ion Gaztañaga 09 Apr '19

09 Apr '19

On 07/04/2019 15:07, Rainer Deyke via Boost-users wrote: > On 07.04.19 14:13, Marshall Clow via Boost-users wrote: >> On Sat, Apr 6, 2019 at 10:53 AM Rainer Deyke via Boost-users < >> boost-users(a)lists.boost.org> wrote: >> >>> On 04.04.19 16:27, Marshall Clow via Boost-users wrote: >>>> 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. >>> >>> boost::movelib::adaptive_merge continues to be broken if >>> iterator_traits<RandIt>::size_type != std::size_t. This breaks >>> boost::container::flat_map when cross-compiling for MacOS. I have >>> previously reported this error here. >>> >>> I consider this a critical error - as in, I won't be able to upgrade to >>> Boost 1.70 at all unless this error is fixed for the release. >>> >> >> This is https://github.com/boostorg/move/issues/25, correct? > > Yes. Sorry for note replying earlier, I was on a trip. The fix is not complicated but several files need to be modified. Committed to develop: https://github.com/boostorg/move/commit/bfdfca0b88fe1cf4e67b42fcacd5e126e93… @Rainer: I've checked this fix in several msvc and gcc's locally. The error report does not contain any example for the bug, so I don't know if I've fixed your use case. I don't know if we are still on time for 1.70. Best, Ion

4 5

[variant2] request to extend the review period
by Andrzej Krzemienski 08 Apr '19

08 Apr '19

Michael, Peter, Would it be possible to extend the review period? I may only be able to devote more time to the review on Thursday, so if it was possible to extend the review at least till 12th, I might have a chance to get it done. I suspect, I might be not the only one in this situation. Regards, &rzej;

1 0

Re: [boost] variant - off topic
by Bjorn Reese 08 Apr '19

08 Apr '19

On 4/3/19 11:27 PM, Robert Ramey via Boost wrote: > d) insert your own use case here. That is, look at your own usages of > assignment to these types and see if there is not an easy way to > re-factor your code to avoid assignment. Such refactoring is difficult for state machines where you use a variant to denote the current state.

4 9

Enabling spectre mitigation in boost libraries
by Riff J 08 Apr '19

08 Apr '19

Hi everyone, I am a developer from Microsoft and currently using boost in our project. We recently get noticed by our security team, that the boost library we use, are not compiled with spectre mitigation (/Qspectre) enabled. Since boost is super powerful, it might not be a good idea to write our own or maintain our own version, so we are reaching out for help. Could anyone please help see if we could enable /Qspectre option in the official build of boost? And here is the vcblog about the spectre mitigation, if anyone is interested: https://blogs.msdn.microsoft.com/vcblog/2018/01/15/spectre-mitigations-in-m… . And since this is the first time I use this mail list, please let me know, if I didn't use it correctly or anything I should pay attention to. Thanks a lot, Riff

10 22

[gsoc19] Boost.Geometry Delaunay triangulation and Voronoi diagrams
by Georgios Venizelos 08 Apr '19

08 Apr '19

Hello, I would like to undertake the Delaunay/Voronoi project in Boost.Geometry library. I have already shared my proposal on GSoC through Google Docs. I think that Delaunay triangulation is a process that has many applications, so it is useful to have such a process implemented in Boost libraries. Main focus of the program would be to implement Delaunay and Voronoi for Cartesian coordinates in 2 and 3 dimensions. If finished early, this could extend either in higher dimensions or in other coordinate types. Would anyone be interested in mentoring such a GSoC project? With respect, Georgios Venizelos --- This email has been checked for viruses by Avast antivirus software. https://www.avast.com/antivirus

1 0

Re: [boost] Enabling spectre mitigation in boost libraries
by Kenneth Porter 08 Apr '19

08 Apr '19

On 4/6/2019 9:46 AM, John Maddock via Boost wrote: > One thing we could look at for future releases would be to provide > differently-named binaries for /Qspectre. Anyone else have thoughts > on that? I just looked at the libraries available via the Visual Studio installer and I see this in my installation (using atls as an example MS-supplied library): C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.20.27508\atlmfc\lib\spectre\x64\atls.lib C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.20.27508\atlmfc\lib\spectre\x86\atls.lib C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.20.27508\atlmfc\lib\x64\atls.lib C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.20.27508\atlmfc\lib\x86\atls.lib That may give some guidance as to how MS can package the Boost libraries for internal distribution.

1 0