rewrite FormatDiagnostic to be less gross and a lot more efficient.

This also makes it illegal to have bare '%'s in diagnostics.  If you
want a % in a diagnostic, use %%.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@59596 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Basic/Diagnostic.h b/include/clang/Basic/Diagnostic.h
index 7742307d4728acf7f1da4e79fa3ac3858454c055..303d83cc66d95b2fdfbb74eb0b64f2671bf21e6d 100644 (file)
--- a/include/clang/Basic/Diagnostic.h
+++ b/include/clang/Basic/Diagnostic.h
@@ -18,6 +18,10 @@
 #include <string>
 #include <cassert>
 
+namespace llvm {
+  template <typename T> class SmallVectorImpl;
+}
+
 namespace clang {
   class DiagnosticClient;
   class SourceRange;
@@ -334,6 +338,10 @@ public:
     return *this;
   }
   
+  /// FormatDiagnostic - Format this diagnostic into a string, substituting the
+  /// formal arguments into the %0 slots.  The result is appended onto the Str
+  /// array.
+  void FormatDiagnostic(llvm::SmallVectorImpl<char> &OutStr) const;
 };
 
 
@@ -341,16 +349,13 @@ public:
 /// diag::kind enum.  This actually returns a new instance of DiagnosticInfo
 /// which emits the diagnostics (through ProcessDiag) when it is destroyed.
 inline DiagnosticInfo Diagnostic::Report(FullSourceLoc Pos, unsigned DiagID) {
-  DiagnosticInfo D(this, Pos, DiagID);
-  return D;
+  return DiagnosticInfo(this, Pos, DiagID);
 }
   
 
 /// DiagnosticClient - This is an abstract interface implemented by clients of
 /// the front-end, which formats and prints fully processed diagnostics.
 class DiagnosticClient {
-protected:
-  std::string FormatDiagnostic(const DiagnosticInfo &Info);
 public:
   virtual ~DiagnosticClient();
 

diff --git a/include/clang/Basic/DiagnosticKinds.def b/include/clang/Basic/DiagnosticKinds.def
index af9df0d0c82c47b30d72cf6af5575f7e2371de64..c98ad4c58bc1d104a76c110c283b84fb1657beb8 100644 (file)
--- a/include/clang/Basic/DiagnosticKinds.def
+++ b/include/clang/Basic/DiagnosticKinds.def
@@ -1392,11 +1392,11 @@ DIAG(warn_not_compound_assign, WARNING,
 DIAG(warn_printf_not_string_constant, WARNING,
      "format string is not a string literal (potentially insecure)")
 DIAG(warn_printf_write_back, WARNING,
-     "use of '%n' in format string discouraged (potentially insecure)")
+     "use of '%%n' in format string discouraged (potentially insecure)")
 DIAG(warn_printf_insufficient_data_args, WARNING,
-    "more '%' conversions than data arguments")
+    "more '%%' conversions than data arguments")
 DIAG(warn_printf_too_many_data_args, WARNING,
-    "more data arguments than '%' conversions")
+    "more data arguments than '%%' conversions")
 DIAG(warn_printf_invalid_conversion, WARNING,
     "invalid conversion '%0'")
 DIAG(warn_printf_missing_format_string, WARNING,

diff --git a/lib/Analysis/PathDiagnostic.cpp b/lib/Analysis/PathDiagnostic.cpp
index aed83ea84fb48e1fa9a664747e0a9bac649a79f6..29ea8c0e3753ae30181377bf18f6feebdb09aed1 100644 (file)
--- a/lib/Analysis/PathDiagnostic.cpp
+++ b/lib/Analysis/PathDiagnostic.cpp
@@ -12,8 +12,8 @@
 //===----------------------------------------------------------------------===//
 
 #include "clang/Analysis/PathDiagnostic.h"
+#include "llvm/ADT/SmallString.h"
 #include <sstream>
-
 using namespace clang;
   
 PathDiagnostic::~PathDiagnostic() {
@@ -36,10 +36,13 @@ void PathDiagnosticClient::HandleDiagnostic(Diagnostic::Level DiagLevel,
   case Diagnostic::Fatal:   LevelStr = "fatal error: "; break;
   }
 
-  std::string Msg = FormatDiagnostic(Info);
+  llvm::SmallString<100> StrC;
+  StrC += LevelStr;
+  Info.FormatDiagnostic(StrC);
   
   PathDiagnosticPiece *P =
-    new PathDiagnosticPiece(Info.getLocation(), LevelStr+Msg);
+    new PathDiagnosticPiece(Info.getLocation(),
+                            std::string(StrC.begin(), StrC.end()));
   
   for (unsigned i = 0, e = Info.getNumRanges(); i != e; ++i)
     P->addRange(Info.getRange(i));

diff --git a/lib/Basic/Diagnostic.cpp b/lib/Basic/Diagnostic.cpp
index 35d454b1b9a113ce9f18ce05a7f1bbef69c6408b..e8c24ab0a715c7f3af4acc4b441e6fdd6d2a8d6d 100644 (file)
--- a/lib/Basic/Diagnostic.cpp
+++ b/lib/Basic/Diagnostic.cpp
@@ -13,7 +13,7 @@
 
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/SourceLocation.h"
-#include <cassert>
+#include "llvm/ADT/SmallVector.h"
 #include <vector>
 #include <map>
 #include <cstring>
@@ -246,20 +246,41 @@ void Diagnostic::ProcessDiag(const DiagnosticInfo &Info) {
 
 DiagnosticClient::~DiagnosticClient() {}
 
-std::string DiagnosticClient::FormatDiagnostic(const DiagnosticInfo &Info) {
-  std::string Msg = Info.getDiags()->getDescription(Info.getID());
+
+/// FormatDiagnostic - Format this diagnostic into a string, substituting the
+/// formal arguments into the %0 slots.  The result is appended onto the Str
+/// array.
+void DiagnosticInfo::
+FormatDiagnostic(llvm::SmallVectorImpl<char> &OutStr) const {
+  const char *DiagStr = getDiags()->getDescription(getID());
+  const char *DiagEnd = DiagStr+strlen(DiagStr);
   
-  // Replace all instances of %0 in Msg with 'Extra'.  This is a pretty horrible
-  // and inefficient way to do this, we could improve this a lot if we care.
-  for (unsigned i = 0; i < Msg.size() - 1; ++i) {
-    if (Msg[i] == '%' && isdigit(Msg[i + 1])) {
-      unsigned StrNo = Msg[i + 1] - '0';
-      Msg = std::string(Msg.begin(), Msg.begin() + i) +
-      (Info.getArgKind(StrNo) == DiagnosticInfo::ak_std_string ? 
-      Info.getArgStdStr(StrNo) : std::string(Info.getArgCStr(StrNo))) +
-            std::string(Msg.begin() + i + 2, Msg.end());
+  while (DiagStr != DiagEnd) {
+    if (DiagStr[0] != '%') {
+      // Append non-%0 substrings to Str if we have one.
+      const char *StrEnd = std::find(DiagStr, DiagEnd, '%');
+      OutStr.append(DiagStr, StrEnd);
+      DiagStr = StrEnd;
+    } else if (DiagStr[1] == '%') {
+      OutStr.push_back('%');  // %% -> %.
+      DiagStr += 2;
+    } else {
+      assert(isdigit(DiagStr[1]) && "Must escape % with %%");
+      unsigned StrNo = DiagStr[1] - '0';
+
+      switch (getArgKind(StrNo)) {
+      case DiagnosticInfo::ak_std_string: {
+        const std::string &S = getArgStdStr(StrNo);
+        OutStr.append(S.begin(), S.end());
+        break;
+      }
+      case DiagnosticInfo::ak_c_string: {
+        const char *S = getArgCStr(StrNo);
+        OutStr.append(S, S + strlen(S));
+        break;
+      }
+      }
+      DiagStr += 2;
     }
   }
-
-  return Msg;
 }

diff --git a/lib/Driver/TextDiagnosticBuffer.cpp b/lib/Driver/TextDiagnosticBuffer.cpp
index b138b1a24d2e412bc435010ce8f806a96c7d0347..5032cdb0b5d9ec4a9811b12ee64625bed40b32b8 100644 (file)
--- a/lib/Driver/TextDiagnosticBuffer.cpp
+++ b/lib/Driver/TextDiagnosticBuffer.cpp
@@ -12,6 +12,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "clang/Driver/TextDiagnosticBuffer.h"
+#include "llvm/ADT/SmallString.h"
 using namespace clang;
 
 /// HandleDiagnostic - Store the errors, warnings, and notes that are
@@ -19,19 +20,19 @@ using namespace clang;
 /// 
 void TextDiagnosticBuffer::HandleDiagnostic(Diagnostic::Level Level,
                                             const DiagnosticInfo &Info) {
+  llvm::SmallString<100> StrC;
+  Info.FormatDiagnostic(StrC);
+  std::string Str(StrC.begin(), StrC.end());
   switch (Level) {
   default: assert(0 && "Diagnostic not handled during diagnostic buffering!");
   case Diagnostic::Note:
-    Notes.push_back(std::make_pair(Info.getLocation().getLocation(),
-                                   FormatDiagnostic(Info)));
+    Notes.push_back(std::make_pair(Info.getLocation().getLocation(), Str));
     break;
   case Diagnostic::Warning:
-    Warnings.push_back(std::make_pair(Info.getLocation().getLocation(),
-                                      FormatDiagnostic(Info)));
+    Warnings.push_back(std::make_pair(Info.getLocation().getLocation(), Str));
     break;
   case Diagnostic::Error:
-    Errors.push_back(std::make_pair(Info.getLocation().getLocation(),
-                                    FormatDiagnostic(Info)));
+    Errors.push_back(std::make_pair(Info.getLocation().getLocation(), Str));
     break;
   }
 }

diff --git a/lib/Driver/TextDiagnosticPrinter.cpp b/lib/Driver/TextDiagnosticPrinter.cpp
index 3b8ec10ec81c7323b19764b8275142a2c4f95c89..cfbe8a7f927df89c987cc4e84f5e26de9557de96 100644 (file)
--- a/lib/Driver/TextDiagnosticPrinter.cpp
+++ b/lib/Driver/TextDiagnosticPrinter.cpp
@@ -15,6 +15,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/ADT/SmallString.h"
 using namespace clang;
 
 void TextDiagnosticPrinter::
@@ -138,13 +139,15 @@ void TextDiagnosticPrinter::HandleDiagnostic(Diagnostic::Level Level,
   case Diagnostic::Warning: OS << "warning: "; break;
   case Diagnostic::Error:   OS << "error: "; break;
   case Diagnostic::Fatal:   OS << "fatal error: "; break;
-    break;
   }
   
-  OS << FormatDiagnostic(Info) << "\n";
+  llvm::SmallString<100> OutStr;
+  Info.FormatDiagnostic(OutStr);
+  OS.write(OutStr.begin(), OutStr.size());
+  OS << '\n';
   
-  if (CaretDiagnostics && Pos.isValid() && ((LastLoc != Pos) ||
-                                            Info.getNumRanges())) {
+  if (CaretDiagnostics && Pos.isValid() &&
+      ((LastLoc != Pos) || Info.getNumRanges())) {
     // Cache the LastLoc, it allows us to omit duplicate source/caret spewage.
     LastLoc = Pos;