- Boost - lists.boost.org

[JSON][review] Some early feedback
by Andrzej Krzemienski 15 Sep '20

15 Sep '20

Hi Everyone, The formal review of Boost.JSON has not started yet, but I thought I could offer some feedback from my playing with the library earlier in the process. Documentation --------------- Very well written. It makes it easy to grasp quickly the spirit and the potential of the library, and the reference section makes it easy to navigate through the features and their interfaces. Some minor issues: 1. Section "Using Numbers" is missing content. 2. Description of storage_ptr's destructor and move constructor is missing noexcept. 3. Docs in a couple of places call `storage_ptr` a "smart pointer container". Is it intentional? "Container" has a very specific meaning in C++. Design. ----------- It is generally sound, and well explained in the rationale. Here are a couple of minor remarks. 1. The usage of `nullptr` for null values. `nullptr` has been introduced to mean "a pointer with null value" and is only used in the standard library for smart pointers, with an ignoble exception of `std::function<>`. Using it for `json::value` is not an obvious choice for me. On the other hand, we do not have a universal type representing a missing value. A library could provide its own `json::null`, although I am not sure if this is an ideal solution either. 2. Why is json::value Semiregular rather than Regular? Equality comparison has a natural interpretation and application for checking if the produced JSON looks exactly as expected, or for checking if some two subtrees are the same. Is it because of efficiency issues with json::object comparison? 3. The library is using a custom string type. We get a rationale for this, and I accept it. But there are still some concerns about it that I would like to mention. 3.1. It is not the first library reviewed his year to have a custom type with std::string-compatible interface. It looks like the idea of having a "vocabulary" string type is unrealistic. If this should be the case, maybe we need a concept specifying what a string-compatible type should provide. It is obvious that json::string does not literally provide the same interface as std::string. Nor does anyone expect 100% compatibility. 3.2. Docs say that json::string is like std::string *except for* a number of things. For instance it does not provide the controversial assignment from `char`. Given that you are "fixing" the shortcomings of std::string, maybe it is an opportunity to fix other things in it also. For instance, std::string has been criticised for having too big an interface. That it mixes an iterator-based interface with an indexed-based one. Myb functions like `find_first_not_of()` are not necessary, given that users can use all sorts of iterator-based algorithms? 3.3. The rationale for json::string says that inisializer_list constructor is unnecessary, because string literal can always be used instead. This does not take into account that initializer_list does not necessarily operate on constant values. There is no concise substitute for: ``` std::string arrange(char a, char b, char c) { return {c, a, b, c}; } ``` I would not miss this construction much, I guess, but I would expect this to be mentioned in the rationale. 4. One thing one often expects of a JSON library is to be able to pretty-print a JSON document. The docs give an example how to do it, but the library does not provide this feature out of the box. Is pretty-printing not compatible with the design goals of the library? Implementation -------------- I didn't look much at it yet, but here is something I found. 1. `json::string_view` is convertible from `std::stirng`, but it is not convertible from `std::string_view`. I think there is no harm in enabling this conversion, and it seems useful. 2. The following piece of code ``` json::value v = 50.0; std::cout << json::serialize(v); ``` oututs: 5E1 which is technically compliant with JSON specification, but quite surprising. Is there a reason for this? Can serialization be controlled to output 50.0 or 50? 3. It is confusing for me to see member function serializer::read. I would think that the action that a serializer does is *write* rather than read. Am I missing something? 4. I really appreciate the ability to use the library in header-only mode with <boost/json/src.hpp>. This makes library testing much simpler. Regards, &rzej;

7 10

[json][review] A review from an embedded perspective
by Brooks Prumo 15 Sep '20

15 Sep '20

