RobotLocomotion / drake

Model-based design and verification for robotics.
https://drake.mit.edu
Other
3.33k stars 1.26k forks source link

Undo the need for FMT_DEPRECATED_OSTREAM #17742

Open jwnimmer-tri opened 2 years ago

jwnimmer-tri commented 2 years ago

To solve #17732 we added -DFMT_DEPRECATED_OSTREAM in #17739 and #17743, but this is not an ideal work-around -- and in any case, will no longer work as of fmt 10.

The risk is that as soon as fmt is released, homebrew will upgrade to it and Drake will no longer build on macOS. Since fmt is a public dependency of Drake, it's difficult to imagine replacing it with a back-dated, non-homebew version.

We will need to rework Drake to concurrently support fmt ~6.1 (what Ubuntu 20 ships) and fmt > 9.

See also FMT_DEPRECATED_OSTREAM in the fmt 9 release notes.

For Drake users

As a guide for users to update their code (due to deprecation warning or other fmt-related errors):

(0) For starters, if you are using fmt version less than 10, you could define -DFMT_DEPRECATED_OSTREAM=1 in your own build system to opt-in to some of Drake's prior behavior. Drake is no longer forcing that option upon you, but we still intend to be compatible with users who choose that option.

For deprecation warnings with drake's operator<< functions, here's how to adjust your code:

(1) If you were using std::cout << "thing = " << drake_thing << "\n"; for debugging, replace that with fmt::print("thing = {}\n", drake_thing);.

(2) Likewise for error output, use fmt::print(stderr, "thing = {}\n", drake_thing);.

(3) If you just need the std::string value of a drake object, use fmt::to_string(drake_thing) to obtain it as a string. You can even use this spelling in your operator<< streaming in case you don't want to convert to fmt::print, i.e., std::cout << "thing = " << fmt::to_string(drake_thing) << "\n";

For Drake developers

(1) When doing temporary printf-debugging, don't use std::cerr with operator<<. Instead use fmt::print("x = {}\n", x) for stdout or fmt::print(stderr, "x = {}\n", x) for stderr. Likewise for any console output from example programs.

(2) When building up exception message text, strongly disprefer std::stringstream with operator<<. Usually fmt can directly do what you want. (See especially fmt::join().) If you really do need to use stringstream for a particular message, don't stream any values directly; rather use fmt for all intermediate data, i.e., s << fmt::format("... with x = {}", x);.

(3) Don't say fmt::format("{}", foo) with a literal "{}" and nothing else. There is a simpler spelling -- fmt::to_string(foo).

(4) When putting Eigen data into your fmt string (whether it be for logging, or print-debugging, or error messages) you need to wrap the Eigen object as fmt_eigen(M) i.e. fmt::format("M = {}", fmt_eigen(M)). You'll need to #include "drake/common/fmt_eigen.h" to access that function.

(5) When printing a third-party type whose only affordance for string output is operator<< use fmt_streamed i.e. fmt::format("third = {}", fmt_streamed(third)). You'll need to #include "drake/common/fmt_ostream.h" to access that function. This is very rare (only 1 use in Drake so far). See if the type offers a to_string() function instead of using operator<<; many do.

(6) When implementing a string output feature for a Drake type, eventually we'll want to teach you how to use fmt::formatter<T>. In the meantime, you can implement operator<< and use an ostream_formatter specialization. Grep around in Drake for examples. Note also the DRAKE_FORMATTER_AS macro. If your type can be formatted by delegating to another type (e.g., for TypeSafeIndex we can delegate to int), then you can use that macro.

PR train

Plan of attack:


WIP branch: https://github.com/jwnimmer-tri/drake/commits/fmt-v9

jwnimmer-tri commented 1 year ago

For the record, the issue title doesn't quite capture what remains (though the checklist does).

The remaining work here is to (1) offer better formatters, and (2) deprecate more operator<< stuff. In short, just port Drake to modern fmt best practices.