Separate the notion of 'context' when recursing down in the parser and actual errors.

Summary:
Change how error messages are constructed and stored in Diagnostics.
Separate the notion of 'context' when recursing down in the parser and actual errors.
This will simplify adding some new features, like argument overloading and error recovery.

Reviewers: klimek

CC: cfe-commits, revane

Differential Revision: http://llvm-reviews.chandlerc.com/D1168

llvm-svn: 186602
diff --git a/clang/lib/ASTMatchers/Dynamic/Diagnostics.cpp b/clang/lib/ASTMatchers/Dynamic/Diagnostics.cpp
index 1ca1f36..71aeb9a 100644
--- a/clang/lib/ASTMatchers/Dynamic/Diagnostics.cpp
+++ b/clang/lib/ASTMatchers/Dynamic/Diagnostics.cpp
@@ -13,20 +13,56 @@
 namespace ast_matchers {
 namespace dynamic {
 
-Diagnostics::ArgStream &
-Diagnostics::ArgStream::operator<<(const Twine &Arg) {
+Diagnostics::ArgStream Diagnostics::pushContextFrame(ContextType Type,
+                                                     SourceRange Range) {
+  ContextStack.push_back(ContextFrame());
+  ContextFrame& data = ContextStack.back();
+  data.Type = Type;
+  data.Range = Range;
+  return ArgStream(&data.Args);
+}
+
+Diagnostics::Context::Context(ConstructMatcherEnum, Diagnostics *Error,
+                              StringRef MatcherName,
+                              const SourceRange &MatcherRange)
+    : Error(Error) {
+  Error->pushContextFrame(CT_MatcherConstruct, MatcherRange) << MatcherName;
+}
+
+Diagnostics::Context::Context(MatcherArgEnum, Diagnostics *Error,
+                              StringRef MatcherName,
+                              const SourceRange &MatcherRange,
+                              unsigned ArgNumber)
+    : Error(Error) {
+  Error->pushContextFrame(CT_MatcherArg, MatcherRange) << ArgNumber
+                                                       << MatcherName;
+}
+
+Diagnostics::Context::~Context() { Error->ContextStack.pop_back(); }
+
+Diagnostics::ArgStream &Diagnostics::ArgStream::operator<<(const Twine &Arg) {
   Out->push_back(Arg.str());
   return *this;
 }
 
-Diagnostics::ArgStream Diagnostics::pushErrorFrame(const SourceRange &Range,
-                                                   ErrorType Error) {
-  Frames.insert(Frames.begin(), ErrorFrame());
-  ErrorFrame &Last = Frames.front();
+Diagnostics::ArgStream Diagnostics::addError(const SourceRange &Range,
+                                             ErrorType Error) {
+  Errors.push_back(ErrorContent());
+  ErrorContent &Last = Errors.back();
+  Last.ContextStack = ContextStack;
   Last.Range = Range;
   Last.Type = Error;
-  ArgStream Out = { &Last.Args };
-  return Out;
+  return ArgStream(&Last.Args);
+}
+
+StringRef ContextTypeToString(Diagnostics::ContextType Type) {
+  switch (Type) {
+    case Diagnostics::CT_MatcherConstruct:
+      return "Error building matcher $0.";
+    case Diagnostics::CT_MatcherArg:
+      return "Error parsing argument $0 for matcher $1.";
+  }
+  llvm_unreachable("Unknown ContextType value.");
 }
 
 StringRef ErrorTypeToString(Diagnostics::ErrorType Type) {
@@ -42,10 +78,6 @@
 
   case Diagnostics::ET_ParserStringError:
     return "Error parsing string token: <$0>";
-  case Diagnostics::ET_ParserMatcherArgFailure:
-    return "Error parsing argument $0 for matcher $1.";
-  case Diagnostics::ET_ParserMatcherFailure:
-    return "Error building matcher $0.";
   case Diagnostics::ET_ParserNoOpenParen:
     return "Error parsing matcher. Found token <$0> while looking for '('.";
   case Diagnostics::ET_ParserNoCloseParen:
@@ -95,25 +127,42 @@
   return Out;
 }
 
-std::string Diagnostics::ErrorFrame::ToString() const {
-  StringRef FormatString = ErrorTypeToString(Type);
-  std::string ErrorOut = FormatErrorString(FormatString, Args);
+static std::string MaybeAddLineAndColumn(Twine Input,
+                                         const SourceRange &Range) {
   if (Range.Start.Line > 0 && Range.Start.Column > 0)
     return (Twine(Range.Start.Line) + ":" + Twine(Range.Start.Column) + ": " +
-            ErrorOut).str();
-  return ErrorOut;
+            Input).str();
+  return Input.str();
+}
+
+std::string Diagnostics::ContextFrame::ToString() const {
+  return MaybeAddLineAndColumn(
+      FormatErrorString(ContextTypeToString(Type), Args), Range);
+}
+
+std::string Diagnostics::ErrorContent::ToString() const {
+  return MaybeAddLineAndColumn(FormatErrorString(ErrorTypeToString(Type), Args),
+                               Range);
 }
 
 std::string Diagnostics::ToString() const {
-  if (Frames.empty()) return "";
-  return Frames[Frames.size() - 1].ToString();
+  std::string Result;
+  for (size_t i = 0, e = Errors.size(); i != e; ++i) {
+    if (i != 0) Result += "\n";
+    Result += Errors[i].ToString();
+  }
+  return Result;
 }
 
 std::string Diagnostics::ToStringFull() const {
   std::string Result;
-  for (size_t i = 0, end = Frames.size(); i != end; ++i) {
-    if (i > 0) Result += "\n";
-    Result += Frames[i].ToString();
+  for (size_t i = 0, e = Errors.size(); i != e; ++i) {
+    if (i != 0) Result += "\n";
+    const ErrorContent &Error = Errors[i];
+    for (size_t i = 0, e = Error.ContextStack.size(); i != e; ++i) {
+      Result += Error.ContextStack[i].ToString() + "\n";
+    }
+    Result += Error.ToString();
   }
   return Result;
 }
diff --git a/clang/lib/ASTMatchers/Dynamic/Marshallers.h b/clang/lib/ASTMatchers/Dynamic/Marshallers.h
index 3aa55d0..9f1cfe3 100644
--- a/clang/lib/ASTMatchers/Dynamic/Marshallers.h
+++ b/clang/lib/ASTMatchers/Dynamic/Marshallers.h
@@ -152,14 +152,14 @@
 /// \brief Helper macros to check the arguments on all marshaller functions.
 #define CHECK_ARG_COUNT(count)                                                 \
   if (Args.size() != count) {                                                  \
-    Error->pushErrorFrame(NameRange, Error->ET_RegistryWrongArgCount)          \
+    Error->addError(NameRange, Error->ET_RegistryWrongArgCount)                \
         << count << Args.size();                                               \
     return MatcherList();                                                      \
   }
 
 #define CHECK_ARG_TYPE(index, type)                                            \
   if (!ArgTypeTraits<type>::is(Args[index].Value)) {                           \
-    Error->pushErrorFrame(Args[index].Range, Error->ET_RegistryWrongArgType)   \
+    Error->addError(Args[index].Range, Error->ET_RegistryWrongArgType)         \
         << (index + 1) << ArgTypeTraits<type>::asString()                      \
         << Args[index].Value.getTypeAsString();                                \
     return MatcherList();                                                      \
@@ -254,7 +254,7 @@
     const ParserValue &Arg = Args[i];
     const VariantValue &Value = Arg.Value;
     if (!ArgTraits::is(Value)) {
-      Error->pushErrorFrame(Arg.Range, Error->ET_RegistryWrongArgType)
+      Error->addError(Arg.Range, Error->ET_RegistryWrongArgType)
           << (i + 1) << ArgTraits::asString() << Value.getTypeAsString();
       HasError = true;
       break;
diff --git a/clang/lib/ASTMatchers/Dynamic/Parser.cpp b/clang/lib/ASTMatchers/Dynamic/Parser.cpp
index fc09a30..54424ce 100644
--- a/clang/lib/ASTMatchers/Dynamic/Parser.cpp
+++ b/clang/lib/ASTMatchers/Dynamic/Parser.cpp
@@ -162,8 +162,7 @@
       SourceRange Range;
       Range.Start = Result->Range.Start;
       Range.End = currentLocation();
-      Error->pushErrorFrame(Range, Error->ET_ParserUnsignedError)
-          << Result->Text;
+      Error->addError(Range, Error->ET_ParserUnsignedError) << Result->Text;
       Result->Kind = TokenInfo::TK_Error;
     }
   }
@@ -198,8 +197,7 @@
     SourceRange Range;
     Range.Start = Result->Range.Start;
     Range.End = currentLocation();
-    Error->pushErrorFrame(Range, Error->ET_ParserStringError)
-        << ErrorText;
+    Error->addError(Range, Error->ET_ParserStringError) << ErrorText;
     Result->Kind = TokenInfo::TK_Error;
   }
 
@@ -239,7 +237,7 @@
   assert(NameToken.Kind == TokenInfo::TK_Ident);
   const TokenInfo OpenToken = Tokenizer->consumeNextToken();
   if (OpenToken.Kind != TokenInfo::TK_OpenParen) {
-    Error->pushErrorFrame(OpenToken.Range, Error->ET_ParserNoOpenParen)
+    Error->addError(OpenToken.Range, Error->ET_ParserNoOpenParen)
         << OpenToken.Text;
     return false;
   }
@@ -256,27 +254,24 @@
       // We must find a , token to continue.
       const TokenInfo CommaToken = Tokenizer->consumeNextToken();
       if (CommaToken.Kind != TokenInfo::TK_Comma) {
-        Error->pushErrorFrame(CommaToken.Range, Error->ET_ParserNoComma)
+        Error->addError(CommaToken.Range, Error->ET_ParserNoComma)
             << CommaToken.Text;
         return false;
       }
     }
 
+    Diagnostics::Context Ctx(Diagnostics::Context::MatcherArg, Error,
+                             NameToken.Text, NameToken.Range, Args.size() + 1);
     ParserValue ArgValue;
     ArgValue.Text = Tokenizer->peekNextToken().Text;
     ArgValue.Range = Tokenizer->peekNextToken().Range;
-    if (!parseExpressionImpl(&ArgValue.Value)) {
-      Error->pushErrorFrame(NameToken.Range,
-                            Error->ET_ParserMatcherArgFailure)
-          << (Args.size() + 1) << NameToken.Text;
-      return false;
-    }
+    if (!parseExpressionImpl(&ArgValue.Value)) return false;
 
     Args.push_back(ArgValue);
   }
 
   if (EndToken.Kind == TokenInfo::TK_Eof) {
-    Error->pushErrorFrame(OpenToken.Range, Error->ET_ParserNoCloseParen);
+    Error->addError(OpenToken.Range, Error->ET_ParserNoCloseParen);
     return false;
   }
 
@@ -293,35 +288,32 @@
     //       explicit about the syntax error.
     if (BindToken.Kind != TokenInfo::TK_Ident ||
         BindToken.Text != TokenInfo::ID_Bind) {
-      Error->pushErrorFrame(BindToken.Range, Error->ET_ParserMalformedBindExpr);
+      Error->addError(BindToken.Range, Error->ET_ParserMalformedBindExpr);
       return false;
     }
     if (OpenToken.Kind != TokenInfo::TK_OpenParen) {
-      Error->pushErrorFrame(OpenToken.Range, Error->ET_ParserMalformedBindExpr);
+      Error->addError(OpenToken.Range, Error->ET_ParserMalformedBindExpr);
       return false;
     }
     if (IDToken.Kind != TokenInfo::TK_Literal || !IDToken.Value.isString()) {
-      Error->pushErrorFrame(IDToken.Range, Error->ET_ParserMalformedBindExpr);
+      Error->addError(IDToken.Range, Error->ET_ParserMalformedBindExpr);
       return false;
     }
     if (CloseToken.Kind != TokenInfo::TK_CloseParen) {
-      Error->pushErrorFrame(CloseToken.Range,
-                            Error->ET_ParserMalformedBindExpr);
+      Error->addError(CloseToken.Range, Error->ET_ParserMalformedBindExpr);
       return false;
     }
     BindID = IDToken.Value.getString();
   }
 
   // Merge the start and end infos.
+  Diagnostics::Context Ctx(Diagnostics::Context::ConstructMatcher, Error,
+                           NameToken.Text, NameToken.Range);
   SourceRange MatcherRange = NameToken.Range;
   MatcherRange.End = EndToken.Range.End;
   MatcherList Result = S->actOnMatcherExpression(
       NameToken.Text, MatcherRange, BindID, Args, Error);
-  if (Result.empty()) {
-    Error->pushErrorFrame(NameToken.Range, Error->ET_ParserMatcherFailure)
-        << NameToken.Text;
-    return false;
-  }
+  if (Result.empty()) return false;
 
   *Value = Result;
   return true;
@@ -338,8 +330,8 @@
     return parseMatcherExpressionImpl(Value);
 
   case TokenInfo::TK_Eof:
-    Error->pushErrorFrame(Tokenizer->consumeNextToken().Range,
-                          Error->ET_ParserNoCode);
+    Error->addError(Tokenizer->consumeNextToken().Range,
+                    Error->ET_ParserNoCode);
     return false;
 
   case TokenInfo::TK_Error:
@@ -352,8 +344,7 @@
   case TokenInfo::TK_Period:
   case TokenInfo::TK_InvalidChar:
     const TokenInfo Token = Tokenizer->consumeNextToken();
-    Error->pushErrorFrame(Token.Range, Error->ET_ParserInvalidToken)
-        << Token.Text;
+    Error->addError(Token.Range, Error->ET_ParserInvalidToken) << Token.Text;
     return false;
   }
 