Hi everyone, this is my first time mailing the Boost developer list, and also submitting a review. Thank you in advance for understanding any issues it may have! I'm writing to review Boost JSON. I've been using it very lightly for the last week or so. My usage has been exclusively on embedded MCUs (ARM Cortex M4, compiling with ARM GCC 9 with C++17), which I'm guessing hasn't been the common case so far. Here's a list of things that matter a lot to me: - Compile from source. Boost JSON passes. - No exceptions. C++ exceptions are turned off, so all libraries I use must support not throwing. Boost JSON passes. - No iostreams/locale. If a library uses iostreams, my binary jumps ~220 KB which is a deal breaker. Boost JSON passes. - Binary size matters a lot, since onboard Flash is very limited. I am using a "big" MCU, and that means my whole binary must be under ~512 KB. Right now Boost JSON is not great. In doing a simple serialization and parse, the library itself adds ~58 KB to my .text section. This is OK for now, but surprisingly big. I know the team is aware of this and has some ideas to reduce the binary size. I will likely need those fixes to actually port my app to Boost JSON. - Control over the sizes of any "large" stack buffers. Boost JSON passes. - No external dependencies that do not meet the above requirements. Boost JSON passes. With this, Boost JSON is an ACCEPT from me. THe biggest reason for me is that the API is nice to work with. I started off with Nlohmann, but the binary size was too big due to iostreams. I'm currently using RapidJSON, and it's definitely not easy to use. In my app, JSON is not in the hot path. I want a library that is easy to use and meets my requirements above. I greatly appreciate being able to use a modern C++ library. And answers to the other questions: - What is your evaluation of the design? So far so good. I do not have complicated JSON uses though. - What is your evaluation of the implementation? I've only looked at the implementation when it comes to binary size. - What is your evaluation of the documentation? Documentation is nice. I personally would enjoy more examples, as that is how I commonly learn new libraries. - What is your evaluation of the potential usefulness of the library? HUGE! I haven't come across another modern C++ JSON library that'll work in the embedded space I live in. It's really exciting! - Did you try to use the library? With which compiler(s)? Did you have any problems? Yep! I used ARM GCC 9.2.1 with C++17. No issues with compilation. - How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? I'd say my evaluation is "initial". I've used it in a way that mimics my real usage, but haven't used many aspects of the whole library. - Are you knowledgeable about the problem domain? Minimally. Just basic JSON knowledge. Thank you! Brooks

3 2

[preprocessor] Appveyor weirdness
by Edward Diener 14 Sep '20

14 Sep '20

In my appveyor.yml file for preprocessor one of the b2 command lines for a test run uses an explicit alias for the target. When run in appveyor.yml I get the messages: notice: could not find main target preprocessor_limits notice: assuming it is a name of file to create . and don't know how to make <e>preprocessor_limits Yet when I run the same test locally using the preprocessor_limits target, everything works fine. Is Appveyor somehow different when running Boost Build tests than when I run it locally ?

1 0

No doc for qvm in 1.74 distro
by Edward Diener 12 Sep '20

12 Sep '20

There is no doc for the qvm library in the 1.74 distro.

2 1

[JSON][review] Should there be an equality operator/function?
by Hadriel Kaplan 12 Sep '20

12 Sep '20

Howdy, I’ve been testing Boost.JSON in preparation for writing a review, and one thing that I found surprising might be better discussed in a separate thread, so here goes… There’s currently no comparison function or operator to compare one json::value with another. While I appreciate that a deep comparison isn’t cheap - it’s O(N) for the average case but O(N^2) in the worst case I believe - I still think it’ something a JSON-based container value type should have out-of-the-box. It’s something other popular JSON libraries offer; usually with equality operators, although a named function would be ok too if that was required instead. I hit the lack of it almost immediately, since test code/assertions frequently check equality. I’m not sure how frequently it’s used for run-time code in general, however. -hadriel

1 1

Looking for a review manager for Real
by Damian Vicino 10 Sep '20

10 Sep '20

Hi, GSOC finished and we are almost-ready for proposing to review a new library, Real. We are looking for a volunteer to be a review manager for it. Best regards, Damian

9 15

Do I need to use boost::mutex in deadline_timer?
by Jupiter 10 Sep '20

10 Sep '20

Hi, I used boost::asio::io_service for a single thread implementation but with multiple boost::asio::deadline_timer objects, because it is a single thread, I don't think I need to use boost::mutex to lock and unlock tasks running by deadline_timer, am I wrong about it? Thank you. Kind regards, jupiter

