boostorg / hana

Your standard library for metaprogramming
http://boostorg.github.io/hana
Boost Software License 1.0
1.69k stars 215 forks source link

Ill-formed equality for std::optional<hana::tuple<int>> #470

Closed tzlaine closed 4 years ago

tzlaine commented 4 years ago

hana/detail/operators/comparable.hpp contains this:

        template <typename X, typename Y, typename = typename std::enable_if<
            detail::comparable_operators<typename hana::tag_of<X>::type>::value ||
            detail::comparable_operators<typename hana::tag_of<Y>::type>::value
        >::type>
        constexpr auto operator==(X&& x, Y&& y)
        { return hana::equal(static_cast<X&&>(x), static_cast<Y&&>(y)); }

The problem is that hana::tag_of is idempotent, and so operator==(hana::tuple<int>(0), hana::tuple_tag{}) remains a viable overload -- the SFINAE constraint happily considers hana::tuple_tag to be a sequence of some sort -- but then blows up inside hana::equal(). Within hana::equal(), the expression hana::length(hana::tuple_tag{}) appears, and is ill-formed.

This will all happen any time you do x == y, where x and y are each a hana::tuple in a std::optional. Minimal repro:

#include <boost/hana.hpp>
#include <optional>
#include <cassert>

int main() {
    std::optional<boost::hana::tuple<int>> attr;
    auto const copy = attr;
    assert(attr == copy);  // <-- Kablooey!
}

Best of luck. This is real thorny. I was able to fix it locally, but my fix broke all kinds of stuff not in the immediate path of my changes -- so, no real fix at all.

ricejasonf commented 4 years ago

Your example compiles fine with -stdlib=libc++. I think the explosion happens with comparing T and optional<T> which the libstdc++ version must be using internally.

#include <boost/hana.hpp>
#include <optional>
#include <cassert>

int main() {
    boost::hana::tuple<int> x{};
    std::optional<boost::hana::tuple<int>> attr;
    assert(attr == x);  // <-- Kablooey!

https://godbolt.org/z/95fP4b

<source>:8:17: note: in instantiation of function template specialization 'boost::hana::detail::operators::operator==<std::__1::optional<boost::hana::tuple<int> > &, boost::hana::tuple<int> &, void>' requested here

    assert(attr == x);  // <-- Kablooey!

boost::hana::detail::operators::operator==<std::__1::optional<boost::hana::tuple<int> > &, boost::hana::tuple<int> &, void> is being called right off the bat.

Perhaps that operator should fail if the underlying hana::equal call should fail. I suggested a similar solution to the same issue with Struct equality in https://github.com/boostorg/hana/issues/460

tzlaine commented 4 years ago

Right! Should have mentioned that this was with GCC/libstdc++. I agree with the approach for the fix, but was unable to figure out how to do so -- there's a lot of code around there that is highly interdependent.

ricejasonf commented 4 years ago

Yeah, the tag_of thing really is the issue.

How about this? https://github.com/boostorg/hana/pull/472

I made those operators not consider types that are the same as their tag type since that is never the case in the Boost.Hana types that use them. (I also fixed the Struct thing)

tzlaine commented 4 years ago

Yes! That fixed about a million errors I had in some crazy, autogenerated test cases. I peed a little from all the excitement. Thanks, Jason.

ldionne commented 4 years ago

Fixed by 4f5157bc425eac8e115d597142932fef2e535282, thanks Jason.