]> granicus.if.org Git - clang/commitdiff
Separate the notion of 'context' when recursing down in the parser and actual errors.
authorSamuel Benzaquen <sbenza@google.com>
Thu, 18 Jul 2013 19:47:59 +0000 (19:47 +0000)
committerSamuel Benzaquen <sbenza@google.com>
Thu, 18 Jul 2013 19:47:59 +0000 (19:47 +0000)
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

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

include/clang/ASTMatchers/Dynamic/Diagnostics.h
lib/ASTMatchers/Dynamic/Diagnostics.cpp
lib/ASTMatchers/Dynamic/Marshallers.h
lib/ASTMatchers/Dynamic/Parser.cpp
lib/ASTMatchers/Dynamic/Registry.cpp

index 973d54cead309ac9d6b363d63a8925c125713d56..49527e059e47cbbb7438fa3abbf9045b3d710d55 100644 (file)
@@ -49,7 +49,13 @@ struct ParserValue {
 
 /// \brief Helper class to manage error messages.
 class Diagnostics {
- public:
+public:
+  /// \brief Parser context types.
+  enum ContextType {
+    CT_MatcherArg = 0,
+    CT_MatcherConstruct = 1
+  };
+
   /// \brief All errors from the system.
   enum ErrorType {
     ET_None = 0,
@@ -60,51 +66,96 @@ class Diagnostics {
     ET_RegistryNotBindable = 4,
 
     ET_ParserStringError = 100,
-    ET_ParserMatcherArgFailure = 101,
-    ET_ParserMatcherFailure = 102,
-    ET_ParserNoOpenParen = 103,
-    ET_ParserNoCloseParen = 104,
-    ET_ParserNoComma = 105,
-    ET_ParserNoCode = 106,
-    ET_ParserNotAMatcher = 107,
-    ET_ParserInvalidToken = 108,
-    ET_ParserMalformedBindExpr = 109,
-    ET_ParserTrailingCode = 110,
-    ET_ParserUnsignedError = 111,
-    ET_ParserOverloadedType = 112
+    ET_ParserNoOpenParen = 101,
+    ET_ParserNoCloseParen = 102,
+    ET_ParserNoComma = 103,
+    ET_ParserNoCode = 104,
+    ET_ParserNotAMatcher = 105,
+    ET_ParserInvalidToken = 106,
+    ET_ParserMalformedBindExpr = 107,
+    ET_ParserTrailingCode = 108,
+    ET_ParserUnsignedError = 109,
+    ET_ParserOverloadedType = 110
   };
 
   /// \brief Helper stream class.
-  struct ArgStream {
+  class ArgStream {
+  public:
+    ArgStream(std::vector<std::string> *Out) : Out(Out) {}
     template <class T> ArgStream &operator<<(const T &Arg) {
       return operator<<(Twine(Arg));
     }
     ArgStream &operator<<(const Twine &Arg);
+
+  private:
     std::vector<std::string> *Out;
   };
 
-  /// \brief Push a frame to the beginning of the list
+  /// \brief Class defining a parser context.
+  ///
+  /// Used by the parser to specify (possibly recursive) contexts where the
+  /// parsing/construction can fail. Any error triggered within a context will
+  /// keep information about the context chain.
+  /// This class should be used as a RAII instance in the stack.
+  struct Context {
+  public:
+    /// \brief About to call the constructor for a matcher.
+    enum ConstructMatcherEnum { ConstructMatcher };
+    Context(ConstructMatcherEnum, Diagnostics *Error, StringRef MatcherName,
+            const SourceRange &MatcherRange);
+    /// \brief About to recurse into parsing one argument for a matcher.
+    enum MatcherArgEnum { MatcherArg };
+    Context(MatcherArgEnum, Diagnostics *Error, StringRef MatcherName,
+            const SourceRange &MatcherRange, unsigned ArgNumber);
+    ~Context();
+
+  private:
+    Diagnostics *const Error;
+  };
+
+  /// \brief Add an error to the diagnostics.
   ///
-  /// Returns a helper class to allow the caller to pass the arguments for the
+  /// All the context information will be kept on the error message.
+  /// \return a helper class to allow the caller to pass the arguments for the
   /// error message, using the << operator.
-  ArgStream pushErrorFrame(const SourceRange &Range, ErrorType Error);
+  ArgStream addError(const SourceRange &Range, ErrorType Error);
+
+  /// \brief Information stored for one frame of the context.
+  struct ContextFrame {
+    ContextType Type;
+    SourceRange Range;
+    std::vector<std::string> Args;
+
+    std::string ToString() const;
+  };
 
-  struct ErrorFrame {
+  /// \brief Information stored for each error found.
+  struct ErrorContent {
+    std::vector<ContextFrame> ContextStack;
     SourceRange Range;
     ErrorType Type;
     std::vector<std::string> Args;
 
     std::string ToString() const;
   };
-  ArrayRef<ErrorFrame> frames() const { return Frames; }
+  ArrayRef<ErrorContent> errors() const { return Errors; }
 
-  /// \brief Returns a string representation of the last frame.
+  /// \brief Returns a simple string representation of each error.
+  ///
+  /// Each error only shows the error message without any context.
   std::string ToString() const;
-  /// \brief Returns a string representation of the whole frame stack.
+
+  /// \brief Returns the full string representation of each error.
+  ///
+  /// Each error message contains the full context.
   std::string ToStringFull() const;
 
- private:
-   std::vector<ErrorFrame> Frames;
+private:
+  /// \brief Helper function used by the constructors of ContextFrame.
+  ArgStream pushContextFrame(ContextType Type, SourceRange Range);
+
+  std::vector<ContextFrame> ContextStack;
+  std::vector<ErrorContent> Errors;
 };
 
 }  // namespace dynamic
index 1ca1f3653985c931fa145f7c72b6810f4a5c2dfa..71aeb9a754047d0f4a61557d00481baeec1956da 100644 (file)
@@ -13,20 +13,56 @@ namespace clang {
 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 @@ StringRef ErrorTypeToString(Diagnostics::ErrorType Type) {
 
   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 @@ std::string FormatErrorString(StringRef FormatString,
   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;
 }
index 3aa55d0a5a57dc742bcebbba35513af966c2efb9..9f1cfe3443fee5f8acf162169703984546d8071e 100644 (file)
@@ -152,14 +152,14 @@ private:
 /// \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 @@ MatcherList variadicMatcherCreateCallback(StringRef MatcherName,
     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;
index fc09a30ddd9928ae3b4bb7b9bc6f33ab3b1bae2f..54424cea7e21a9e5bfa815399916b46cb5ffe5d7 100644 (file)
@@ -162,8 +162,7 @@ private:
       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 @@ private:
     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 @@ bool Parser::parseMatcherExpressionImpl(VariantValue *Value) {
   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 @@ bool Parser::parseMatcherExpressionImpl(VariantValue *Value) {
       // 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 @@ bool Parser::parseMatcherExpressionImpl(VariantValue *Value) {
     //       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 @@ bool Parser::parseExpressionImpl(VariantValue *Value) {
     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 @@ bool Parser::parseExpressionImpl(VariantValue *Value) {
   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 @@ bool Parser::parseExpression(StringRef Code, Sema *S,
   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 @@ DynTypedMatcher *Parser::parseMatcherExpression(StringRef Code,
   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;
   }
index f523cc4abb09570a5020f88eb919dbaf851bcf51..6cab4073b962b5e7b8e6bf54b771dc2c6d8a968a 100644 (file)
@@ -298,8 +298,7 @@ MatcherList Registry::constructMatcher(StringRef MatcherName,
   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 @@ MatcherList Registry::constructBoundMatcher(StringRef MatcherName,
       return *Bound;
     }
   }
-  Error->pushErrorFrame(NameRange, Error->ET_RegistryNotBindable);
+  Error->addError(NameRange, Error->ET_RegistryNotBindable);
   return MatcherList();
 }