Add Formatv() versions of all our printf style formatting functions

[zturner]

Summary

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.

[clayborg]

It is nice to be able to use StringRef and std::string as is. I was wondering if you have:

StringRef null;
StringRef empty("");
StringRef hello("hello")

What does this show:

formatv("{0} {1} {2}", null, empty, hello);

I would hope for:

(null) "" "hello"

Where "(null)" is used for the empty StringRef since it doesn't have a value. This is what printf does if you pass nullptr in for a %s. The other string has a pointer, but no size, so it should show the empty string. Also for std::string and for StringRef if would be great if we can show binary values since a StringRef and std::string can contain NULL characters and also non printable characters. One option that would be really cool for both StringRef and std::string would be the memory view stuff we did a month back! So we could do:

StringRef bytes("\x01\x02\x03");
formatv("{0:memory}", bytes);

And get out a memory dump.

source/Target/Target.cpp

1558–1559 ↗

(On Diff #80949)

Can we add lldb_private::FileSpec support with this patch as well? Then the code can be:

error.SetErrorStringWithFormatv(
            "{0}[{1:x+}] can't be resolved, {0;filename} is not currently loaded",
            addr_module_sp->GetFileSpec(), resolved_addr.GetFileAddress());

[clayborg]

I would like to see the FileSpec having its formatv stuff done for this since it will start a whole slew of people wanting to use it and we should have an example of a class that show how to add your own formatting options.

[zturner]

Added a format provider for FileSpec. Style syntax is documented in this patch. Added some unit tests so you can see the output.

To answer Greg's earlier question about printing c-strings, formatv("'{0}' '{1}' '{2}'", (const char *)nullptr, "", "test"); would print "'' '' 'test'";. So yes that means that nullptr doesn't print "(null)". It could be made to do so if desired, but if it's any consolation the goal is really to move away from c-strings, and StringRef doesn't have any concept of null. There's just empty and not empty.

One nice thing about is that you can easily change the behavior of existing formatters using a mechanism which I've called "adapters". You can see an example of a builtin adapter in this patch, where I use fmt_repeat to repeat a character 7 times. To write an adapter for const char * that prints (null), you could do this:

struct fmt_or_null {
  explicit fmt_or_null(const char *s) : s(s) {}
  void format(llvm::raw_ostream &Stream, StringRef Style) const {
    if (!s)
      Stream << "(null)";  // Override the default behavior for nullptr;
    else
      llvm::format_provider<const char *>::format(s, Stream, Style);  // Otherwise just use the default;
  }
  const char *s;
};

void foo() {
  const char *s = nullptr;
  std::string result = llvm::formatv("{0}", fmt_or_null(s));
}

[labath]

This looks like a step in the right direction. I trust you won't go on a reformatting spree of the log messages until we settle on the syntax there. ;)

include/lldb/Core/ModuleSpec.h

244 ↗

(On Diff #80992)

you can delete the uint64_t cast now. It was only there to make this portably printable.

include/lldb/Host/FileSpec.h

784 ↗

(On Diff #80992)

I am wondering.. what is the preferred style for adding formatting capability. I sort of assumed that the format_provider thingy was reserved for cases where you had no control over the object being formatted (e.g. because it comes from a third party library), and that for cases like this we would use the member format function (or, to put it differently: if we're going to be using the format_provider everywhere, what's the point of having the member functions in the first place?).

Also, my preference would be to not have the F and D styles for FileSpec. I think they are redundant, as we have FileSpec::GetFilename and FileSpec::GetDirectory, and also they are not compiler-enforced --
Formatv('{0}', fs.GetFilename()) is slightly longer than Formatv('{0:F}', fs), but if I make a typo in the former one, it will be a compiler error, while the other one won't. I suppose it could be useful, if one wants to format say an array of FileSpecs using the array format syntax, but I'm not sure how often will someone want to do that *and* display only the filenames.

I don't care strongly about any of these things, but I want to know what to expect.

[clayborg]

Looking good. A few little changes and possibly supporting long option names that can be minimally specified. See inlined comments.

source/Host/common/FileSpec.cpp

1394 ↗

(On Diff #80992)

If we are going to not care about case I would say we should use lower case in our initial usages of this formatter.

This leads to another question: should we allow format strings to be specified with longer names and that all formatv strings can be minimally specified like the LLDB command names? Then we can choose more descriptive names like "filename" and "directory" which can be shortened to "f" and "d" since they minimally match. The main issue with this is if you wanted to add another format flavor that starts with any letter that already matches, like say we wanted to later add "filename-no-extension". We could say that the first in the list that partially matches would always win. This would mean that when adding support for a new flavor you would always need to add it to the end of the list. This can help keeps things more readable in the ::format functions while allowing users to also be explicit if they need to.

It would be nice to avoid asserts if possible in the logging ::format code as if we ever open up the logging to users somehow (uncontrolled input), we don't want to crash. I would rather see something in the output like "invalid format style 'ffff' for lldb_private::FileSpec" just to be safe. The user will see this output and it would be nice not to crash.

source/Target/Target.cpp

1558 ↗

(On Diff #80992)

I would prefer us to use lower case format characters in our example code if possible, so 'f' instead of 'F'.

[zturner]

source/Host/common/FileSpec.cpp

1394 ↗

(On Diff #80992)

I'm sort of ambivalent about long option names in the way you suggest. It definitely allows for less cryptic format strings, but on the other hand with our 80-column limit, with run the risk of leading to additional levels of wrapping which could reduce readability. That said, it's easy to modify these formatters, so maybe we could start with short only, and then re-evaluate after it starts getting some usage? Would give people a chance to build up a stronger opinion one way or the other.

As for asserts, I don't think it should be an issue here. The reason I say this is because the assert is compiled out in release mode, so the assert itself will never be the cause of a crash for the user. The only way it could crash is if the code that followed did not perform gracefully under the condition that triggered the assert to fire in the first place.

The assert here is used only to notify the programmer that they've made a mistake in their debug build, but in a normal build it will fall back to using the default Dir + Separator + Filename format if anything in the format string is unrecognized.

I prefer this to adding additional conditional branches, because printing something sane is better than printing "Unknown Format String" in a log file.

[clayborg]

source/Host/common/FileSpec.cpp

1394 ↗

(On Diff #80992)

I am saying that we add long format name support in now only in the ::format() functions but use them with the minimal characters on our code when logging. This would avoid any 80 char limitations when using them, but keep the ::format() functions more readable.

I am fine with the assert being there if the code still is designed to function when the assert isn't there. LLVM tends to say asserts are invariants and the code isn't expect to function when the asserts are off, so as long as we avoid avoid this in LLDB I am fine (use asserts for alerting, but code around it).

1401 ↗

(On Diff #80992)

Do we want all strings and classes that don't have valid state to print "(empty)"? If so maybe we build in something into formatv like:

Stream << formatv::empty();

This would allow us to get a consistent output from everyone when their objects are invalid.

[zturner]

source/Host/common/FileSpec.cpp

1394 ↗

(On Diff #80992)

Ahh I see what you mean. I wouldn't want us to do anything like that across the board, because we can't foresee all possible ways we might want to use it, and I don't think we'd be able to apply this rule consistently across all types and all formatters we'd ever make in LLDB. For example, I can imagine a hypothetical situation where you might do llvm::formatv("{0:pxest}") where p, x, e, s, and t are "codes" for some specific field, and it just strings all the results together. Then there are some situations where case matters, like when printing integers X` means hex with uppercase digits and x means hex with lowercase digits.

[clayborg]

So we need to formalize this in the formatv documentation before we begin wide adoption otherwise people can do what ever they want. It would be good to write this up so we get consistent implementations. But that is in here looks good enough to start with.

[zturner]

Thanks. Not going to commit this just yet, as I know there was at least one concern over on llvm-dev about adopting this. So I'll give it another few days to see if the complaints remain.

[clayborg]

Sounds good. Take it as my vote this is ok to start with if we get consensus

lldb/trunk/include/lldb/Core/Error.h

 #include "llvm/Support/DataTypes.h"
 
 #include <cstdarg>
 #include <cstdio>
 #include <string>
 
 #include "lldb/lldb-private.h"
 
+#include "llvm/Support/FormatVariadic.h"
+
 namespace lldb_private {
 
 class Log;
 
 //----------------------------------------------------------------------
 /// @class Error Error.h "lldb/Core/Error.h"
 /// @brief An error handling class.
 ///
public:
   /// @param format
   ///     A printf style format string
   //------------------------------------------------------------------
   int SetErrorStringWithFormat(const char *format, ...)
       __attribute__((format(printf, 2, 3)));
 
   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.
   ///
   /// Returns true if the error code in this object is considered a
   /// successful return value.
   ///
   /// @return
   ///     \b true if this object contains an value that describes

lldb/trunk/include/lldb/Core/Log.h

 // Other libraries and framework includes
 // Project includes
 #include "lldb/Core/ConstString.h"
 #include "lldb/Core/Flags.h"
 #include "lldb/Core/Logging.h"
 #include "lldb/Core/PluginInterface.h"
 #include "lldb/lldb-private.h"
 
+#include "llvm/Support/FormatVariadic.h"
+
 //----------------------------------------------------------------------
 // Logging Options
 //----------------------------------------------------------------------
 #define LLDB_LOG_OPTION_THREADSAFE (1u << 0)
 #define LLDB_LOG_OPTION_VERBOSE (1u << 1)
 #define LLDB_LOG_OPTION_DEBUG (1u << 2)
 #define LLDB_LOG_OPTION_PREPEND_SEQUENCE (1u << 3)
 #define LLDB_LOG_OPTION_PREPEND_TIMESTAMP (1u << 4)
 #define LLDB_LOG_OPTION_PREPEND_PROC_AND_THREAD (1u << 5)
 #define LLDB_LOG_OPTION_PREPEND_THREAD_NAME (1U << 6)
 #define LLDB_LOG_OPTION_BACKTRACE (1U << 7)
 #define LLDB_LOG_OPTION_APPEND (1U << 8)
 
 //----------------------------------------------------------------------
 // Logging Functions
 //----------------------------------------------------------------------
 namespace lldb_private {
 
-class Log {
+class Log final {
 public:
   //------------------------------------------------------------------
   // Callback definitions for abstracted plug-in log access.
   //------------------------------------------------------------------
   typedef void (*DisableCallback)(const char **categories,
                                   Stream *feedback_strm);
   typedef Log *(*EnableCallback)(lldb::StreamSP &log_stream_sp,
                                  uint32_t log_options, const char **categories,
public:
 
   //------------------------------------------------------------------
   // Member functions
   //------------------------------------------------------------------
   Log();
 
   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();
 
   const Flags &GetOptions() const;
 
   Flags &GetMask();
 
   const Flags &GetMask() const;

lldb/trunk/include/lldb/Core/ModuleSpec.h

if (m_object_size > 0) {
       if (dumped_something)
         strm.PutCString(", ");
       strm.Printf("object size = %" PRIu64, m_object_size);
       dumped_something = true;
     }
     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)));
     }
   }
 
   bool Matches(const ModuleSpec &match_module_spec,
                bool exact_arch_match) const {
     if (match_module_spec.GetUUIDPtr() &&
         match_module_spec.GetUUID() != GetUUID())

lldb/trunk/include/lldb/Core/Stream.h

 #include <stdarg.h>
 
 // C++ Includes
 // Other libraries and framework includes
 // Project includes
 #include "lldb/Core/Flags.h"
 #include "lldb/lldb-private.h"
 
+#include "llvm/Support/FormatVariadic.h"
+
 namespace lldb_private {
 
 //----------------------------------------------------------------------
 /// @class Stream Stream.h "lldb/Core/Stream.h"
 /// @brief A stream class that can stream formatted output to a file.
 //----------------------------------------------------------------------
 class Stream {
 public:
public:
   /// @param[in] ...
   ///     Variable arguments that are needed for the printf style
   ///     format string \a format.
   //------------------------------------------------------------------
   size_t Printf(const char *format, ...) __attribute__((format(printf, 2, 3)));
 
   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.
   ///
   /// Print a double quoted NULL terminated C string to the stream
   /// using the printf format in \a format.
   ///
   /// @param[in] cstr
   ///     A NULL terminated C string value.

lldb/trunk/include/lldb/Host/FileSpec.h

 
 // Other libraries and framework includes
 // Project includes
 #include "lldb/Core/ConstString.h"
 #include "lldb/Core/STLUtils.h"
 #include "lldb/Host/PosixApi.h"
 #include "lldb/lldb-private.h"
 
+#include "llvm/Support/FormatVariadic.h"
+
 namespace lldb_private {
 
 //----------------------------------------------------------------------
 /// @class FileSpec FileSpec.h "lldb/Host/FileSpec.h"
 /// @brief A file utility class.
 ///
 /// A file specification class that divides paths up into a directory
 /// and basename. These string values of the paths are put into uniqued

 
 //----------------------------------------------------------------------
 /// Dump a FileSpec object to a stream
 //----------------------------------------------------------------------
 Stream &operator<<(Stream &s, const FileSpec &f);
 
 } // namespace lldb_private
 
+namespace llvm {
+
+/// Implementation of format_provider<T> for FileSpec.
+///
+/// The options string of a FileSpec has the grammar:
+///
+///   file_spec_options   :: (empty) | F | D
+///
+///   =======================================================
+///   |  style  |     Meaning          |      Example       |
+///   -------------------------------------------------------
+///   |         |                      |  Input   |  Output |
+///   =======================================================
+///   |    F    | Only print filename  | /foo/bar |   bar   |
+///   |    D    | Only print directory | /foo/bar |  /foo/  |
+///   | (empty) | Print file and dir   |          |         |
+///   =======================================================
+///
+/// Any other value is considered an invalid format string.
+///
+template <> struct format_provider<lldb_private::FileSpec> {
+  static void format(const lldb_private::FileSpec &F, llvm::raw_ostream &Stream,
+                     StringRef Style);
+};
+}
+
 #endif // liblldb_FileSpec_h_

lldb/trunk/include/lldb/Interpreter/CommandReturnObject.h

 // Project includes
 #include "lldb/Core/STLUtils.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Core/StreamString.h"
 #include "lldb/Core/StreamTee.h"
 #include "lldb/lldb-private.h"
 
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/FormatVariadic.h"
 
 #include <memory>
 
 namespace lldb_private {
 
 class CommandReturnObject {
 public:
   CommandReturnObject();
public:
 
   void AppendError(llvm::StringRef in_string);
 
   void AppendRawError(llvm::StringRef in_string);
 
   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);
 
   lldb::ReturnStatus GetStatus();
 
   void SetStatus(lldb::ReturnStatus status);
 

lldb/trunk/source/Breakpoint/BreakpointOptions.cpp

BreakpointOptions::CommandData::CreateFromStructuredData(
   if (!success) {
     error.SetErrorString("Missing command language value.");
     return data_up;
   }
 
   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;
 
   StructuredData::Array *user_source;
   success = options_dict.GetValueForKeyAsArray(GetKey(OptionNames::UserSource),
                                                user_source);
   if (success) {

lldb/trunk/source/Commands/CommandObjectApropos.cpp

if (!search_word.empty()) {
         }
       }
 
       std::vector<const Property *> properties;
       const size_t num_properties =
           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);
       }
 
       result.SetStatus(eReturnStatusSuccessFinishNoResult);
     } else {
       result.AppendError("'' is not a valid search word.\n");

lldb/trunk/source/Core/Log.cpp

void Log::Debug(const char *format, ...) {
 
   va_list args;
   va_start(args, format);
   VAPrintf(format, args);
   va_end(args);
 }
 
 //----------------------------------------------------------------------
-// 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, ...) {
   if (!m_options.AllSet(bits))
     return;
 
   va_list args;
   va_start(args, format);
void Log::VAError(const char *format, va_list args) {
   if (arg_msg == nullptr)
     return;
 
   Printf("error: %s", arg_msg);
   free(arg_msg);
 }
 
 //----------------------------------------------------------------------
-// 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.
 //----------------------------------------------------------------------
 void Log::Verbose(const char *format, ...) {
   if (!m_options.Test(LLDB_LOG_OPTION_VERBOSE))
     return;
 
   va_list args;
   va_start(args, format);
   VAPrintf(format, args);
   va_end(args);
 }
 
 //----------------------------------------------------------------------
-// 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, ...) {
   char *arg_msg = nullptr;
   va_list args;
   va_start(args, format);
   ::vasprintf(&arg_msg, format, args);
   va_end(args);

lldb/trunk/source/Host/common/FileSpec.cpp

void FileSpec::SetFile(llvm::StringRef pathname, bool resolve,
 
   llvm::SmallString<64> resolved(pathname);
 
   if (resolve) {
     FileSpec::Resolve(resolved);
     m_is_resolved = true;
   }
 
-  Normalize(resolved, syntax);
+  Normalize(resolved, m_syntax);
 
   llvm::StringRef resolve_path_ref(resolved.c_str());
-  size_t dir_end = ParentPathEnd(resolve_path_ref, syntax);
+  size_t dir_end = ParentPathEnd(resolve_path_ref, m_syntax);
   if (dir_end == 0) {
     m_filename.SetString(resolve_path_ref);
     return;
   }
 
   m_directory.SetString(resolve_path_ref.substr(0, dir_end));
 
   size_t filename_begin = dir_end;
-  size_t root_dir_start = RootDirStart(resolve_path_ref, syntax);
+  size_t root_dir_start = RootDirStart(resolve_path_ref, m_syntax);
   while (filename_begin != llvm::StringRef::npos &&
          filename_begin < resolve_path_ref.size() &&
          filename_begin != root_dir_start &&
-         IsPathSeparator(resolve_path_ref[filename_begin], syntax))
+         IsPathSeparator(resolve_path_ref[filename_begin], m_syntax))
     ++filename_begin;
   m_filename.SetString((filename_begin == llvm::StringRef::npos ||
                         filename_begin >= resolve_path_ref.size())
                            ? "."
                            : resolve_path_ref.substr(filename_begin));
 }
 
 void FileSpec::SetFile(llvm::StringRef path, bool resolve, ArchSpec arch) {
bool FileSpec::IsRelative() const {
   } else if (m_filename) {
     // No directory, just a basename, return true
     return true;
   }
   return false;
 }
 
 bool FileSpec::IsAbsolute() const { return !FileSpec::IsRelative(); }
+
+void llvm::format_provider<FileSpec>::format(const FileSpec &F,
+                                             raw_ostream &Stream,
+                                             StringRef Style) {
+  assert(
+      (Style.empty() || Style.equals_lower("F") || Style.equals_lower("D")) &&
+      "Invalid FileSpec style!");
+
+  StringRef dir = F.GetDirectory().GetStringRef();
+  StringRef file = F.GetFilename().GetStringRef();
+
+  if (dir.empty() && file.empty()) {
+    Stream << "(empty)";
+    return;
+  }
+
+  if (Style.equals_lower("F")) {
+    Stream << (file.empty() ? "(empty)" : file);
+    return;
+  }
+
+  // Style is either D or empty, either way we need to print the directory.
+  if (!dir.empty()) {
+    // Directory is stored in normalized form, which might be different
+    // than preferred form.  In order to handle this, we need to cut off
+    // the filename, then denormalize, then write the entire denorm'ed
+    // directory.
+    llvm::SmallString<64> denormalized_dir = dir;
+    Denormalize(denormalized_dir, F.GetPathSyntax());
+    Stream << denormalized_dir;
+    Stream << GetPreferredPathSeparator(F.GetPathSyntax());
+  }
+
+  if (Style.equals_lower("D")) {
+    // We only want to print the directory, so now just exit.
+    if (dir.empty())
+      Stream << "(empty)";
+    return;
+  }
+
+  if (!file.empty())
+    Stream << file;
+}

lldb/trunk/source/Symbol/ClangASTContext.cpp

 //===-- ClangASTContext.cpp -------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Symbol/ClangASTContext.h"
 
+#include "llvm/Support/FormatAdapters.h"
+#include "llvm/Support/FormatVariadic.h"
+
 // C Includes
 // C++ Includes
 #include <mutex> // std::once
 #include <string>
 #include <vector>
 
 // Other libraries and framework includes
 
CompilerType ClangASTContext::GetChildCompilerTypeAtIndex(
 
   case clang::Type::ConstantArray:
   case clang::Type::IncompleteArray:
     if (ignore_array_bounds || idx_is_valid) {
       const clang::ArrayType *array = GetQualType(type)->getAsArrayTypeUnsafe();
       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;
           return element_type;
         }
       }
     }
     break;
if (GetCompleteType(type)) {
             s->PutChar('{');
           else
             s->PutChar(',');
 
           clang::QualType base_class_qual_type = base_class->getType();
           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);
 
           // Dump the value of the member
           CompilerType base_clang_type(getASTContext(), base_class_qual_type);
           base_clang_type.DumpValue(
               exe_ctx,

lldb/trunk/source/Target/Target.cpp

size_t Target::ReadMemory(const Address &addr, bool prefer_file_cache,
 
   if (ProcessIsValid()) {
     if (load_addr == LLDB_INVALID_ADDRESS)
       load_addr = resolved_addr.GetLoadAddress(this);
 
     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",
-            addr_module_sp->GetFileSpec().GetFilename().AsCString("<Unknown>"),
-            resolved_addr.GetFileAddress(),
-            addr_module_sp->GetFileSpec().GetFilename().AsCString("<Unknonw>"));
+        error.SetErrorStringWithFormatv(
+            "{0:F}[{1:x+}] can't be resolved, {0:F} is not currently loaded",
+            addr_module_sp->GetFileSpec(), resolved_addr.GetFileAddress());
       else
         error.SetErrorStringWithFormat("0x%" PRIx64 " can't be resolved",
                                        resolved_addr.GetFileAddress());
     } else {
       bytes_read = m_process_sp->ReadMemory(load_addr, dst, dst_len, error);
       if (bytes_read != dst_len) {
         if (error.Success()) {
           if (bytes_read == 0)

lldb/trunk/unittests/Host/FileSpecTest.cpp

TEST(FileSpecTest, GetNormalizedPath) {
   for (auto test : windows_tests) {
     EXPECT_EQ(test.second,
               FileSpec(test.first, false, FileSpec::ePathSyntaxWindows)
                   .GetNormalizedPath()
                   .GetPath())
         << "Original path: " << test.first;
   }
 }
+
+TEST(FileSpecTest, FormatFileSpec) {
+  auto win = FileSpec::ePathSyntaxWindows;
+
+  FileSpec F;
+  EXPECT_EQ("(empty)", llvm::formatv("{0}", F).str());
+  EXPECT_EQ("(empty)", llvm::formatv("{0:D}", F).str());
+  EXPECT_EQ("(empty)", llvm::formatv("{0:F}", F).str());
+
+  F = FileSpec("C:\\foo\\bar.txt", false, win);
+  EXPECT_EQ("C:\\foo\\bar.txt", llvm::formatv("{0}", F).str());
+  EXPECT_EQ("C:\\foo\\", llvm::formatv("{0:D}", F).str());
+  EXPECT_EQ("bar.txt", llvm::formatv("{0:F}", F).str());
+
+  F = FileSpec("foo\\bar.txt", false, win);
+  EXPECT_EQ("foo\\bar.txt", llvm::formatv("{0}", F).str());
+  EXPECT_EQ("foo\\", llvm::formatv("{0:D}", F).str());
+  EXPECT_EQ("bar.txt", llvm::formatv("{0:F}", F).str());
+
+  F = FileSpec("foo", false, win);
+  EXPECT_EQ("foo", llvm::formatv("{0}", F).str());
+  EXPECT_EQ("foo", llvm::formatv("{0:F}", F).str());
+  EXPECT_EQ("(empty)", llvm::formatv("{0:D}", F).str());
+}
 No newline at end of file