Until recently the Yosys stringf() and logging APIs used C-style variadic functions. That means C++ objects like std::string and std::string_view can’t be passed directly, so the code is littered with .c_str() calls… and fear of associated lifetime issues has led to temporary space leaks like log_id_cache.
[std::string_view is not used much in Yosys currently, but it’s nice because unlike const char*, it lets you print not-null-terminated substrings without any copying.]
It would be good to fix this so C++ standard string types and also Yosys types such as RTLIL::IdString are passed directly. This allows removing a lot of .c_str() calls to declutter the code.
There was some discussion about the way forward, here. There’s some interest in adopting std::format or {fmt}. However, that requires converting all format strings from %-style syntax to {}-style syntax. Instead, I proposed using C++ variadic templates to implement %-style formatting. I implemented that and my PR for stringf() was actually merged, so today you can pass std::string and std::string_view to stringf().
I have two followup PRs waiting for review. One adds support for IdString to stringf() and mass-removes .c_str() calls from stringf() parameters. The other updates the logging APIs to use the same formatting machinery as stringf() uses, thus immediately making std::string, std::string_view and IdString usable as log() parameters directly. (Of course std::string and IdString are passed by reference.)
Once these PRs are merged, we can look at fixing log_id and log_signal to avoid use of log_str() and the log_id_cache by returning std::string or std::string_view.
Even if the desired end goal is to use std::format or {fmt}, I think these PRs are a good incremental step forward.
I’m looking forward to this and agree that making incremental improvements is the way forward, even if we ultimately want to migrate to std::format or {fmt}.
As I mentioned during the dev JF, I’m particularly excited by the possibility to add formatters for RTLIL types and other custom types. Since, as you noted, some of the types commonly logged have multiple commonly used log representations (e.g. log_id stripping a \ prefix vs .c_str() that doesn’t), that does raise the question of how to handle that. IMO if the goal is to migrate to another formatting library it would make sense to choose a way that’s as forwards compatible as possible, although off the top of my head I have no idea what that should look like. Discussing this aspect might also be a bit premature, it certainly shouldn’t get in the way of getting the base functionality reviewed and merged, I just wanted to make sure my thoughts on this were written down somewhere.
One option would be to just retain log_id() as the syntax for stripping prefixes from IdString. I would change it to return std::string_vew, but that’s a minor change. Returning std::string_view is not any safer than return const char*, but at one point I hand-audited all log_id() calls and didn’t find any lifetime issues. That’s not surprising — even if you call log_id() on a temporary IdString, the temporary won’t be destroyed until the end of the enclosing statement, so in parameters tolog() calls the temporary will live long enough.
Alternatively, we could have dedicated format-string syntax to select alternate forms. A natural choice for printf-style formatting would be %#s to strip prefixes from IdString. {fmt} and std::format also support formatting syntax extension, with different syntax of course.
Of course we could support both log_id() and dedicated format-string syntax.