zturner created this revision. zturner added reviewers: labath, clayborg, jingham. zturner added a subscriber: lldb-commits.
We have various functions like `Stream::Printf()`, and `Error::SetErrorStringWithFormat()`, `Log::Printf()`, and various others. I added functions that delegate to `formatv` in case people want to see some actual usage in practice. I also converted a few various Printfs, etc to this syntax just to demonstrate what it looks like. I tried to find callsites that would illustrate how it makes things simpler, such as : - not needing to call `.c_str()` and being able to pass `std::string` and/or `StringRef` directly as the argument. - places where the same argument is printed multiple times, only having to pass it once (incidentally this fixed a spelling mistake since when passing the argument the 2nd time, there had been a typo. So this also serves as an example of how this is safer than Printf, since less code typed = fewer chances to mess up). - places where complex format strings are constructed, such as using the printf width specifier `*` or `PRIx64` are used and now we can simply pass whatever type we have directly, even if it's architecture dependent like `size_t`. Various other uses. Obviously a change like this is too big to do all in one pass, since it would have a roughly 100% chance of introducing new bugs and be impossible to bisect. This is basically just to get the hookups in place and have a single-digit number of illustrative use cases. https://reviews.llvm.org/D27632 Files: include/lldb/Core/Error.h include/lldb/Core/Log.h include/lldb/Core/ModuleSpec.h include/lldb/Core/Stream.h include/lldb/Interpreter/CommandReturnObject.h source/Breakpoint/BreakpointOptions.cpp source/Commands/CommandObjectApropos.cpp source/Core/Log.cpp source/Symbol/ClangASTContext.cpp source/Target/Target.cpp
Index: source/Target/Target.cpp =================================================================== --- source/Target/Target.cpp +++ source/Target/Target.cpp @@ -1554,11 +1554,10 @@ if (load_addr == LLDB_INVALID_ADDRESS) { ModuleSP addr_module_sp(resolved_addr.GetModule()); if (addr_module_sp && addr_module_sp->GetFileSpec()) - error.SetErrorStringWithFormat( - "%s[0x%" PRIx64 "] can't be resolved, %s in not currently loaded", + error.SetErrorStringWithFormatv( + "{0}[{1:x+}] can't be resolved, {0} is not currently loaded", addr_module_sp->GetFileSpec().GetFilename().AsCString("<Unknown>"), - resolved_addr.GetFileAddress(), - addr_module_sp->GetFileSpec().GetFilename().AsCString("<Unknonw>")); + resolved_addr.GetFileAddress()); else error.SetErrorStringWithFormat("0x%" PRIx64 " can't be resolved", resolved_addr.GetFileAddress()); Index: source/Symbol/ClangASTContext.cpp =================================================================== --- source/Symbol/ClangASTContext.cpp +++ source/Symbol/ClangASTContext.cpp @@ -9,6 +9,9 @@ #include "lldb/Symbol/ClangASTContext.h" +#include "llvm/Support/FormatAdapters.h" +#include "llvm/Support/FormatVariadic.h" + // C Includes // C++ Includes #include <mutex> // std::once @@ -6739,10 +6742,7 @@ if (array) { CompilerType element_type(getASTContext(), array->getElementType()); if (element_type.GetCompleteType()) { - char element_name[64]; - ::snprintf(element_name, sizeof(element_name), "[%" PRIu64 "]", - static_cast<uint64_t>(idx)); - child_name.assign(element_name); + child_name = llvm::formatv("[{0}]", idx); child_byte_size = element_type.GetByteSize( exe_ctx ? exe_ctx->GetBestExecutionContextScope() : NULL); child_byte_offset = (int32_t)idx * (int32_t)child_byte_size; @@ -8883,8 +8883,8 @@ std::string base_class_type_name(base_class_qual_type.getAsString()); // Indent and print the base class type name - s->Printf("\n%*s%s ", depth + DEPTH_INCREMENT, "", - base_class_type_name.c_str()); + s->Format("\n{0}{1}", llvm::fmt_repeat(" ", depth + DEPTH_INCREMENT), + base_class_type_name); clang::TypeInfo base_class_type_info = getASTContext()->getTypeInfo(base_class_qual_type); Index: source/Core/Log.cpp =================================================================== --- source/Core/Log.cpp +++ source/Core/Log.cpp @@ -138,20 +138,6 @@ } //---------------------------------------------------------------------- -// Print debug strings if and only if the global debug option is set to -// a non-zero value. -//---------------------------------------------------------------------- -void Log::DebugVerbose(const char *format, ...) { - if (!GetOptions().AllSet(LLDB_LOG_OPTION_DEBUG | LLDB_LOG_OPTION_VERBOSE)) - return; - - va_list args; - va_start(args, format); - VAPrintf(format, args); - va_end(args); -} - -//---------------------------------------------------------------------- // Log only if all of the bits are set //---------------------------------------------------------------------- void Log::LogIf(uint32_t bits, const char *format, ...) { @@ -186,24 +172,6 @@ } //---------------------------------------------------------------------- -// Printing of errors that ARE fatal. Exit with ERR exit code -// immediately. -//---------------------------------------------------------------------- -void Log::FatalError(int err, const char *format, ...) { - char *arg_msg = nullptr; - va_list args; - va_start(args, format); - ::vasprintf(&arg_msg, format, args); - va_end(args); - - if (arg_msg != nullptr) { - Printf("error: %s", arg_msg); - ::free(arg_msg); - } - ::exit(err); -} - -//---------------------------------------------------------------------- // Printing of warnings that are not fatal only if verbose mode is // enabled. //---------------------------------------------------------------------- @@ -218,27 +186,6 @@ } //---------------------------------------------------------------------- -// Printing of warnings that are not fatal only if verbose mode is -// enabled. -//---------------------------------------------------------------------- -void Log::WarningVerbose(const char *format, ...) { - if (!m_options.Test(LLDB_LOG_OPTION_VERBOSE)) - return; - - char *arg_msg = nullptr; - va_list args; - va_start(args, format); - ::vasprintf(&arg_msg, format, args); - va_end(args); - - if (arg_msg == nullptr) - return; - - Printf("warning: %s", arg_msg); - free(arg_msg); -} - -//---------------------------------------------------------------------- // Printing of warnings that are not fatal. //---------------------------------------------------------------------- void Log::Warning(const char *format, ...) { Index: source/Commands/CommandObjectApropos.cpp =================================================================== --- source/Commands/CommandObjectApropos.cpp +++ source/Commands/CommandObjectApropos.cpp @@ -90,9 +90,9 @@ m_interpreter.GetDebugger().Apropos(search_word, properties); if (num_properties) { const bool dump_qualified_name = true; - result.AppendMessageWithFormat( - "\nThe following settings variables may relate to '%s': \n\n", - args[0].c_str()); + result.AppendMessageWithFormatv( + "\nThe following settings variables may relate to '{0}': \n\n", + args[0].ref); for (size_t i = 0; i < num_properties; ++i) properties[i]->DumpDescription( m_interpreter, result.GetOutputStream(), 0, dump_qualified_name); Index: source/Breakpoint/BreakpointOptions.cpp =================================================================== --- source/Breakpoint/BreakpointOptions.cpp +++ source/Breakpoint/BreakpointOptions.cpp @@ -86,8 +86,8 @@ found_something = true; interp_language = ScriptInterpreter::StringToLanguage(interpreter_str); if (interp_language == eScriptLanguageUnknown) { - error.SetErrorStringWithFormat("Unknown breakpoint command language: %s.", - interpreter_str.c_str()); + error.SetErrorStringWithFormatv("Unknown breakpoint command language: {0}.", + interpreter_str); return data_up; } data_up->interpreter = interp_language; Index: include/lldb/Interpreter/CommandReturnObject.h =================================================================== --- include/lldb/Interpreter/CommandReturnObject.h +++ include/lldb/Interpreter/CommandReturnObject.h @@ -21,6 +21,7 @@ #include "lldb/lldb-private.h" #include "llvm/ADT/StringRef.h" +#include "llvm/Support/FormatVariadic.h" #include <memory> @@ -113,6 +114,21 @@ void AppendErrorWithFormat(const char *format, ...) __attribute__((format(printf, 2, 3))); + template <typename... Args> + void AppendMessageWithFormatv(const char *format, Args &&... args) { + AppendMessage(llvm::formatv(format, std::forward<Args>(args)...).str()); + } + + template <typename... Args> + void AppendWarningWithFormatv(const char *format, Args &&... args) { + AppendWarning(llvm::formatv(format, std::forward<Args>(args)...).str()); + } + + template <typename... Args> + void AppendErrorWithFormatv(const char *format, Args &&... args) { + AppendError(llvm::formatv(format, std::forward<Args>(args)...).str()); + } + void SetError(const Error &error, const char *fallback_error_cstr = nullptr); void SetError(llvm::StringRef error_cstr); Index: include/lldb/Core/Stream.h =================================================================== --- include/lldb/Core/Stream.h +++ include/lldb/Core/Stream.h @@ -19,6 +19,8 @@ #include "lldb/Core/Flags.h" #include "lldb/lldb-private.h" +#include "llvm/Support/FormatVariadic.h" + namespace lldb_private { //---------------------------------------------------------------------- @@ -485,6 +487,10 @@ size_t PrintfVarArg(const char *format, va_list args); + template <typename... Args> void Format(const char *format, Args &&... args) { + PutCString(llvm::formatv(format, std::forward<Args>(args)...).str()); + } + //------------------------------------------------------------------ /// Output a quoted C string value to the stream. /// Index: include/lldb/Core/ModuleSpec.h =================================================================== --- include/lldb/Core/ModuleSpec.h +++ include/lldb/Core/ModuleSpec.h @@ -240,7 +240,7 @@ if (m_object_mod_time != llvm::sys::TimePoint<>()) { if (dumped_something) strm.PutCString(", "); - strm.Printf("object_mod_time = 0x%" PRIx64, + strm.Format("object_mod_time = {0:x+}", uint64_t(llvm::sys::toTimeT(m_object_mod_time))); } } Index: include/lldb/Core/Log.h =================================================================== --- include/lldb/Core/Log.h +++ include/lldb/Core/Log.h @@ -25,6 +25,8 @@ #include "lldb/Core/PluginInterface.h" #include "lldb/lldb-private.h" +#include "llvm/Support/FormatVariadic.h" + //---------------------------------------------------------------------- // Logging Options //---------------------------------------------------------------------- @@ -43,7 +45,7 @@ //---------------------------------------------------------------------- namespace lldb_private { -class Log { +class Log final { public: //------------------------------------------------------------------ // Callback definitions for abstracted plug-in log access. @@ -102,42 +104,32 @@ Log(const lldb::StreamSP &stream_sp); - virtual ~Log(); + ~Log(); + + void PutCString(const char *cstr); + void PutString(llvm::StringRef str); - virtual void PutCString(const char *cstr); - virtual void PutString(llvm::StringRef str); + template <typename... Args> void Format(const char *fmt, Args &&... args) { + PutString(llvm::formatv(fmt, std::forward<Args>(args)...).str()); + } // CLEANUP: Add llvm::raw_ostream &Stream() function. - virtual void Printf(const char *format, ...) - __attribute__((format(printf, 2, 3))); + void Printf(const char *format, ...) __attribute__((format(printf, 2, 3))); - virtual void VAPrintf(const char *format, va_list args); + void VAPrintf(const char *format, va_list args); - virtual void LogIf(uint32_t mask, const char *fmt, ...) + void LogIf(uint32_t mask, const char *fmt, ...) __attribute__((format(printf, 3, 4))); - virtual void Debug(const char *fmt, ...) - __attribute__((format(printf, 2, 3))); - - virtual void DebugVerbose(const char *fmt, ...) - __attribute__((format(printf, 2, 3))); + void Debug(const char *fmt, ...) __attribute__((format(printf, 2, 3))); - virtual void Error(const char *fmt, ...) - __attribute__((format(printf, 2, 3))); - - virtual void VAError(const char *format, va_list args); - - virtual void FatalError(int err, const char *fmt, ...) - __attribute__((format(printf, 3, 4))); + void Error(const char *fmt, ...) __attribute__((format(printf, 2, 3))); - virtual void Verbose(const char *fmt, ...) - __attribute__((format(printf, 2, 3))); + void VAError(const char *format, va_list args); - virtual void Warning(const char *fmt, ...) - __attribute__((format(printf, 2, 3))); + void Verbose(const char *fmt, ...) __attribute__((format(printf, 2, 3))); - virtual void WarningVerbose(const char *fmt, ...) - __attribute__((format(printf, 2, 3))); + void Warning(const char *fmt, ...) __attribute__((format(printf, 2, 3))); Flags &GetOptions(); Index: include/lldb/Core/Error.h =================================================================== --- include/lldb/Core/Error.h +++ include/lldb/Core/Error.h @@ -19,6 +19,8 @@ #include "lldb/lldb-private.h" +#include "llvm/Support/FormatVariadic.h" + namespace lldb_private { class Log; @@ -258,6 +260,11 @@ int SetErrorStringWithVarArg(const char *format, va_list args); + template <typename... Args> + void SetErrorStringWithFormatv(const char *format, Args &&... args) { + SetErrorString(llvm::formatv(format, std::forward<Args>(args)...).str()); + } + //------------------------------------------------------------------ /// Test for success condition. ///
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits