[llvm-commits] [llvm] r172086 - in /llvm/trunk: include/llvm/Support/SourceMgr.h lib/Support/SourceMgr.cpp
David Blaikie
dblaikie at gmail.com
Thu Jan 10 11:04:41 PST 2013
On Thu, Jan 10, 2013 at 10:50 AM, Jordan Rose <jordan_rose at apple.com> wrote:
> Author: jrose
> Date: Thu Jan 10 12:50:15 2013
> New Revision: 172086
>
> URL: http://llvm.org/viewvc/llvm-project?rev=172086&view=rev
> Log:
> Add basic fix-its to SMDiagnostic.
>
> Like Clang's FixItHint, SMFixIt represents an insertion, replacement, or
> removal of source text. One or more fix-its can be emitted as part of
> a diagnostic, and will be printed below the source range line to show the
> user how they can fix their code.
>
> Currently, the only client of SMFixIt is clang-tblgen; thus, the tests for
> this behavior live in clang/test/TableGen/tg-fixits.td. If/when SMFixIt is
> adopted within LLVM itself, those tests should be moved to the LLVM suite.
a unit test in LLVM mighte be suitable here - but I realize the
overhead/hassle of adding one might not be worthwhile/a priority
>
> Modified:
> llvm/trunk/include/llvm/Support/SourceMgr.h
> llvm/trunk/lib/Support/SourceMgr.cpp
>
> Modified: llvm/trunk/include/llvm/Support/SourceMgr.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/SourceMgr.h?rev=172086&r1=172085&r2=172086&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/Support/SourceMgr.h (original)
> +++ llvm/trunk/include/llvm/Support/SourceMgr.h Thu Jan 10 12:50:15 2013
> @@ -17,6 +17,8 @@
> #define LLVM_SUPPORT_SOURCEMGR_H
>
> #include "llvm/ADT/ArrayRef.h"
> +#include "llvm/ADT/StringRef.h"
> +#include "llvm/ADT/Twine.h"
> #include "llvm/Support/SMLoc.h"
> #include <string>
>
> @@ -24,6 +26,7 @@
> class MemoryBuffer;
> class SourceMgr;
> class SMDiagnostic;
> + class SMFixIt;
> class Twine;
> class raw_ostream;
>
> @@ -143,6 +146,7 @@
> /// the default error handler is used.
> void PrintMessage(SMLoc Loc, DiagKind Kind, const Twine &Msg,
> ArrayRef<SMRange> Ranges = ArrayRef<SMRange>(),
> + ArrayRef<SMFixIt> FixIts = ArrayRef<SMFixIt>(),
> bool ShowColors = true) const;
>
>
> @@ -152,7 +156,8 @@
> /// @param Msg If non-null, the kind of message (e.g., "error") which is
> /// prefixed to the message.
> SMDiagnostic GetMessage(SMLoc Loc, DiagKind Kind, const Twine &Msg,
> - ArrayRef<SMRange> Ranges = ArrayRef<SMRange>()) const;
> + ArrayRef<SMRange> Ranges = ArrayRef<SMRange>(),
> + ArrayRef<SMFixIt> FixIts = ArrayRef<SMFixIt>()) const;
>
> /// PrintIncludeStack - Prints the names of included files and the line of the
> /// file they were included from. A diagnostic handler can use this before
> @@ -164,6 +169,38 @@
> };
>
>
> +/// Represents a single fixit, a replacement of one range of text with another.
> +class SMFixIt {
> + SMRange Range;
> +
> + std::string Text;
> +
> +public:
> + // FIXME: Twine.str() is not very efficient.
> + SMFixIt(SMLoc Loc, const Twine &Insertion)
> + : Range(Loc, Loc), Text(Insertion.str()) {
> + assert(Loc.isValid());
> + }
> +
> + // FIXME: Twine.str() is not very efficient.
> + SMFixIt(SMRange R, const Twine &Replacement)
> + : Range(R), Text(Replacement.str()) {
> + assert(R.isValid());
> + }
> +
> + StringRef getText() const { return Text; }
> + SMRange getRange() const { return Range; }
> +
> + bool operator<(const SMFixIt &Other) const {
> + if (Range.Start.getPointer() != Other.Range.Start.getPointer())
> + return Range.Start.getPointer() < Other.Range.Start.getPointer();
> + if (Range.End.getPointer() != Other.Range.End.getPointer())
> + return Range.End.getPointer() < Other.Range.End.getPointer();
> + return Text < Other.Text;
> + }
> +};
> +
> +
> /// SMDiagnostic - Instances of this class encapsulate one diagnostic report,
> /// allowing printing to a raw_ostream as a caret diagnostic.
> class SMDiagnostic {
> @@ -174,35 +211,46 @@
> SourceMgr::DiagKind Kind;
> std::string Message, LineContents;
> std::vector<std::pair<unsigned, unsigned> > Ranges;
> + SmallVector<SMFixIt, 4> FixIts;
>
> public:
> // Null diagnostic.
> SMDiagnostic()
> : SM(0), LineNo(0), ColumnNo(0), Kind(SourceMgr::DK_Error) {}
> // Diagnostic with no location (e.g. file not found, command line arg error).
> - SMDiagnostic(const std::string &filename, SourceMgr::DiagKind Knd,
> - const std::string &Msg)
> + SMDiagnostic(StringRef filename, SourceMgr::DiagKind Knd, StringRef Msg)
> : SM(0), Filename(filename), LineNo(-1), ColumnNo(-1), Kind(Knd),
> Message(Msg) {}
>
> // Diagnostic with a location.
> - SMDiagnostic(const SourceMgr &sm, SMLoc L, const std::string &FN,
> + SMDiagnostic(const SourceMgr &sm, SMLoc L, StringRef FN,
> int Line, int Col, SourceMgr::DiagKind Kind,
> - const std::string &Msg, const std::string &LineStr,
> - ArrayRef<std::pair<unsigned,unsigned> > Ranges);
> + StringRef Msg, StringRef LineStr,
> + ArrayRef<std::pair<unsigned,unsigned> > Ranges,
> + ArrayRef<SMFixIt> FixIts = ArrayRef<SMFixIt>());
>
> const SourceMgr *getSourceMgr() const { return SM; }
> SMLoc getLoc() const { return Loc; }
> - const std::string &getFilename() const { return Filename; }
> + StringRef getFilename() const { return Filename; }
> int getLineNo() const { return LineNo; }
> int getColumnNo() const { return ColumnNo; }
> SourceMgr::DiagKind getKind() const { return Kind; }
> - const std::string &getMessage() const { return Message; }
> - const std::string &getLineContents() const { return LineContents; }
> - const std::vector<std::pair<unsigned, unsigned> > &getRanges() const {
> + StringRef getMessage() const { return Message; }
> + StringRef getLineContents() const { return LineContents; }
> + ArrayRef<std::pair<unsigned, unsigned> > getRanges() const {
> return Ranges;
> }
> - void print(const char *ProgName, raw_ostream &S, bool ShowColors = true) const;
> +
> + void addFixIt(const SMFixIt &Hint) {
> + FixIts.push_back(Hint);
> + }
> +
> + ArrayRef<SMFixIt> getFixIts() const {
> + return FixIts;
> + }
> +
> + void print(const char *ProgName, raw_ostream &S,
> + bool ShowColors = true) const;
> };
>
> } // end llvm namespace
>
> Modified: llvm/trunk/lib/Support/SourceMgr.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/SourceMgr.cpp?rev=172086&r1=172085&r2=172086&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Support/SourceMgr.cpp (original)
> +++ llvm/trunk/lib/Support/SourceMgr.cpp Thu Jan 10 12:50:15 2013
> @@ -15,12 +15,16 @@
>
> #include "llvm/Support/SourceMgr.h"
> #include "llvm/ADT/OwningPtr.h"
> +#include "llvm/ADT/SmallString.h"
> #include "llvm/ADT/Twine.h"
> +#include "llvm/Support/Locale.h"
> #include "llvm/Support/MemoryBuffer.h"
> #include "llvm/Support/raw_ostream.h"
> #include "llvm/Support/system_error.h"
> using namespace llvm;
>
> +static const size_t TabStop = 8;
> +
> namespace {
> struct LineNoCacheTy {
> int LastQueryBufferID;
> @@ -146,7 +150,8 @@
> /// prefixed to the message.
> SMDiagnostic SourceMgr::GetMessage(SMLoc Loc, SourceMgr::DiagKind Kind,
> const Twine &Msg,
> - ArrayRef<SMRange> Ranges) const {
> + ArrayRef<SMRange> Ranges,
> + ArrayRef<SMFixIt> FixIts) const {
>
> // First thing to do: find the current buffer containing the specified
> // location to pull out the source line.
> @@ -193,6 +198,7 @@
> R.End = SMLoc::getFromPointer(LineEnd);
>
> // Translate from SMLoc ranges to column ranges.
> + // FIXME: Handle multibyte characters.
> ColRanges.push_back(std::make_pair(R.Start.getPointer()-LineStart,
> R.End.getPointer()-LineStart));
> }
> @@ -202,13 +208,13 @@
>
> return SMDiagnostic(*this, Loc, BufferID, LineAndCol.first,
> LineAndCol.second-1, Kind, Msg.str(),
> - LineStr, ColRanges);
> + LineStr, ColRanges, FixIts);
> }
>
> void SourceMgr::PrintMessage(SMLoc Loc, SourceMgr::DiagKind Kind,
> const Twine &Msg, ArrayRef<SMRange> Ranges,
> - bool ShowColors) const {
> - SMDiagnostic Diagnostic = GetMessage(Loc, Kind, Msg, Ranges);
> + ArrayRef<SMFixIt> FixIts, bool ShowColors) const {
> + SMDiagnostic Diagnostic = GetMessage(Loc, Kind, Msg, Ranges, FixIts);
>
> // Report the message with the diagnostic handler if present.
> if (DiagHandler) {
> @@ -231,15 +237,104 @@
> // SMDiagnostic Implementation
> //===----------------------------------------------------------------------===//
>
> -SMDiagnostic::SMDiagnostic(const SourceMgr &sm, SMLoc L, const std::string &FN,
> +SMDiagnostic::SMDiagnostic(const SourceMgr &sm, SMLoc L, StringRef FN,
> int Line, int Col, SourceMgr::DiagKind Kind,
> - const std::string &Msg,
> - const std::string &LineStr,
> - ArrayRef<std::pair<unsigned,unsigned> > Ranges)
> + StringRef Msg, StringRef LineStr,
> + ArrayRef<std::pair<unsigned,unsigned> > Ranges,
> + ArrayRef<SMFixIt> Hints)
> : SM(&sm), Loc(L), Filename(FN), LineNo(Line), ColumnNo(Col), Kind(Kind),
> - Message(Msg), LineContents(LineStr), Ranges(Ranges.vec()) {
> + Message(Msg), LineContents(LineStr), Ranges(Ranges.vec()),
> + FixIts(Hints.begin(), Hints.end()) {
> + std::sort(FixIts.begin(), FixIts.end());
> +}
> +
> +void buildFixItLine(std::string &CaretLine, std::string &FixItLine,
> + ArrayRef<SMFixIt> FixIts, ArrayRef<char> SourceLine) {
> + if (FixIts.empty())
> + return;
> +
> + const char *LineStart = SourceLine.begin();
> + const char *LineEnd = SourceLine.end();
> +
> + size_t PrevHintEndCol = 0;
> +
> + for (ArrayRef<SMFixIt>::iterator I = FixIts.begin(), E = FixIts.end();
> + I != E; ++I) {
> + // If the fixit contains a newline or tab, ignore it.
> + if (I->getText().find_first_of("\n\r\t") != StringRef::npos)
> + continue;
> +
> + SMRange R = I->getRange();
> +
> + // If the line doesn't contain any part of the range, then ignore it.
> + if (R.Start.getPointer() > LineEnd || R.End.getPointer() < LineStart)
> + continue;
> +
> + // Translate from SMLoc to column.
> + // Ignore pieces of the range that go onto other lines.
> + // FIXME: Handle multibyte characters in the source line.
> + unsigned FirstCol;
> + if (R.Start.getPointer() < LineStart)
> + FirstCol = 0;
> + else
> + FirstCol = R.Start.getPointer() - LineStart;
> +
> + // If we inserted a long previous hint, push this one forwards, and add
> + // an extra space to show that this is not part of the previous
> + // completion. This is sort of the best we can do when two hints appear
> + // to overlap.
> + //
> + // Note that if this hint is located immediately after the previous
> + // hint, no space will be added, since the location is more important.
> + unsigned HintCol = FirstCol;
> + if (HintCol < PrevHintEndCol)
> + HintCol = PrevHintEndCol + 1;
> +
> + // FIXME: This assertion is intended to catch unintended use of multibyte
> + // characters in fixits. If we decide to do this, we'll have to track
> + // separate byte widths for the source and fixit lines.
> + assert((size_t)llvm::sys::locale::columnWidth(I->getText()) ==
> + I->getText().size());
> +
> + // This relies on one byte per column in our fixit hints.
> + unsigned LastColumnModified = HintCol + I->getText().size();
> + if (LastColumnModified > FixItLine.size())
> + FixItLine.resize(LastColumnModified, ' ');
> +
> + std::copy(I->getText().begin(), I->getText().end(),
> + FixItLine.begin() + HintCol);
> +
> + PrevHintEndCol = LastColumnModified;
> +
> + // For replacements, mark the removal range with '~'.
> + // FIXME: Handle multibyte characters in the source line.
> + unsigned LastCol;
> + if (R.End.getPointer() >= LineEnd)
> + LastCol = LineEnd - LineStart;
> + else
> + LastCol = R.End.getPointer() - LineStart;
> +
> + std::fill(&CaretLine[FirstCol], &CaretLine[LastCol], '~');
> + }
> }
>
> +static void printSourceLine(raw_ostream &S, StringRef LineContents) {
> + // Print out the source line one character at a time, so we can expand tabs.
> + for (unsigned i = 0, e = LineContents.size(), OutCol = 0; i != e; ++i) {
> + if (LineContents[i] != '\t') {
> + S << LineContents[i];
> + ++OutCol;
> + continue;
> + }
> +
> + // If we have a tab, emit at least one space, then round up to 8 columns.
> + do {
> + S << ' ';
> + ++OutCol;
> + } while ((OutCol % TabStop) != 0);
> + }
> + S << '\n';
> +}
>
> void SMDiagnostic::print(const char *ProgName, raw_ostream &S,
> bool ShowColors) const {
> @@ -297,43 +392,49 @@
> if (LineNo == -1 || ColumnNo == -1)
> return;
>
> + // FIXME: If there are multibyte characters in the source, all our ranges will
> + // be wrong. To do this properly, we'll need a byte-to-column map like Clang's
> + // TextDiagnostic. For now, we'll just handle tabs by expanding them later,
> + // and bail out rather than show incorrect ranges and misaligned fixits for
> + // any other odd characters.
> + SmallString<128> PrintableLine(LineContents);
> + std::replace(PrintableLine.begin(), PrintableLine.end(), '\t', ' ');
> + size_t NumColumns = (size_t)llvm::sys::locale::columnWidth(PrintableLine);
> + if (NumColumns != PrintableLine.size()) {
> + printSourceLine(S, LineContents);
> + return;
> + }
> +
> // Build the line with the caret and ranges.
> - std::string CaretLine(LineContents.size()+1, ' ');
> + std::string CaretLine(NumColumns+1, ' ');
>
> // Expand any ranges.
> for (unsigned r = 0, e = Ranges.size(); r != e; ++r) {
> std::pair<unsigned, unsigned> R = Ranges[r];
> - for (unsigned i = R.first,
> - e = std::min(R.second, (unsigned)LineContents.size()); i != e; ++i)
> - CaretLine[i] = '~';
> + std::fill(&CaretLine[R.first],
> + &CaretLine[std::min((size_t)R.second, CaretLine.size())],
> + '~');
> }
> -
> +
> + // Add any fix-its.
> + // FIXME: Find the beginning of the line properly for multibyte characters.
> + std::string FixItInsertionLine;
> + buildFixItLine(CaretLine, FixItInsertionLine, FixIts,
> + makeArrayRef(Loc.getPointer() - ColumnNo,
> + LineContents.size()));
> +
> // Finally, plop on the caret.
> - if (unsigned(ColumnNo) <= LineContents.size())
> + if (unsigned(ColumnNo) <= NumColumns)
> CaretLine[ColumnNo] = '^';
> else
> - CaretLine[LineContents.size()] = '^';
> + CaretLine[NumColumns] = '^';
>
> // ... and remove trailing whitespace so the output doesn't wrap for it. We
> // know that the line isn't completely empty because it has the caret in it at
> // least.
> CaretLine.erase(CaretLine.find_last_not_of(' ')+1);
>
> - // Print out the source line one character at a time, so we can expand tabs.
> - for (unsigned i = 0, e = LineContents.size(), OutCol = 0; i != e; ++i) {
> - if (LineContents[i] != '\t') {
> - S << LineContents[i];
> - ++OutCol;
> - continue;
> - }
> -
> - // If we have a tab, emit at least one space, then round up to 8 columns.
> - do {
> - S << ' ';
> - ++OutCol;
> - } while (OutCol & 7);
> - }
> - S << '\n';
> + printSourceLine(S, LineContents);
>
> if (ShowColors)
> S.changeColor(raw_ostream::GREEN, true);
> @@ -350,11 +451,36 @@
> do {
> S << CaretLine[i];
> ++OutCol;
> - } while (OutCol & 7);
> + } while ((OutCol % TabStop) != 0);
> }
> + S << '\n';
>
> if (ShowColors)
> S.resetColor();
> +
> + // Print out the replacement line, matching tabs in the source line.
> + if (FixItInsertionLine.empty())
> + return;
>
> + for (size_t i = 0, e = FixItInsertionLine.size(), OutCol = 0; i != e; ++i) {
> + if (i >= LineContents.size() || LineContents[i] != '\t') {
> + S << FixItInsertionLine[i];
> + ++OutCol;
> + continue;
> + }
> +
> + // Okay, we have a tab. Insert the appropriate number of characters.
> + do {
> + S << FixItInsertionLine[i];
> + // FIXME: This is trying not to break up replacements, but then to re-sync
> + // with the tabs between replacements. This will fail, though, if two
> + // fix-it replacements are exactly adjacent, or if a fix-it contains a
> + // space. Really we should be precomputing column widths, which we'll
> + // need anyway for multibyte chars.
> + if (FixItInsertionLine[i] != ' ')
> + ++i;
> + ++OutCol;
> + } while (((OutCol % TabStop) != 0) && i != e);
> + }
> S << '\n';
> }
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list