[http::message] type requirement (concept) versus single template

[Vinícius dos Santos Oliveira]

Hey Vinnie, could you share that ideas about the future changes to beast::http::message again?

btw, how do you solve the problem of a `message` with/without the `version` field if you don't go the "type requirement"/concept route?

--

Vinícius dos Santos Oliveira

https://vinipsmaker.github.io/

[Vinnie Falco]

On Mon, May 2, 2016 at 6:08 AM, Vinícius dos Santos Oliveira
<vini.i...@gmail.com> wrote:
> how do you solve the problem of a `message` with/without the `version`
> field if you don't go the "type requirement"/concept route?

template<bool isRequest, class Body, class Headers>
struct message : ...
{
headers_type headers;
typename Body::value_type body;
};

template<bool isRequest, class Body, class Headers>
struct message_v1 : message<isRequest, Body, Headers>
{
int version;
};

[Vinícius dos Santos Oliveira]

2016-05-02 7:46 GMT-03:00 Vinnie Falco <vinnie...@gmail.com>:

template<bool isRequest, class Body, class Headers>
struct message_v1 : message<isRequest, Body, Headers>
{
    int version;
};

But this can be error-prone. message's destructor is not virtual.

[Vinnie Falco]

On Mon, May 2, 2016 at 6:48 AM, Vinícius dos Santos Oliveira
<vini.i...@gmail.com> wrote:
> But this can be error-prone. message's destructor is not virtual.

Destroying a `message_v1` through a pointer to just a `message` is not
a supported operation. And I don't think it should be. There is
precendet for this: `std::vector::~vector` is non virtual for example.

[Vinícius dos Santos Oliveira]

Is there any other place in the C++ libraries (or Boost) where this approach is chosen?

This doesn't look like a clean solution to me.

[Vinnie Falco]

On Mon, May 2, 2016 at 8:28 AM, Vinícius dos Santos Oliveira
<vini.i...@gmail.com> wrote:
> This doesn't look like a clean solution to me.

Can you please provide sample code showing why the proposed solution
is not clean?

[Vinícius dos Santos Oliveira]

Well, the reason why I argue it's not clean it's because this solution is easy-to-misuse.

The default behaviour for any class is that the user can delete an object to it through pointers and it's uncommon to libraries define derived classes with non-polymorphic destructors. Therefore, it's easy to assume the user will misuse such class.

I don't think it's a standard practice (outside implementation a detail namespace) too and that's why I ask if you can point me to any other C++ or Boost library that does this.

[Vinnie Falco]

On Monday, May 2, 2016 at 9:17:06 AM UTC-4, Vinícius dos Santos Oliveira wrote: 

The default behaviour for any class is that the user can delete an object to it through pointers

Agreed.

* You can delete a `message_v1` through a `message_v1 *`.
* You can delete a `message` through a `message *`

However,

* You cannot delete a `message_v1` through a `message *`

Its simply part of the type's contract that it does not support deleting through a pointer to the base class. If you think that deleting a `message_v1` through a pointer to a `message` is an important use case then I'd be more than happy to have that discussion with you (I don't think its an important use-case).

The declaration of `message` makes it very clear that the destructor is not virtual; its reasonable to assume that users of the library have read the documentation. The solution to the problem of class misuse is to make sure things are well documented, not to avoid the utility of language constructs.

 

it's uncommon to libraries define derived classes with non-polymorphic destructors.

* `std::enable_shared_from_this`: base class with no virtual destructor.

* `std::scoped_allocator_adapter`: base class with no virtual destructor
* `std::sub_match`: derived from `std::pair`, with no virtual destructor
* `std::iterator` and `std::unary_function`

There are plenty of examples, this is a common practice.

[Vinícius dos Santos Oliveira]

2016-05-02 11:44 GMT-03:00 Vinnie Falco <vinnie...@gmail.com>:

Its simply part of the type's contract that it does not support deleting through a pointer to the base class. If you think that deleting a `message_v1` through a pointer to a `message` is an important use case then I'd be more than happy to have that discussion with you (I don't think its an important use-case).

I don't think it's important because a use case.

I think it's important to avoid user mistakes. A huge code base and a new employer starting to work on the code base. If he receives a pointer to a a derived class and he know he is responsible to delete it, I hardly believe he'll do a static_cast first. The set of recomendations in the C++ community goes against your recomendation.

I don't think it's worth.

The declaration of `message` makes it very clear that the destructor is not virtual; its reasonable to assume that users of the library have read the documentation. The solution to the problem of class misuse is to make sure things are well documented, not to avoid the utility of language constructs.

 

it's uncommon to libraries define derived classes with non-polymorphic destructors.

* `std::enable_shared_from_this`: base class with no virtual destructor.

* `std::scoped_allocator_adapter`: base class with no virtual destructor
* `std::sub_match`: derived from `std::pair`, with no virtual destructor
* `std::iterator` and `std::unary_function`

There are plenty of examples, this is a common practice.

In these cases it's because it's not expected to have a pointer to the base class laying around.

I believe a "survey" against the larger Boost community could be beneficial.

[Vinnie Falco]

On Sun, Jun 12, 2016 at 10:15 AM, Vinícius dos Santos Oliveira
<vini.i...@gmail.com> wrote:
> I think it's important to avoid user mistakes.

Here I'm going to disagree. The destructors in question are clearly
marked non-virtual. I don't think its the programmer's job to mitigate
misuse arising from users who do not inspect the classes from which
they are deriving. The spectrum of such possible mistakes is
potentially infinite.

Neither std::pair nor std::vector have virtual destructors, or attempt
to mitigate the consequence of slicing a derived class with a
non-trivial destructor.

If you feel that such mitigation is more important to your users than
say, avoiding the cost of giving a class an unnecessary virtual table,
then of course you should take measures to protect those users. But I
hardly think that this should be a practice adopted generally, for the
reason that it goes against "pay for what you use" which is one of the
primary advantages of using C++ over other languages that try to
"help" the user by doing unnecessary things (for example, automatic
zero-initialization of data members in Java objects).