- Boost - lists.preview.boost.org

Re: [boost] [Boost-users] [review][fixed_string] FixedString review starts today
by Peter Dimov 02 Dec '19

02 Dec '19

> > Zach Laine wrote: > > Ok, I tried this myself on Quickbench: > > > > http://quick-bench.com/WZPNHFBKI-hFxThBYRZVjndtBho > > I tried to fix that to add the null terminator, but as a result both > push_backs were entirely optimized away. > > http://quick-bench.com/DtPR9Yov1TSzUFeYRx0ipVvUYBk Wait, I forgot to increment the size! http://quick-bench.com/sI71NY7OpQ8ft27HpELko8q_4xM Not much improvement anyway.

1 0

Boost 1.69 on Fedora/PPC64LE: tests results
by REIX, Tony 02 Dec '19

02 Dec '19

Hi, I'm testing Boost v1.69 on Fedora 31 / PPC64LE , as explained at: https://www.boost.org/development/running_regression_tests.html And I see many errors: 1 Failed test out of about 11 Run tests: # cd status # ../b2 ... Passed 101532 tests Failed 10368 tests 90.7% of tests completed sucsessefully Passed 1188 tests Failed 15 tests 98.8% of tests completed sucsessefully I do not know what I made wrong. So, since I do not see any Fedora/PPC64LE machine in the BuildFarm: https://www.boost.org/development/tests/master/developer/summary.html I'd like to know where I could find the summary of the official tests of Boost 1.69 on Fedora/PPC64LE. Thanks/Regards, Tony Reix

1 0

Matrix Multiplication
by Siddhartha Sen 01 Dec '19

01 Dec '19

Having gone through quite a few libraries, the ones that intrigued me the most are Boost.Algorithms and Boost.GIL. I had an idea for Boost.GIl in fact. Basically what I wish to propose(please do correct me if it already exist) is a library that will have the functionality to convert a image to a negative one (I found the conversion of colour image to black and white functionality, but not this). Also, the process can be massively sped up by the incorporation of parallel programming paradigms like OpenMP. Do let me know what you guys think about this.

1 0

[fixed_string] Why "fixed" rather than "static"?
by Andrzej Krzemienski 29 Nov '19

29 Nov '19

Hi Everyone, First, I would like to thank Vinnie and Krystian for writing and sharing the library. While this is not a review, I would like to share a thought about the choice of name. Given that we have static_vector, which seems to be doing a similar thing, why was this library called fixed_string rather than static_string? I note that name fixed_string has some potential to mislead the users as it might imply that it is the size that is fixed (like "a string that always stores 2-letter country codes"). I am not saying that the name choice is wrong. I would just want to see the rationale. Regards, &rzej;

5 8

Re: [boost] [Boost-users] [review][fixed_string] FixedString review starts today
by Peter Dimov 29 Nov '19

29 Nov '19

> and yes, you can write code that will be correct if op+= doesn't check. Maybe you can, but I apparently can't. This is what I just wrote: void my_append( fixed_string<512> & s, std::string_view s1, std::string_view s2 ) { if( s.size() + s1.size() + s2.size() > s.max_size() ) throw std::length_error( "" ); s += s1; s += s2; } Is this correct? (Spoiler: no.)

2 2

Re: [boost] [Boost-users] [review][fixed_string] FixedString review starts today
by Peter Dimov 29 Nov '19

29 Nov '19

> https://godbolt.org/z/Z37Pmx > https://godbolt.org/z/CXHRKQ I forgot to maintain a 0 terminator in this sketch; it's even worse with it. .L5: add rax, 1 movzx ecx, BYTE PTR [rdx] cmp rax, 512 ja .L11 add rdx, 1 mov BYTE PTR [rdi+7+rax], cl mov BYTE PTR [rdi+8+rax], 0 mov QWORD PTR [rdi], rax cmp rsi, rdx jne .L5 https://godbolt.org/z/nhYZQY .L3: movzx ecx, BYTE PTR [rdx+rax] mov BYTE PTR [rdi+8+rax], cl add rax, 1 mov BYTE PTR [rdi+8+rax], 0 mov QWORD PTR [rdi], rax cmp r8, rax jne .L3 https://godbolt.org/z/pGJRWs

1 0

Re: [boost] [review][fixed_string] FixedString review starts Nov 25
by Peter Dimov 28 Nov '19

28 Nov '19

> Also, a synopsis section in the documentation might have been nice, to > allow one to see all the declarations in one place. (Admittedly, the main > header does serve that purpose somewhat.) I join the others in noting the pervasive lack of constexpr and noexcept. Even the default constructor isn't either. Can't think of a reason why it would throw. (The lack of constexpr is much easier to explain.) Also, Traits template parameters were wrong in C++98, and they remain wrong today, even though the committee has propagated that mistake into string_view.

2 1

Re: [boost] [Boost-users] [review][fixed_string] FixedString review starts today
by Peter Dimov 28 Nov '19

28 Nov '19

> I would "precondition" `op+=( char const (&)[ M ] )` (and op=) by failing > at compile time when M > N, because the size is known, but in > fixed_string's case, the size isn't known. No I wouldn't. :-) The size isn't known in this case either. It's better to diagnose_if the whole thing.

1 0

[STLInterfaces] only three operations?
by Andrzej Krzemienski 28 Nov '19

28 Nov '19

