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

Reply via email to