gabime / spdlog

Fast C++ logging library.
Other
24.21k stars 4.53k forks source link

User defined types no longer streamable #39

Closed sohailsomani closed 9 years ago

sohailsomani commented 9 years ago

Not sure when this changed but my custom types are no longer streamable to the log. Is there an acceptable alternative?

gabime commented 9 years ago

Could you post a complete example ?

sohailsomani commented 9 years ago

Sure. This used to work:

#include "boost/date_time/posix_time/posix_time_types.hpp"
#include "spdlog/spdlog.h"

int main() {
    namespace spd = spdlog;
    namespace bpt = boost::posix_time;
    auto console = spd::stdout_logger_mt("console");
    console->info() << bpt::microsec_clock::universal_time();
}

Here is the error message (truncated):

vendor\spdlog\include\spdlog\details\./line_logger.h(99) : error C2678: binary '<<' : no operator found which takes a left-hand operand of type 'spdlog::details::fmt::MemoryWriter' (or there is no acceptable conversion)
...
 ..\..\app\lib\common\util\DateTime.cpp(58) : see reference to function template instantiation 'spdlog::details::line_logger &spdlog::details::line_logger::operator <<<time_type>(const T &)' being compiled
        with
        [
            time_type=boost::posix_time::ptime
,            T=boost::posix_time::ptime
        ]
sohailsomani commented 9 years ago

I also have a similar problem for my own types which are streamable to ostream. Adding overloads helps the compilation, but not sure if it is the right way to go:

  inline
  spdlog::details::fmt::Writer&
  operator<<(spdlog::details::fmt::Writer & writer, String const & s) {
    return writer << s.as_utf8_string();
  }
gabime commented 9 years ago

One way around this is for example: console->info("{}", bpt::microsec_clock::universal_time());

sohailsomani commented 9 years ago

For sure. But if you support the insertion operator, it stands to reason that I should be able to stream my own types via some overlading. You might want to hoist Writer out of the details::fmt namespace and make it a proper part of the API. For now, I had to revert back to an old version of spdlog since I don't have time to change all the code.

gabime commented 9 years ago

I committed a fix - 56ee7316e92a250eba2288061708b73bce310e1a. Please feedback if that indeed solves the problem..

sohailsomani commented 9 years ago