Hi, While the formal review of STLInterfaces has not yet started, some early feedback may be of help. The idea to implement relational operators from binary operator- for random_access_iterator looks too clever for me. Maybe this deserves a mention in the documentation it is from operator- that the library will generate all relational ops. For non-contiguous random_access_iterators (like in deque) it might be suboptimal to do it this way: a directly provided operator== might be faster than the one synthesized from operator-. Of course, I can provide the 6 relops operators myself, but I could have only 2 if the facade were capable of generating operator!= from operator== (and similar). Could operator!= for random_access_iterator be generated from operator!= ? Regards, &rzej;

3 5

[FixedString] Deniz' review
by Deniz Bahadir 27 Nov '19

27 Nov '19

This is my review of Krystian Stasiowski and Vinnie Falco's FixedString library. The important question and its answer upfront: - Do you think the library should be accepted as a Boost library? I think it should be ACCEPTED into Boost. It is a very small (almost trivial) library and in the beginning I was not sure if it really should go into Boost. However, for me it looks like a useful building-block, so I vote ACCEPT. And if I remember correctly Vinnie said he is already having some other libraries using it which he wanted to propose for Boost later, so there seem to be some real use-cases for it already. Besides, Boost already has some other smaller building-block libraries, so FixedString should be in good company. On a side-note: It would be helpful for the Boost library documentation overview (e.g. https://www.boost.org/doc/libs/1_71_0/) to allow more views/sort-orders for listing available Boost libraries than the views/sort-orders which are already there. (Having "building-blocks" and especially "newer C++ standard features for C++03" or similar "standard compatibility" categories for the Boost libraries comes to mind.) OK, now back to the review. - What is your evaluation of the design? The overall design looks straight forward. `boost::fixed_string`'s API resembles that of `std::string` and only the internal representation differs largely. This looks like the correct design for the aimed purpose of this library (to provide a dynamically resizable string with compile-time fixed capacity.) - What is your evaluation of the implementation? The implementation looks solid and also straight forward. However, I have some remarks to make which you can read further down in this email. - What is your evaluation of the documentation? The documentation is quite small, except for the reference section. This, however, should be fine considering the very simple and small focus which FixedString has. Regarding the "Iterators" section, it was not clear to me on first read for which string type the iterators are invalidated when moving/swapping. I think, `boost::fixed_string` is meant and not `std::string`, but that was not obvious from first read and the wording should probably be improved there to be unambiguous. (Maybe make this clear by speaking of "FixedString" or "fixed_string" instead of just "string".) - What is your evaluation of the potential usefulness of the library? For the use-cases mentioned in the "Motivation" section of the documentation it seems to be useful. All other use-cases I could think of are specializations of the mentioned use-cases. - Did you try to use the library? With what compiler? Did you have any problems? No, I have not tried to use the library. However, I am curious to know if Boost.String_Algorithms can operate on `boost::fixed_string`, too. If not, it would be nice for the code of FixedString and/or Boost.String_Algorithms to be modified in such a way that they can inter-operate with each other. - How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? I read the documentation, but the reference section I merely visually scanned through. (I assume, there are no mistakes in it, as they mainly resemble the API of `std::string`.) I looked at the implementation but did not look at all function implementations in all depths. I, however, looked at the CMakeLists.txt file, too, and have some remarks for it (see below). Overall, I probably spent about 1.5 hours for the review, not including the writing of this email. - Are you knowledgeable about the problem domain? Only as far as it gets if you are a general user of `std::string`. I have the following additional remarks: * `boost::to_fixed_string` / `boost::detail::max_digits`: - Is the returned `boost::fixed_string` type of `boost::to_fixed_string` large enough to also hold the (plus) sign of an unsigned integer (if a user might want to append it later)? (Reading the comment and implementation of `boost::detail::max_digits` I assume the answer is yes.) Maybe, make this clear in its reference documentation. * "boost/fixed_string/config.hpp": - At the top of this file we have `#define BOOST_FIXED_STRING_USE_BOOST`, which therefore makes all following conditional definitions always use Boost equivalents of standard library types internally. Is it really desired to have it hard-coded here? Shouldn't the user decide if s/he wants to use Boost compatibility types instead of the ones coming from the standard library? * Filenames - I do not really like, that all files have the same name "fixed_string.hpp". There is the main one in "boost/fixed_string", another one in "boost/fixed_string/detail" and a third one in "boost/fixed_string/impl". I would prefer these files having some (at least slightly) different names. * "CMakeLists.txt": - This CMakeLists.txt file is written in Traditional CMake (old style CMake) although requiring CMake 3.5.1 which knows how to handle Modern CMake. You should consider using Modern CMake instead. - You should leave `CMAKE_CXX_FLAGS` untouched. It is for the user of the library, not for the developer! - Similar, set requirements for a specific minimum required C++ version using the `CXX_STANDARD` property instead of hard-coding it into `CMAKE_CXX_FLAGS`. - You should probably replace usage of `include_directories` with `target_include_directories` on the specific targets. - Similar for `add_definitions` and `link_directories`. - Using `file(GLOB...)` for globbing sources is dangerous and CMake might not always realize that some sources have changed. (see note at: https://cmake.org/cmake/help/latest/command/file.html#glob) - In general, I am not sure if that CMakeLists.txt file should have been part of the review, but I recommend to provide a useful and more modern one if this is going to become a part of Boost, too. Best regards, Deniz

6 10