Re: [boost] [review][beast] Review of Beast starts today : July 1 - July 10

Subject: Re: [boost] [review][beast] Review of Beast starts today : July 1 - July 10
From: Vinnie Falco (vinnie.falco_at_[hidden])
Date: 2017-07-09 16:55:22

On Sun, Jul 9, 2017 at 9:38 AM, Daniela Engert via Boost
<boost_at_[hidden]> wrote:
> That said, I agree on the opinion which other reviewers already stated
> on that it lacks some convenience helper functions when it comes to
> draining or construction of requests. I'd love to see those added to the
> library.

Yeah this message is coming through loud and clear.. LOL...

I will make adjustments to the scope of Beast.

>  * Beast's zlib emits a few harmless but stupid warnings C4127
> "conditional expression is constant" (Microsoft, get rid of them!) and
> quite some narrowing warnings C4244. These need some cleansing before we
> can use Beast in our team because of our /W4 /WX policy.

It should be "real" zlib not Beast zlib which is emitting those
warnings. There are two versions of zlib in Beast:

1. The one in <beast/zlib/*> is a header-only port of zlib to
modern-ish C++11. There is a warning "‘*((void*)& old_flush +4)’ may
be used uninitialized in this function [-Wmaybe-uninitialized]".

2. The original zlib distribution in test/zlib/zlib-1.2.8. This is
used for the tests, to compare Beast's output with zlib's output, and
to ensure that output produced by zlib can be consumed by Beast and
vice-versa. I am reluctant to change the source code for zlib because
I'd like it to perfectly track the upstream repository (it was brought
in using git-subtree).

For #1 The variable is not used uninitialized, I believe it is a false
positive cause by the implementation of boost::optional. I have spent
a lot of time trying to fix this, to no avail - if someone smarter
than me can offer a patch I would be grateful.

For #2 the warning should not affect users, since it is limited to the tests.

>  * Beast's test-suite uses the deprecated Boost.Coroutine library which
> leads to deprecation warnings

Beast's tests use Boost.Asio coroutines (to make sure they work with
Beast). It is Asio that uses the deprecated Boost.Coroutine. Asio
needs to be updated to work with Coroutine2. I can't do much about
this other than not use coroutines in the tests, which would yield
less coverage.

>  * there is an error in the Jamfile of the test-suite which creates a
> truncated compiler options that msvc complains about (this is already
> rectified in a later commit, though)

This is a weird issue in bjam would should be fixed in Boost 1.66.0.

> I didn't implement the actual download so
> far because Beast's file_body is not part of the review branch.

`file_body` is now a public interface as of version 74 (no more "out
of scope" hehe)

Thanks