Skip to content

Commit

Permalink
[common] Add fmt_runtime compatibility shim for C++20 (RobotLocomotio…
Browse files Browse the repository at this point in the history
…n#17958)

Add previously-overlooked C++17 flag to an acceptance test, now that
we use <string_view> in one of the headers (text_logging) that happens
to be covered by that acceptance test.
  • Loading branch information
jwnimmer-tri authored Sep 22, 2022
1 parent 6f88ff9 commit 3c39254
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 4 deletions.
6 changes: 6 additions & 0 deletions common/test/text_logging_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,12 @@ GTEST_TEST(TextLoggingTest, ConstantTest) {
#endif
}

// Check that the "warn once" idiom compiles and doesn't crash at runtime.
GTEST_TEST(TextLoggingTest, WarnOnceTest) {
static const drake::logging::Warn log_once(
"The log_once happened as expected.");
}

// Abuse gtest internals to verify that logging actually prints when enabled,
// and that the default level is INFO.
GTEST_TEST(TextLoggingTest, CaptureOutputTest) {
Expand Down
13 changes: 12 additions & 1 deletion common/text_logging.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,14 @@ used by Drake might be older.)

namespace drake {

#if FMT_VERSION >= 80000 || defined(DRAKE_DOXYGEN_CXX)
/// When using fmt >= 8, this is an alias for fmt::runtime.
/// When using fmt < 8, this is a no-op.
inline auto fmt_runtime(std::string_view s) { return fmt::runtime(s); }
#else
inline auto fmt_runtime(std::string_view s) { return s; }
#endif

#ifdef HAVE_SPDLOG
namespace logging {

Expand Down Expand Up @@ -195,7 +203,10 @@ sink* get_dist_sink();
struct Warn {
template <typename... Args>
Warn(const char* a, const Args&... b) {
drake::log()->warn(a, b...);
// TODO(jwnimmer-tri) Ideally we would compile-time check our Warn format
// strings without using fmt_runtime here, but I haven't figured out how
// to forward the arguments properly for all versions of fmt.
drake::log()->warn(fmt_runtime(a), b...);
}
};

Expand Down
5 changes: 3 additions & 2 deletions geometry/meshcat.cc
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,8 @@ class Meshcat::Impl {
// values) through to fmt to allow any fmt-specific exception to percolate.
// Then, confirm that the user's pattern started with a valid protocol.
const std::string url = fmt::format(
params.web_url_pattern, fmt::arg("host", "foo"), fmt::arg("port", 1));
fmt_runtime(params.web_url_pattern),
fmt::arg("host", "foo"), fmt::arg("port", 1));
if (url.substr(0, 4) != "http") {
throw std::logic_error("The web_url_pattern must be http:// or https://");
}
Expand Down Expand Up @@ -611,7 +612,7 @@ class Meshcat::Impl {
const bool is_localhost = host.empty() || host == "*";
const std::string display_host = is_localhost ? "localhost" : host;
return fmt::format(
params_.web_url_pattern,
fmt_runtime(params_.web_url_pattern),
fmt::arg("host", display_host),
fmt::arg("port", port_));
}
Expand Down
2 changes: 1 addition & 1 deletion systems/sensors/image_writer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ std::string ImageWriter::MakeFileName(const std::string& format,

int64_t u_time = static_cast<int64_t>(time * 1e6 + 0.5);
int m_time = static_cast<int>(time * 1e3 + 0.5);
return fmt::format(format, fmt::arg("port_name", port_name),
return fmt::format(fmt_runtime(format), fmt::arg("port_name", port_name),
fmt::arg("image_type", labels_.at(pixel_type)),
fmt::arg("time_double", time),
fmt::arg("time_usec", u_time),
Expand Down
1 change: 1 addition & 0 deletions tools/install/bazel/test/drake_bazel_installed_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ def main():
cc_test(
name = "text_logging_test",
srcs = ["text_logging_test.cc"],
copts = ["--std=c++17"],
# TODO(jwnimmer-tri) On macOS, we need to pkg-config fmt for this to pass.
# For the moment, we'll say that :drake_shared_library is Ubuntu-only.
tags = ["manual"] if OS_NAME == "mac os x" else [],
Expand Down

0 comments on commit 3c39254

Please sign in to comment.