scylladb / seastar

High performance server-side application framework
http://seastar.io
Apache License 2.0
8.36k stars 1.55k forks source link

Pretty-printers - should Seastar have more of them? None of them? #98

Open nyh opened 8 years ago

nyh commented 8 years ago

Currently, we have in sstring.hh an operator<< implementation for std::vector. But why only std::vector? We can easily add similar implementations for std::set, std::unordered_set, std::unordered_map, and so on, and ScyllaDB for example does this.

I might be nice to have a Seastar header file which has all of these (and more) pretty printers - it probably should not be in sstring.hh, which isn't very related.

Finally, since Seastar code deals with std::exception_ptr a lot, and often wants to log them, having an operator<<() for std::exception_ptr is extremely useful for logging exceptions from futures. ScyllaDB for example has such operator<< for exception_ptr (and Seastar itself also has specialized exception printing code, report_exception(), which could use operator<< instead once we have it).

If the decision is that we should NOT add more operator<< implementations , we should probably remove the std::vector one as well - having just one is really unjustified and confusing.

nyh commented 8 years ago

Looking at Seastar again today, it seems the decision has been to add more of these pretty printers for pre-existing standard types. But they are still disorganized in various header files: We now have pretty printers for std::vector, std::unordered_map in core/sstring.hh, and for std::exception_ptr in util/log.hh.

avikivity commented 8 years ago

Maybe we should move them to a different header, and also push them to a namespace, so in order to use them you'd use using namespace seastar::pretty_printers, and avoid namespace pollution if you don't want them.

nyh commented 8 years ago

I like the separate header-file idea. I think the separate namespace is a bit excessive (and I'm not even convinced it will work, given that we're trying to pretty-print something in the std:: namespace) - I don't mind these pretty-printers, it's just strange I need to include "util/log.hh" (for example) to get them even if I don't plan to use the logger - or need to include "sstring.hh" if I want to print a vector of integers.