Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fails to compile with operator<< available for equality test types in Clang, but not GCC #1403

Closed
thecppzoo opened this issue Oct 9, 2018 · 4 comments

Comments

@thecppzoo
Copy link
Contributor

Given a valid overload of operator<< as in this code:

#include <array>
#include <iostream>

template<typename T, std::size_t L>
std::ostream &operator<<(std::ostream &out, const std::array<T, L> &a) {
    return out << "[array]";
}

#include <catch.hpp>

TEST_CASE("fails to compile", "[bummer]") {
    std::array<int, 1> a1{1}, a2{2};
    if(a1 == a2) {
        std::cout << a1; // proves the operator<< is fine for a1
    }
    REQUIRE(a1 == a2);
}

Fails to compile with recent versions of Clang:

bug.cpp:16:16: note: in instantiation of member function
      'Catch::BinaryExpr<const std::__1::array<int, 1> &, const
      std::__1::array<int, 1> &>::streamReconstructedExpression' requested here
    REQUIRE(a1 == a2);
               ^
bug.cpp:5:15: note: 'operator<<' should be declared prior to the call site
std::ostream &operator<<(std::ostream &out, const std::array<T, L> &a) {

It can be reproduced in the compiler explorer:
https://gcc.godbolt.org/z/Wkwab4

Originally I was using Clang 7.0.0 on Mac Os X

@thecppzoo
Copy link
Contributor Author

I suspect the bug has to do with Clang being more strict about violations of using an overload in a template before the overload has been declared; that is, inside Catch2 there is a template that uses a not-yet-declared Catch2 function overload dependent on user types; the declaration happens afterwards, GCC incorrectly accepts it, but Catch2 correctly does not.

@thecppzoo
Copy link
Contributor Author

Guys, I think you may have a very serious error, here is why:
If you do this:

#include <internal/catch_stream.h>
#include <catch.hpp>

The problem goes away. That means:

  1. The semantics of your code is dependent on inclusion order!
  2. In particular operator<< for ReusableStringStream is affected.

I think your using ::operator<< in catch.hpp is a smell. Why do you need it? it seems you are putting global << into namespace Catch, this alters ADL (Argument Dependent Lookup of overloads, aka Köenig Lookup). The bug I reported is related to that, since if you don't do the using at least in Clang everything compiles.

It seems you are doing some SFINAE on operator<< before you have declared ReusableStringStream::operator<< and this causes a side effect, which might be a Clang bug that hides the global operator<<(std::ostream &, <some std type here>) needed at

        template<typename T>
        auto operator << ( T const& value ) -> ReusableStringStream& {
            *m_oss << value;
            return *this;
        }

@thecppzoo
Copy link
Contributor Author

Minimal example, at the compiler explorer:

#include <array>
#include <iosfwd>

template<typename T, std::size_t L>
std::ostream &operator<<(std::ostream &, const std::array<T, L> &);

// Inclusion of internal/catch_stream.h ahead is a fix
// #include <internal/catch_stream.h>
#include <catch.hpp>

namespace Catch {
void triggerBug(ReusableStringStream &rss, std::array<int, 1> a) {
    rss << a;
}
}

@thecppzoo
Copy link
Contributor Author

thecppzoo commented Oct 10, 2018

I spent a bunch of time figuring out this issue, here's what I got:

#include <array>
#include <iosfwd>

// non-template
std::ostream &operator<<(std::ostream &, const std::array<int, 1> &);

template<std::size_t L>
std::ostream &operator<<(std::ostream &, const std::array<long, L> &);

// resembles #include <internal/catch_common.h>
namespace Catch {
    struct SourceLineInfo;
    // introduces an overload of operator<< into namespace Catch
    void operator<<(std::ostream &, SourceLineInfo const &);
}

// resembles #include <internal/catch_stream.h>
namespace Catch {
    struct HasOperator {
        std::ostream *m_;
    public:
        template<typename T>
        void operator<<(const T &v) {
            *m_ << v;
        }
    };
}

// done in #include <internal/catch_tostring.h>
namespace Catch {
    using ::operator<<;
}

namespace Catch {
    void usesGlobalNonTemplateOperator(HasOperator &rss, std::array<int, 1> a) {
        rss.operator<<(a);
    }

    void usesGlobalTemplate(HasOperator &rss, std::array<long, 1> a) {
        rss.operator<<(a);
    }
}

void ok(std::ostream &o, std::array<long, 1> a) {
    o << a;
}

The introduction of the overload of << for SourceLineInfo "blocks" unqualified lookup to reach for the global template <<.

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

No branches or pull requests

1 participant