Re: Boost.Charconv review runs January 15th - 25th, 2024

From: Andrey Semashev (andrey.semashev_at_[hidden])
Date: 2024-01-24 18:35:33

On 1/24/24 20:57, Christopher Kormanyos via Boost wrote:
>   >>Â The review of Boost.Charconv by Matt Borland runs>> Monday, January 15th through January 25th, 2024.
> 
>> Dear all,
>> This post is intended to kindly encourage the> submission of more reviews for the proposed> Boost.Charconv.
> Again, I kindly encourage you to review andsubmit review for the exciting proposedBoost.Charconv, which aids in domain-specificsolutions and portability.
> 
> We have now 1 good library reviewand 1 document review.
> I think it would be great to have 3 reviewsby Friday, tomorrow, 25-January-2024in order to form a solid review basis.
> 
> Please if interest and timeallow, review and submit review.
> Kind regards, Christopher

Christopher, please avoid top-posting. This is against posting rules to
this ML.

https://www.boost.org/community/policy.html#quoting

Also, I'm not sure if this is genuine typos, an issue with your email
client, or some broken HTML to plain text translation, but your posts
often miss spaces between words, which make them difficult to read.
In particular, in the initial post about starting the review this broke
links to library code and documentation and others.

Regardless of the source of the issue, please post only as plain text to
this list. Don't post as HTML.

>     On Tuesday, January 23, 2024 at 02:37:42 PM GMT+1, Christopher Kormanyos <e_float_at_[hidden]> wrote:  
>  
>   > The review of Boost.Charconv by Matt Borland runs> Monday, January 15th through January 25th, 2024.
> 
> Dear all,
> This post is intended to kindly encourage thesubmission of more reviews for the proposedBoost.Charconv.
> We've had thoughtful, deep discussion,
> but we really need a few more reviews to form
> a solid basis for results. The review periodis formally scheduled until 25-January-2024.
> 
> Your review can be brief, and handle someof the major queries. A terse review is probablybetter than an absent one.
> Thank you for considering the proposedBoost.Charconv.
> 
> Christopher
>     On Tuesday, January 23, 2024 at 08:43:36 AM GMT+1, Matt Borland via Boost <boost_at_[hidden]> wrote:  
>  
>  
> 
> 
> 
> 
> 
> On Monday, January 22nd, 2024 at 9:57 PM, Ruben Perez via Boost <boost_at_[hidden]> wrote:
> 
>>
> 
>>
> 
>> Hi all,
>>
> 
>> This is my review of the proposed Boost.Charconv.
>>
> 
> 
> Reuben,
> 
> Thanks for taking time to review.
> 
>>
> 
>> I’ve only had a quick glance. It surprised me finding an instance of
>> malloc/strtold, since these functions violate the “non-allocating,
>> locale-independent” guarantees. I understand this is for an extreme
>> edge case, and that the “locale-independent” guarantee is still
>> provided through internal logic. I don’t know enough about these
>> parsing algorithms to determine whether this is the right decision or
>> not. How much effort would require rolling a hand-rolled strtold
>> alternative for such edge cases? In any case, this can be done once
>> the library is in Boost.
>>
> 
> 
> The locale issue from last week is fixed on develop and master. strtold and strtof128 from scratch will be non-trivial additions to the library. The malloc could easily be dropped by rejecting strings in fallback over 1024 bytes, but that seems like the worse option of the two for now.
> 
> 
>> As much as I want to get rid of quickbook, my feeling is that
>> quickbook-based docs currently generate more usable output than
>> asciidoc-based ones. It may be a matter of tuning templates and
>> developing additional tools, but we’re not there yet.
>>
> 
> 
> I think Klemens has multi-page asciidoc libraries. I'll look at how he does that.
> 
> 
>> I recommend CONDITIONALLY ACCEPT Charconv into Boost, under the
>> condition that proper fuzzing is added to the test suite. I’ve worked
>> with libfuzzer before and can assist the author if required.
>>
> 
> 
> I will add the fuzzing, and I am sure I will ask questions as I have never used it before.
> 
> 
>> Minor comments on documentation:
>>
> 
>> - The first example uses to_chars(... sizeof(buffer) - 1), which seems
>> to imply a NULL-terminator somewhere. Since there is none, I think it
>> should just be sizeof(buffer).
>> - Docs don't specify the behavior about NULL-terminators. Either
>> specify "adheres to standard" or clarify that no NULL-terminator is
>> written.
>> - IMO the front page should clearly state that this library is
>> compliant with the charconv standard header, noting any exceptions.
>> - from_chars overview is missing semicolons in the friends section.
>> - Please use monospace in the discussion to refer to identifiers, like
>> [first, last).
>> - Basic usage: from_chars name is qualified but from_chars_result is not.
>> - Code snippets wrap, which makes them difficult to read.
>>
> 
> 
> See: https://github.com/cppalliance/charconv/issues/131
> 
> Matt
> 
> _______________________________________________
> Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
>     
> 
> _______________________________________________
> Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost