From: Chris Lattner Date: Wed, 19 Nov 2008 06:51:40 +0000 (+0000) Subject: rewrite FormatDiagnostic to be less gross and a lot more efficient. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=f4c839657742b823cea1a95b18422f1ba74d3ddd;p=clang 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 7742307d47..303d83cc66 100644 --- a/include/clang/Basic/Diagnostic.h +++ b/include/clang/Basic/Diagnostic.h @@ -18,6 +18,10 @@ #include #include +namespace llvm { + template 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 &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 af9df0d0c8..c98ad4c58b 100644 --- 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 aed83ea84f..29ea8c0e37 100644 --- 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 - 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 35d454b1b9..e8c24ab0a7 100644 --- 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 +#include "llvm/ADT/SmallVector.h" #include #include #include @@ -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 &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 b138b1a24d..5032cdb0b5 100644 --- 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 3b8ec10ec8..cfbe8a7f92 100644 --- 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;