3 4

Re: [boost] boost.real
by Kostas Savvidis 09 Sep '20

09 Sep '20

Hi Damian, Laouen, I have been able to compile the test programs with your library after making some simple changes. The patch is below the message. However the example program from your documentation still does not compile, unless I comment out some lines - you can see the listing below. I ran a tricky calculation which is from the Aberth book, your program is able to give the answer and moreover the true value is indeed between the upper and lower bounds that your program gives. This is good! The conclusion is that your library can do useful work, but it is very limited in capability. No special functions, trig is there but not working well, and no simple way to interface with linear algebra. I may want to use it at some point, but I would need at least the ability to cast int and double to your boost::real::real type, because as it is now, the only way to import numbers into your software seems to be through std::string. Finally what seem to be some bugs: 1) unsigned int real::maximum_precision = 10; // error: specializing member 'boost::real::real<int>::maximum_precision' requires 'template<>' syntax 2) auto it = e.cbegin(); // error: 'using real = class boost::real::real<int>' {aka 'class boost::real::real<int>'} has no member named 'cbegin' 3) real::sin(f) // enters infinite loop 4) real c = (std::string)"0.999999"; prints: c: [0.99999899999999999999999999999999999999999999999999999999999999999999999999999999999999999933600778729, 0.99999899999999999999999999999999999999999999999999999999999999999999999999999999999999999982691714296] as far as i can see the interval is incorrect ! Best Regards, Kostas ============================================================================================= diff -r -u real.orig/integer_number.hpp real/integer_number.hpp --- real.orig/integer_number.hpp 2020-09-07 16:22:04.000000000 +0300 +++ real/integer_number.hpp 2020-09-08 14:11:22.000000000 +0300 @@ -103,7 +103,8 @@ } // this function always expects (*this) to bigger than other, so if borrow is not zero at the end // there should be error - if(borrow != 0); + if(borrow != 0) + ; //throw subtraction_number_required_lower_exception(); diff -r -u real.orig/real_rational.hpp real/real_rational.hpp --- real.orig/real_rational.hpp 2020-09-07 16:22:04.000000000 +0300 +++ real/real_rational.hpp 2020-09-08 11:59:51.000000000 +0300 @@ -262,7 +262,7 @@ // constructor to generate get number from string constexpr explicit real_rational(std::string_view num){ // searching for "/" in string - size_t pos; + size_t pos=0; bool found = false; std::string_view num1, num2; pos = num.size(); ============================================================================================= #include <iostream> #include <string> #include <real/real.hpp> using real = boost::real::real<int>; //unsigned int real::maximum_precision = 10; //unsigned int real_algorithm::maximum_precision = 10; int main() { real c = (std::string)"0.999999"; real d = (std::string)"0.999999"; real e = c + d; std::cout << "c: " << c << std::endl; std::cout << "d: " << d << std::endl; std::cout << "e=c+d: " << e << std::endl; // ANSWER IS WRONG ??!!! // for(auto it = e.cbegin(); it != e.cend(); ++it) { // std::cout << "e iteration " << it.approximation_interval << std::endl; // } real g = (std::string)"0.999998"; std::cout << "g: " << g << std::endl; if (g < d) { std::cout << "g < d --> true" << std::endl; } else { std::cout << "g < d --> false" << std::endl; } real h = d - g; std::cout << "h=d-g: " << h << std::endl; std::cout << "h++ is: " << h+real("1.0") << std::endl; { // Example from Aberth 1992, Precise Computation Using Range Arithmetic real a("77617.0"); real b("33096.0"); std::cout << "\n\n" << "a: " << a << std::endl; std::cout << "b: " << b << std::endl; real f = real("333.75")*b*b*b*b*b*b + a*a*(real("11.0")*a*a*b*b-b*b*b*b*b*b - real("121.0")*b*b*b*b-real("2.0")) + real("5.5")*b*b*b*b*b*b*b*b + a/(real("2.0")*b); // should be -0.827396 std::cout << "f: " << f << std::endl; //std::cout << "sin(f): " << real::sin(f) << std::endl; // doesn't come out of the loop } return 0; }

