[PATCH] Separate the notion of 'context' when recursing down in the parser and actual errors.
Samuel Benzaquen
sbenza at google.com
Thu Jul 18 11:46:56 PDT 2013
================
Comment at: include/clang/ASTMatchers/Dynamic/Diagnostics.h:132
@@ +131,3 @@
+
+ /// \brief Inforation stored for each error found.
+ struct ErrorContent {
----------------
Manuel Klimek wrote:
> *Information
Done.
================
Comment at: include/clang/ASTMatchers/Dynamic/Diagnostics.h:145
@@ -102,1 +144,3 @@
+ ///
+ /// Each error only show the error message without any context.
std::string ToString() const;
----------------
Manuel Klimek wrote:
> *shows
Done.
================
Comment at: include/clang/ASTMatchers/Dynamic/Diagnostics.h:146
@@ -102,1 +145,3 @@
+ /// Each error only show the error message without any context.
std::string ToString() const;
+ /// \brief Returns the full string representation of each error.
----------------
Manuel Klimek wrote:
> Nit: I personally prefer some more vertical whitespace.
Done.
================
Comment at: lib/ASTMatchers/Dynamic/Diagnostics.cpp:18-22
@@ +17,7 @@
+ SourceRange Range) {
+ ContextStack.push_back(ContextFrame());
+ ContextFrame& data = ContextStack.back();
+ data.Type = Type;
+ data.Range = Range;
+ return ArgStream(&data.Args);
+}
----------------
Manuel Klimek wrote:
> Not really important I think, but I'd probably make ContextFrame to have a constructor and a method to get the ArgStream, so that this method would become:
> ContextStack.push_back(ContextFrame(Type, Range));
> return ContextStack.bac().getArgStream()
I would have to add a default constructor too, and then the struct would have too much logic and I would be inclined to make it a class.
All possible to do, but since they are only created here, I didn't think was necessary.
================
Comment at: lib/ASTMatchers/Dynamic/Diagnostics.cpp:130
@@ -97,5 +129,3 @@
-std::string Diagnostics::ErrorFrame::ToString() const {
- StringRef FormatString = ErrorTypeToString(Type);
- std::string ErrorOut = FormatErrorString(FormatString, Args);
+static std::string MaybeAddLineCol(Twine Input, const SourceRange& Range) {
if (Range.Start.Line > 0 && Range.Start.Column > 0)
----------------
Manuel Klimek wrote:
> /me cringes; any reason not to spend the additional 3 characters to make that Column?
Done.
http://llvm-reviews.chandlerc.com/D1168
BRANCH
context
ARCANIST PROJECT
clang
More information about the cfe-commits
mailing list