Utility::Debug: add std::ostream output operator call converter

[rune-scape]

Compile errors are much clearer for types without a stream operator, and avoids being an actual catch-all.

This is a bit of a selfish pr, as my code to translate Debug operator<<s to std::ostream operator<<s is bugged when there is a catchall for Debug stream operator<<s.
Both stream fallbacks use each other. This results in being able to compile cout << UnprintableClass{}.
At runtime it gets stuck in recursion, flipflopping between both operator<<s. I thought that was pretty interesting.

Please tell me if I made any formatting mistakes.

[codecov-io]

Codecov Report

Merging #88 into master will increase coverage by 2.57%.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           master    #88      +/-   ##
========================================
+ Coverage   97.42%   100%   +2.57%     
========================================
  Files          92      1      -91     
  Lines        6412    336    -6076     
========================================
- Hits         6247    336    -5911     
+ Misses        165      0     -165

Impacted Files	Coverage Δ
src/Corrade/TestSuite/Tester.h
src/Corrade/Utility/System.cpp
src/Corrade/Utility/String.h
src/Corrade/PluginManager/PluginMetadata.h
src/Corrade/Utility/Format.cpp
src/Corrade/Containers/ScopeGuard.h
src/Corrade/Containers/Tags.h
src/Corrade/TestSuite/Compare/Container.h
src/Corrade/TestSuite/Compare/SortedContainer.h
src/Corrade/Utility/TweakableParser.cpp
... and 79 more

---

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 6964c21...ccb7bcd. Read the comment docs.

[mosra]

The extremely huge compiler errors on a mismatched operator<< were a pain, so thank you for making everyone's life better :)

This is a bit of a selfish pr

Why? :) When I saw the notification, I actually thought you'd be adding the inverse operator here -- making Debug-printable types work with std::ostream. I'm happy to accept that, you'd be definitely not the only one using it.

[mosra]

There's a (admittedly weirdly named) CORRADE_HAS_TYPE(className, typeExpression) macro that already contains the above functionality, but your variant seems to be a lot simpler than what I stole from somewhere back in the day :) If I fix the macro to have the second parameter variadic (so the comma doesn't break it), I think we could then reduce the above to be just this (containing the potentially hard-to-understand template magic in a single place):

CORRADE_HAS_TYPE(HasOstreamOperator, typename std::is_same<
    decltype(declareLvalueReference<std::ostream>() << std::declval<C>()),
    std::add_lvalue_reference<std::ostream>::type
>::type)

(Also adapting the naming to what Corrade uses for type traits.) I hope I didn't overlook something that would make this impossible tho.

[mosra]

CORRADE_HAS_TYPE() is fixed to allow this in 8d4eb5b, and I think it would work with a minor change:

CORRADE_HAS_TYPE(HasOstreamOperator, typename std::enable_if<std::is_same<
    decltype(declareLvalueReference<std::ostream>() << std::declval<C>()),
    std::add_lvalue_reference<std::ostream>::type
>::value>::type)

I assume you have some way to test the infinite recursion already -- can you confirm? :)

[rune-scape]

yes! works perfectly. A very useful macro too

[rune-scape]

I figured I should separate this from the feature adding one, but if you want I can add it in.

[mosra]

Adding the inverse operator would have a good side effect of testing that the above SFINAE restriction actually works and there's no infinite recursion anymore (while this alone doesn't check for that, and so I'm afraid of breaking it by accident).

So, by all means, add it in :)

[rune-scape]

The extra checks are to avoid ambiguous calls

[mosra]

This is great, the diff is so nicely small and tidy that at first I thought I was looking at the previous state with the operator<< not added yet :)

[mosra]

Minor: I'm usually not putting a newline after template<..>, keeping the whole signature a single line. (Or generally not adding a newline unless really needed to keep the code vertically terse.)

[mosra]

One last thing: can you

• add a short documentation block for this new function,
• mention this in the class documentation so people get to know it exists -- there's a "Printing STL types" section, so maybe extending that a bit ("Printing STL types, interaction with iostreams"?)
• and add a test in Test/DebugTest.cpp? There's ostreamFallback and ostreamFallbackPriority, so something similar (ostreamDelegate?) and also similarly checking the priority works for the inverse case, picking the ostream operator over the Debug one -- btw., don't forget to list the two new methods in the addTests() so these get properly called

Thank you! 👍

[rune-scape]

yes! wow ostreamDelegate is way better than any name I considered

[rune-scape]

a problem:
implicit conversions take priority over the operator<<(only because HasOstreamOperator only checks for any possible ostream print operator), meaning Container::Array get printed as a void *

a fix is making HasOstreamOperatorExact and HasOstreamMemberOperatorExact, and checking like 4 versions of each... but like.. no

[mosra]

@bowling-allie finally today I got a totally random idea and it seems it works -- or at least made the test pass. Full details in the 3 new commits I just pushed to your branch (I hope that's okay, hehe).

Can you check with your own code if everything is still behaving as expected and no circular recursion or anything happens again?

EDIT: argh, Clang and MSVC doesn't like this ... ambiguous overload :/