2 1

[LEAF] Review Results
by Michael Caisse 09 Sep '20

09 Sep '20

I want to thank Emil for his patience for the publishing of the review results as life got in my way. Thank you Emil for submitting LEAF to be reviewed for inclusion into Boost. As an already-Boost-author, you knew what you were getting into and continued into the process. I also want to thank you for once again pushing our thoughts on error handling. -------- Result -------- This is the part everybody scrolls to the bottom to read, so let’s just put it at the top. Boost.LEAF is ACCEPTED into Boost. After the following modifications are made to the library, the repository can be transitioned to Boost for inclusion into the next release (with the release cycle timing constraints): Specify the TLS requirement sooner in the documentation. Optional usage of Boost.Config to perform all of the Boost Configy things. (further discussion on this below) Macro naming and general Boostification clean-up. -------- Reviews -------- There was a good amount of discussion about the library on both the dev and user mail lists. I want to personally thank those who got involved in the review. You make the process work! Result summary: Accept: 7 Reject: 1 No-decision: 3 The no-decision reviews provided valuable input and discussion threads. While it would be my preference to have the review authors provide a final verdict, I was grateful for the time and energy they spent with the library and formulating thoughts into emails for discussion. Some comments from reviews: “I find function preload() really great and novel. … This really makes the error reporting code brief, clean and bug-free.” “I like LEAF very much and there's a lot offered here…” “I like it.” “...it allows bridging multiple error handling strategies and usage patterns in an efficient way.” About the docs: “They look good and manage to explain hard concepts in a very approachable fashion.” “The documentation is great -- in terms of styles it's probably the best I've seen for a C++ library.” “Clear, friendly and complete.” -------------- Some Thoughts -------------- While there was overwhelming consensus that LEAF should be included into Boost, the process is not democratic. I have spent considerable time understanding the concerns raised in each of the reviews and in particular the singular rejection. Emil took time responding to each of the reviews. The exchanges were fruitful and both “conditional accept” and “reject” reviewers changed their verdicts to accept along the way. Most of the concerns were amicably resolved. I am comfortable with the design choices and implementation made by the author for the remaining items. One reviewer brought up the re-implementation of Boost.Config type facilities. This of course is a complex issue for a library with zero dependencies. The ability to simply drop the Boost.LEAF directory into a project and run is a definite plus for a specific user segment. Being able to modify a single Boost.Config repository to support WackyCompiler14 targeting LEG Core and get a bunch of libraries for the ride is also a plus. Emil and I have discussed these concerns and the best choice seems to be defaulting to zero dependencies and the option to pull in Boost.Config to provide configuration type facilities if needed. Accepting a library into Boost is as much about the library as it is bringing the author into a group. While Emil is already part of the author crowd, it was clear to me through his careful, informative, and patient responses that the addition of Boost.LEAF brings professional maturity that strengthens the community. A user with more than a year of experience with the library voted acceptance based on the “qualities of the library itself and the responsiveness and openness for feedback of its author.” Throughout the review period, Emil integrated feedback to both the documentation and code in another branch. It was wonderful to see the community provide input and the author respond so quickly to produce a better final result. Thank you Emil for your vision of better error handling and striving to bring us along! -- Michael Caisse Ciere Consulting ciere.com

3 2

[log] logfile not created where expected
by Barend Gehrels 08 Sep '20

08 Sep '20

hi, I encountered a bug, most probably, in Boost.Log It is reproduceable in the tutorial_logging.cpp : https://www.boost.org/doc/libs/1_74_0/libs/log/example/doc/tutorial_logging… The short code should create a file named "sample.log". But it does not, instead it creates a file called "00000.log", with the expected lines logged in that file. If I compile and run exactly the same tutorial in Boost 1.70, the file sample.log is indeed created with the same content. The problem is already there in Boost 1.73 What is the problem? Is there a workaround for it? Thanks, regards, Barend

3 2