Stiffstream / restinio

Cross-platform, efficient, customizable, and robust asynchronous HTTP(S)/WebSocket server C++ library with the right balance between performance and ease of use
Other
1.15k stars 93 forks source link

Error compiling tls_socket_t under MSVC #23

Closed rcane closed 5 years ago

rcane commented 5 years ago

When trying to create a tls-enabled server the MSVC compiler complains about this:

...restinio/impl/tls_socket.hpp(93): error C3779: 'restinio::impl::tls_socket_t::lowest_layer': a function that returns 'auto' cannot be used before it is defined
...restinio/impl/tls_socket.hpp(60): note: see declaration of 'restinio::impl::tls_socket_t::lowest_layer'
...restinio/impl/tls_socket.hpp(93): note: This diagnostic occurred in the compiler generated function 'void restinio::impl::tls_socket_t::cancel(Args &&...)'
...

Simply not using lowest_layer() inside tls_socket_t's methods and instead replacing it with m_socket->lowest_layer() fixes the problem.

eao197 commented 5 years ago

Which version of MSVC do you use?

rcane commented 5 years ago

I am on Visual Studio 2017 version 15.9.11. So its not that old.

eao197 commented 5 years ago

It's strange but I can't reproduce that error on VC++ 15.9.11 (Version 19.16.27030.1 for x64) on our tests/examples.

Can you show the code on that you get that error?

rcane commented 5 years ago

This is the bare minimum code that shows the error here:

#include "restinio/all.hpp"
#include "restinio/tls.hpp"

#include "boost/asio.hpp"

void test()
{
  using tls_server_t = restinio::http_server_t<restinio::default_tls_traits_t>;
  using settings_t = restinio::server_settings_t<restinio::default_tls_traits_t>;

  boost::asio::ssl::context sslContext(boost::asio::ssl::context::sslv23);

  boost::asio::io_context io_context;

  auto server = std::make_unique<tls_server_t>(
    io_context,
    settings_t{}
      .tls_context(std::move(sslContext))
    );
}
eao197 commented 5 years ago

Thanks, I'll check it.

eao197 commented 5 years ago

I've tried this version of your code with Boost-1.67.0:

#include "restinio/all.hpp"
#include "restinio/tls.hpp"

#include "boost/asio.hpp"

void test()
{
  using tls_server_t = restinio::http_server_t<restinio::default_tls_traits_t>;
  using settings_t = restinio::server_settings_t<restinio::default_tls_traits_t>;

  boost::asio::ssl::context sslContext(boost::asio::ssl::context::sslv23);

  boost::asio::io_context io_context;

  auto server = std::make_unique<tls_server_t>(
    restinio::external_io_context(io_context),
    settings_t{}
      .tls_context(std::move(sslContext))
    );
}

It compiles without errors with VC++ 15.9.11

rcane commented 5 years ago

I am using boost-1.69 here.

Interestingly, when I apply my fix to the RESTinio code this test code produces other compile errors:

error C2664: 'restinio::http_server_t<restinio::default_tls_traits_t>::http_server_t(const restinio::http_server_t<restinio::default_tls_traits_t> &)': cannot convert argument 1 from 'boost::asio::io_context' to 'restinio::io_context_holder_t'

This looks like an include order problem. My actual code does compile successfully but includes more things than this test.

eao197 commented 5 years ago

Interestingly, when I apply my fix to the RESTinio code this test code produces other compile errors:

Because of that error, I changed your code:

  auto server = std::make_unique<tls_server_t>(
    io_context,
    settings_t{}
      .tls_context(std::move(sslContext))
    );

that way:

  auto server = std::make_unique<tls_server_t>(
    restinio::external_io_context(io_context),
    settings_t{}
      .tls_context(std::move(sslContext))
    );

It is because http_server_t's constructor expects io_context_holder_t, but not io_context directly.

eao197 commented 5 years ago

I am using boost-1.69 here.

I checked my modification of your example with boost-1.69 and there is no compilation error.

rcane commented 5 years ago

Then I am out of ideas. I am not doing anything special. I tried including some additional headers from asio and shuffling them around to see if that fixes the problem but without luck. The next thing would be to analyze the RESTinio and asio headers to see what gets included where and how this could trigger this error. But I am not really keen on doing that. ;-)

eao197 commented 5 years ago

Which kind of Visual Studio do you use? Community, Professional or something else?

rcane commented 5 years ago

I am using the Community Edition right now. Although I cannot think of a reason why this should make a difference since the compiler should be the same for all editions. But who knows what MS really does there. ;-)

eao197 commented 5 years ago

I'm using Community edition too.

It is a very strange error because it is declared and defined just before the first usage of it in restinio::impl::tls_socket_t. So I don't understand why VC++ complains about that error at all.

rcane commented 5 years ago

I found the reason for the error.

I am using /permissive- compiler option. It is not impossible that this is simply a false positive.

eao197 commented 5 years ago

Thanks. I've fixed the code.

It is not impossible that this is simply a false positive.

Various versions of GCC and clang compile that code cleanly. So it looks like premature pessimization from VC++.

rcane commented 5 years ago

This is fixed with in the current version.