Skip to content

Conversation

@sbchisholm
Copy link
Contributor

@sbchisholm sbchisholm commented May 12, 2020

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 395830c

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
Copy link

codecov bot commented May 12, 2020

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
Copy link
Owner

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

Thank you for your contribution!

@basiliscos basiliscos merged commit aa1553f into basiliscos:master May 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants