basiliscos / cpp-bredis

Boost::ASIO low-level redis client (connector)
MIT License
147 stars 36 forks source link

clang >=8 w/C++17 detects trying to access protected dtor #41

Closed sbchisholm closed 4 years ago

sbchisholm commented 4 years ago

We encountered an issue when compiling with clang-tidy >= 8.0 and C++17, constructing the boost::static_visitor, in this case count_unwrapper_t, with the initializer-list, the compiler complains about trying to access a protected destructor:

/usr/local/include/bredis/impl/protocol.ipp:292:52: error: temporary of type 'boost::static_visitor<count_variant_t<buffers_iterator<const_buffer
s_1, char>, drop_result> >' (aka 'static_visitor<variant<bredis::details::count_value_t, variant<bredis::not_enough_data_t, positive_parse_result
_t<boost::asio::buffers_iterator<boost::asio::const_buffers_1, char>, bredis::parsing_policy::drop_result>, bredis::protocol_error_t> > >') has p
rotected destructor [clang-diagnostic-error]
            boost::apply_visitor(count_unwrapper_t{}, count_result);
                                                   ^
/usr/local/include/bredis/impl/protocol.ipp:386:24: note: in instantiation of member function 'bredis::details::bulk_string_parser_t<boost::asio:
:buffers_iterator<boost::asio::const_buffers_1, char>, bredis::parsing_policy::drop_result>::apply' requested here
        return Parser::apply(next_from, to_, 1);
                       ^
...
/usr/local/include/boost/variant/static_visitor.hpp:53:5: note: declared protected here
    ~static_visitor() = default;

gcc 10.1, boost 1.69, clang 10.0, bredis 395830c1b68d47ff54b93724ac22e2255765015d

The following explanation describes how {} vs () differs in how each of them access ctor's and in our case dtor's of the parent classes: https://stackoverflow.com/questions/56367480/should-this-code-fail-to-compile-in-c17/56367566#56367566

codecov[bot] commented 4 years ago

Codecov Report

Merging #41 into master will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #41   +/-   ##
=======================================
  Coverage   93.79%   93.79%           
=======================================
  Files          11       11           
  Lines         451      451           
=======================================
  Hits          423      423           
  Misses         28       28           
Impacted Files Coverage Δ
include/bredis/impl/protocol.ipp 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 395830c...5d33ee5. Read the comment docs.

basiliscos commented 4 years ago

I wasn't able to reproduce it, but it seems your explanation is correct. I'll merge the PR.

Thank you for your contribution!