Skip to content

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

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.

Sign up for GitHub

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

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
6964c21
Utility::Debug: DebugOstreamFallback enable if has ostream operator
rune-scape Jan 24, 2020
bcc6544
Merge https://github.com/mosra/corrade into pr88
rune-scape Jan 27, 2020
dfa849b
Move DeclareLvalueReference to TypeTraits.h
rune-scape Jan 28, 2020
8b1e935
Utility::Debug: add HasDebugStreamOperator check to DebugOstreamFallback
rune-scape Jan 29, 2020
cf3f6fd
Utility::Debug: added std::ostream output operator call converter.
rune-scape Jan 29, 2020
8b048e6
Utility: doc++
rune-scape Jan 29, 2020
05cfb24
Utility::Debug: OstreamDebug::operator<< is now in its own namespace
rune-scape Feb 2, 2020
3b75172
Utility::Debug: more docs
rune-scape Feb 2, 2020
be5dfa9
Utility: snippet had narrowing conversion
rune-scape Feb 2, 2020
9abf7c3
Utility: DebugTest.cpp: corrected sourceLocation test
rune-scape Feb 2, 2020
b3129ed
Utility: whitespace at EOL.
mosra Feb 5, 2020
07dad62
Utility: this is how it is expected to work.
mosra Feb 5, 2020
ccb7bcd
Utility: eureka!
mosra Feb 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions doc/snippets/Utility.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,21 @@ else
/* [Debug-usage] */
}

{
auto sendMessage = [](const std::string &) {};
/* [Debug-ostream-delegation] */
using Utility::OstreamDebug::operator<<;
/// unfinished
Containers::Array<float> array{Containers::InPlaceInit, { 0.1f, 22.22f, 3.14f }};
std::cout << "array = " << array << std::endl;

std::ostringstream o;
o << array << std::endl;
sendMessage(o.str());
/// unfinished
/* [Debug-ostream-delegation] */
}