@@ -392,8 +383,8 @@
   CodeTokenizer Tokenizer(Code, Error);
   if (!Parser(&Tokenizer, S, Error).parseExpressionImpl(Value)) return false;
   if (Tokenizer.peekNextToken().Kind != TokenInfo::TK_Eof) {
-    Error->pushErrorFrame(Tokenizer.peekNextToken().Range,
-                          Error->ET_ParserTrailingCode);
+    Error->addError(Tokenizer.peekNextToken().Range,
+                    Error->ET_ParserTrailingCode);
     return false;
   }
   return true;
@@ -412,11 +403,11 @@
   if (!parseExpression(Code, S, &Value, Error))
     return NULL;
   if (!Value.isMatchers()) {
-    Error->pushErrorFrame(SourceRange(), Error->ET_ParserNotAMatcher);
+    Error->addError(SourceRange(), Error->ET_ParserNotAMatcher);
     return NULL;
   }
   if (Value.getMatchers().matchers().size() != 1) {
-    Error->pushErrorFrame(SourceRange(), Error->ET_ParserOverloadedType)
+    Error->addError(SourceRange(), Error->ET_ParserOverloadedType)
         << Value.getTypeAsString();
     return NULL;
   }
diff --git a/clang/lib/ASTMatchers/Dynamic/Registry.cpp b/clang/lib/ASTMatchers/Dynamic/Registry.cpp
index f523cc4..6cab407 100644
--- a/clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ b/clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -298,8 +298,7 @@
   ConstructorMap::const_iterator it =
       RegistryData->constructors().find(MatcherName);
   if (it == RegistryData->constructors().end()) {
-    Error->pushErrorFrame(NameRange, Error->ET_RegistryNotFound)
-        << MatcherName;
+    Error->addError(NameRange, Error->ET_RegistryNotFound) << MatcherName;
     return MatcherList();
   }
 
@@ -322,7 +321,7 @@
       return *Bound;
     }
   }
-  Error->pushErrorFrame(NameRange, Error->ET_RegistryNotBindable);
+  Error->addError(NameRange, Error->ET_RegistryNotBindable);
   return MatcherList();
 }