Not sure if related, but I have this new problem (that didn't happen before):

vendor\spdlog\include\spdlog\details\./format.h(1877) : error C2248: 'spdlog::details::fmt::internal::MakeValue<char>::MakeValue' : cannot access private member declared in class 'spdlog::details::fmt::internal::MakeValue<char>'
sohailsomani commented 9 years ago

I think the culprit is here:

    // The following two methods are private to disallow formatting of
    // arbitrary pointers. If you want to output a pointer cast it to
    // "void *" or "const void *". In particular, this forbids formatting
    // of "[const] volatile char *" which is printed as bool by iostreams.
    // Do not implement!
    template <typename T>
    MakeValue(const T *value);
    template <typename T>
    MakeValue(T *value);

So the problem might be fixed. I'll keep compiling and see.

Edit: TBH, I'm not sure why you wouldn't just cast the pointer yourself since there is nothing else the user would do.

sohailsomani commented 9 years ago

Looks like everything compiled and runs as expected. Thanks for the quick fix.

gabime commented 9 years ago

Great. Thanks for reporting

lzuwei commented 9 years ago

I am encountering some issues as well for custom types. I have overloaded ostream but spdlog is complaining. Any ideas what else I need to implement.

error: invalid operands to binary expression ('std::basic_ostringstream<char>' and 'const iq::message::v1::InsertRequest')
    os << value;
    ~~ ^  ~~~~~
/Users/zu/workspace/iq-vision2/thirdparty/spdlog/include/spdlog/sinks/../details/format.h:951:9: note: in instantiation of function template specialization 'fmt::format<char, iq::message::v1::InsertRequest>' requested here
        format(*static_cast<BasicFormatter<Char>*>(formatter),
        ^
/Users/zu/workspace/iq-vision2/thirdparty/spdlog/include/spdlog/sinks/../details/format.h:1043:26: note: in instantiation of function template specialization 'fmt::internal::MakeValue<char>::format_custom_arg<iq::message::v1::InsertRequest>' requested here
        custom.format = &format_custom_arg<T>;
                         ^
/Users/zu/workspace/iq-vision2/thirdparty/spdlog/include/spdlog/sinks/../details/format.h:1995:5: note: in instantiation of function template specialization 'fmt::internal::MakeValue<char>::MakeValue<iq::message::v1::InsertRequest>' requested here
    FMT_VARIADIC_VOID(write, BasicStringRef<Char>)
    ^
/Users/zu/workspace/iq-vision2/thirdparty/spdlog/include/spdlog/sinks/../details/format.h:1670:7: note: expanded from macro 'FMT_VARIADIC_VOID'
      fmt::internal::MakeValue<Char>(args)... \

The definition of the InsertRequest is roughly this.


struct Request {
    Request()
            : version(1),
              type(UNKNOWN),
              id(-1){
    }
    Request(RequestId i, ClientRequestType t)
            :version(1),
             type(t),
             id(i){
    }
    VersionId version;
    ClientRequestType type;
    RequestId id;
};

struct InsertRequest: Request {
    InsertRequest()
            : Request(id, INSERT) {

    }
    std::vector<std::string> image_ids;
    std::vector<std::string> images;
};

//overload output operators
std::ostream& operator<< (std::ostream& o, const iq::message::ClientRequestHeader& h);
std::ostream& operator<< (std::ostream& o, const iq::message::v1::SearchRequest& r);
gabime commented 9 years ago

You need to implement the operator<< for the InsertRequest class. Something like

std::ostream& operator<< (std::ostream& o, const InsertRequest& h)
{

//You code here. Something like
//o << "Version: " << h.version <<...;
}
lzuwei commented 9 years ago

Hi Gabime,

Thanks for the spot, I missed out on that!

jkhoogland commented 9 years ago

Hi Gabime,

first thanks for writing this very useful library.

I tried to stream some std::vector to a logger, but get compiler errors.

In file included from /Users/Jiri/prj/bb/spdlog/sinks/../details/./log_msg.h:29:
/Users/Jiri/prj/bb/spdlog/sinks/../details/format.h:2341:6: error: invalid operands to binary expression ('std::basic_ostringstream<char>'
      and 'const std::__1::vector<unsigned long, std::__1::allocator<unsigned long> >')
  os << value;

When streaming to std::cout, no issues, ie the operator<< works fine it seems. Code is added below.

I'm not sure what I'm missing... Any suggestions would be much appreciated, Jiri

System: $ uname -a Darwin JiriMBP.home 14.4.0 Darwin Kernel Version 14.4.0: Thu May 28 11:35:04 PDT 2015; root:xnu-2782.30.5~1/RELEASE_X86_64 x86_64 i386 MacBookPro11,2 Darwin $ /usr/bin/c++ --version Apple LLVM version 6.1.0 (clang-602.0.53) (based on LLVM 3.6.0svn) Target: x86_64-apple-darwin14.4.0 Thread model: posix

template <typename T>
std::ostream& operator<<( std::ostream& os, std::vector<T>& v )
{
    auto it = std::ostream_iterator<T>( os, " " );
    std::copy( std::begin( v ), std::end( v ), it );
    return os;
}

TEST( Logging, STL )
{
    size_t q_size = 1048576;
    spdlog::set_async_mode( q_size );
    auto logger = spdlog::stdout_logger_mt( "Log.STL" );

    logger->info( "info logged." );
    logger->info( "info logged. {}", 1 );
    std::vector<size_t> v{ 1, 2, 3, 4 };
    std::cout << v << std::endl;
    logger->info( "info logged. {}" ) << v;
    logger->info( "info logged. " ) << v;
}
vitaut commented 9 years ago

The problem is that const is missing in the declaration of operator<<, so it is not considered. Should be:

template <typename T>
std::ostream& operator<<( std::ostream& os, const std::vector<T>& v )
                                            ^^^^^
jkhoogland commented 9 years ago

why would it not recognize it if the streaming operator if it accepts a non const vector ? even with the const added, it still barfs.

the error message is suggesting it is still not picking up the instantiation of operator<< for std::vector even though the (bloody) definition is definition is right before it ...

In file included from /Users/Jiri/prj/bb/test/Core/testLogging.cpp:18:
In file included from /Users/Jiri/prj/bb/src/Core/Logging.h:230:
In file included from /Users/Jiri/prj/bb/src/Core/Logging/LoggingSpdLog.h:15:
In file included from /Users/Jiri/prj/bb/src/Core/Logging/spdlog/spdlog.h:33:
In file included from /Users/Jiri/prj/bb/src/Core/Logging/spdlog/sinks/../details/../logger.h:36:
In file included from /Users/Jiri/prj/bb/src/Core/Logging/spdlog/sinks/./base_sink.h:35:
In file included from /Users/Jiri/prj/bb/src/Core/Logging/spdlog/sinks/../details/../sinks/sink.h:27:
In file included from /Users/Jiri/prj/bb/src/Core/Logging/spdlog/sinks/../details/./log_msg.h:29:
/Users/Jiri/prj/bb/src/Core/Logging/spdlog/sinks/../details/format.h:2341:6: error: call to function 'operator<<' that is neither visible in the
      template definition nor found by argument-dependent lookup
  os << value;
     ^
/Users/Jiri/prj/bb/src/Core/Logging/spdlog/sinks/../details/format.h:857:5: note: in instantiation of function template specialization
      'fmt::format<char, std::__1::vector<unsigned long, std::__1::allocator<unsigned long> > >' requested here
    format(*static_cast<BasicFormatter<Char>*>(formatter),
    ^
/Users/Jiri/prj/bb/src/Core/Logging/spdlog/sinks/../details/format.h:941:22: note: in instantiation of function template specialization
      'fmt::internal::MakeValue<char>::format_custom_arg<std::__1::vector<unsigned long, std::__1::allocator<unsigned long> > >' requested here
    custom.format = &format_custom_arg<T>;
                     ^
/Users/Jiri/prj/bb/src/Core/Logging/spdlog/sinks/../details/format.h:1804:3: note: in instantiation of function template specialization
      'fmt::internal::MakeValue<char>::MakeValue<std::__1::vector<unsigned long, std::__1::allocator<unsigned long> > >' requested here
  FMT_VARIADIC_VOID(write, BasicStringRef<Char>)
  ^
/Users/Jiri/prj/bb/src/Core/Logging/spdlog/sinks/../details/format.h:1503:7: note: expanded from macro 'FMT_VARIADIC_VOID'
      internal::MakeValue<Char>(args)... \
      ^
/Users/Jiri/prj/bb/src/Core/Logging/spdlog/sinks/../details/./line_logger.h:199:26: note: in instantiation of function template specialization
      'fmt::BasicWriter<char>::write<std::__1::vector<unsigned long, std::__1::allocator<unsigned long> > >' requested here
            _log_msg.raw.write("{}", what);
                         ^
/Users/Jiri/prj/bb/test/Core/testLogging.cpp:130:40: note: in instantiation of function template specialization
      'spdlog::details::line_logger::operator<<<std::__1::vector<unsigned long, std::__1::allocator<unsigned long> > >' requested here
    JMT_LOG( INFO ) << "info logged. " << v;
                                       ^
/Users/Jiri/prj/bb/test/Core/testLogging.cpp:115:15: note: 'operator<<' should be declared prior to the call site
std::ostream& operator<<( std::ostream& os, const std::vector<T>& v )
              ^
1 error generated.
/
vitaut commented 9 years ago

why would it not recognize it if the streaming operator if it accepts a non const vector ?

Technically it is possible to recognize an overload taking non-const reference, but in this particular case I think it's better to make operator<< const-correct.

The error

call to function 'operator<<' that is neither visible in the template definition nor found by argument-dependent lookup

simply means that your operator<< is not visible at the call site according to the C++ lookup rules. It is best illustrated with the following example:

#include <vector>
#include <iostream>
#include <iterator>

template <typename T>
void f(const T &v) {
  std::cout << v; // operator<< is not visible here because it is defined after f and it is not found by ADL
}

template <typename T>
std::ostream& operator<<( std::ostream& os, const std::vector<T>& v )
{
    auto it = std::ostream_iterator<T>( os, " " );
    std::copy( std::begin( v ), std::end( v ), it );
    return os;
}

int main() {
    std::vector<size_t> v{ 1, 2, 3, 4 };
    f(v);
}

To fix it either make sure that operator<< is defined before including spdlog.h or define it in namespace std for argument-dependent lookup to work.

jkhoogland commented 9 years ago

Hi Victor, thanks for taking the time for your explanation.

I managed to get the stuff to compile, by putting the declaration before the including of spdlog.h. It still surprises me that the test I wrote (declared in std namespace ) didn't pick up the operator<< defined exactly in front it... I ll study the lookup rules further... Regards Jiri

KingDuckZ commented 7 years ago

I'm having problem with boost::string_ref:

boost::string_ref str_ref("hello");
spdlog::get("statuslog")->info("testing \"{}\"", str_ref);

Even providing a global stream operator (which I'm not sure how I would implement btw) doesn't fix the build:

namespace spdlog {
    template <typename OStream>
    inline OStream& operator<< (OStream& parOS, const boost::string_ref& parStr) {
        return parOS << "hello";
    }
}

Am I doing something wrong? It's the first time I try this library so I know very little about it.

jartu commented 5 years ago

I have precisely the same problem. Has this been resolved?

gabime commented 5 years ago

Take a look at fmts docs onchow to do it correctly (http://fmtlib.net/latest/api.html#formatting-user-defined-types)