{
/* [Debug-scoped-output] */
std::ostringstream debugOut, errorOut;
Expand Down
11 changes: 11 additions & 0 deletions src/Corrade/Utility/Debug.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,17 @@ type using @ref Debug. Note that printing @ref std::vector or @ref std::map
containers is already possible with the generic iterable container support in
@ref Corrade/Utility/Debug.h.

@subsection Utility-Debug-stl-ostream-delegation Ostream Delegation

@ref Corrade/Utility/DebugStl.h also provides an @ref std::ostream @cpp
operator<<() @ce for printing builtin types:

@snippet Utility.cpp Debug-ostream-delegation

It must be brought into the current namespace by a using declaration:

@cpp using Corrade::Utility::OstreamDebug::operator<<; @ce

@section Utility-Debug-scoped-output Scoped output redirection

Output specified in class constructor is used for all instances created during
Expand Down
59 changes: 58 additions & 1 deletion src/Corrade/Utility/DebugStl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,46 @@ template<class ...Args> Debug& operator<<(Debug& debug, const std::tuple<Args...

namespace Implementation {

/* In order to ensure the most fitting operator<< overload gets called, we
can't simply check for *some* operator<< overload. As an example, for Array
there's both operator<<(ostream&, void*) and operator<<(Debug&, Iterable)
that can print it, but only the second one is desirable as the first prints
just a pointer. Directly checking for presence of either via

decltype(DeclareLvalueReference<std::ostream> << std::declval<T>()) and
decltype(DeclareLvalueReference<Debug> << std::declval<T>())

would yield "yes" in both cases, not telling us which one is better. Instead
we supply an object that's implicitly convertible to both as the first
argument, giving the overload resolution magic a chance to pick the better
fitting one. */
struct OstreamOrDebug {
/*implicit*/ operator std::ostream&();
/*implicit*/ operator Debug&();
};
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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? :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes! works perfectly. A very useful macro too


CORRADE_HAS_TYPE(
HasBestFittingOstreamOperator,
typename std::enable_if<std::is_same<
decltype(std::declval<OstreamOrDebug>() << std::declval<T>()),
std::add_lvalue_reference<std::ostream>::type
>::value>::type
);

CORRADE_HAS_TYPE(
HasBestFittingDebugOperator,
typename std::enable_if<std::is_same<
decltype(std::declval<OstreamOrDebug>() << std::declval<T>()),
std::add_lvalue_reference<Debug>::type
>::value>::type
);

/* Used by Debug::operator<<(Implementation::DebugOstreamFallback&&) */
struct DebugOstreamFallback {
template<class T> /*implicit*/ DebugOstreamFallback(const T& t): applier(&DebugOstreamFallback::applyImpl<T>), value(&t) {}
template<
class T,
typename = typename std::enable_if<HasBestFittingOstreamOperator<T>::value>::type
> /*implicit*/ DebugOstreamFallback(const T& t): applier(&DebugOstreamFallback::applyImpl<T>), value(&t) {}

void apply(std::ostream& s) const {
(this->*applier)(s);
Expand All @@ -128,6 +165,26 @@ struct DebugOstreamFallback {
CORRADE_UTILITY_EXPORT Debug& operator<<(Debug& debug, Implementation::DebugOstreamFallback&& value);
#endif

namespace OstreamDebug {

/**
@brief Print a builtin type to an ostream

Allows calls like @cpp std::cout << Magnum::Vector2{0.2, 3.14}; @ce to be
delegated to a @ref Debug object. Creates a @ref Debug object on every call.
*/
template<typename T>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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! 👍

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

typename std::enable_if<
Implementation::HasBestFittingDebugOperator<T>::value,
std::ostream
>::type &operator<<(std::ostream &os, const T &val) {
Corrade::Utility::Debug debug{&os, Corrade::Utility::Debug::Flag::NoNewlineAtTheEnd};
debug << val;
return os;
}

}

}}

#endif
76 changes: 72 additions & 4 deletions src/Corrade/Utility/Test/DebugTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
#include <vector>

#include "Corrade/TestSuite/Tester.h"
#include "Corrade/Utility/Debug.h"
#include "Corrade/Containers/Array.h"
#include "Corrade/Utility/DebugStl.h"

#ifndef CORRADE_TARGET_EMSCRIPTEN
Expand Down Expand Up @@ -82,6 +82,12 @@ struct DebugTest: TestSuite::Tester {
void ostreamFallback();
void ostreamFallbackPriority();

void ostreamDelegationInternalUsing();
void ostreamDelegationExternalUsing();
void ostreamDelegationCyclicDependency();
void ostreamDelegationPriority();
void ostreamDelegationPriorityImplicitConversion();

void scopedOutput();

void debugColor();
Expand Down Expand Up @@ -151,6 +157,12 @@ DebugTest::DebugTest() {
&DebugTest::ostreamFallback,
&DebugTest::ostreamFallbackPriority,

&DebugTest::ostreamDelegationInternalUsing,
&DebugTest::ostreamDelegationExternalUsing,
&DebugTest::ostreamDelegationCyclicDependency,
&DebugTest::ostreamDelegationPriority,
&DebugTest::ostreamDelegationPriorityImplicitConversion,

&DebugTest::scopedOutput,

&DebugTest::debugColor,
Expand Down Expand Up @@ -819,6 +831,62 @@ void DebugTest::ostreamFallbackPriority() {
CORRADE_COMPARE(out.str(), "baz from Debug\n");
}

namespace OstreamDelegationNamespace0 {
struct Corge {
int i;
};

inline Debug& operator<<(Debug& debug, const Corge& val) {
return debug << val.i << "corge from Debug";
}
}

void DebugTest::ostreamDelegationInternalUsing() {
using OstreamDebug::operator<<;
std::ostringstream out;
out << OstreamDelegationNamespace0::Corge{42};
CORRADE_COMPARE(out.str(), "42 corge from Debug");
}

namespace OstreamDelegationNamespace1 {
struct Grault {
int i;
};

inline Debug& operator<<(Debug& debug, const Grault& val) {
return debug << val.i << "grault from Debug";
}

using OstreamDebug::operator<<;
}

void DebugTest::ostreamDelegationExternalUsing() {
std::ostringstream out;
out << OstreamDelegationNamespace1::Grault{36};
CORRADE_COMPARE(out.str(), "36 grault from Debug");
}

struct ClassWithoutStreamOperator {};

void DebugTest::ostreamDelegationCyclicDependency() {
CORRADE_VERIFY(!Implementation::HasBestFittingOstreamOperator<ClassWithoutStreamOperator>::value);
CORRADE_VERIFY(!Implementation::HasBestFittingDebugOperator<ClassWithoutStreamOperator>::value);
}

void DebugTest::ostreamDelegationPriority() {
std::ostringstream out;
out << Baz{};
CORRADE_COMPARE(out.str(), "baz from ostream");
}

void DebugTest::ostreamDelegationPriorityImplicitConversion() {
using OstreamDebug::operator<<;
Containers::Array<int> array{Containers::InPlaceInit, { 1, 2, 3 }};
std::ostringstream out;
out << array;
CORRADE_COMPARE(out.str(), "{1, 2, 3}");
}

void DebugTest::scopedOutput() {
std::ostringstream debug1, debug2, warning1, warning2, error1, error2;

Expand Down Expand Up @@ -955,9 +1023,9 @@ void DebugTest::sourceLocation() {

#ifdef CORRADE_UTILITY_DEBUG_HAS_SOURCE_LOCATION
CORRADE_COMPARE(out.str(),
__FILE__ ":947: hello\n"
__FILE__ ":949: and this is from another line\n"
__FILE__ ":951\n"
__FILE__ ":1015: hello\n"
__FILE__ ":1017: and this is from another line\n"
__FILE__ ":1019\n"
"this no longer\n");
#else
CORRADE_COMPARE(out.str(),
Expand Down