[llvm] r366217 - [Remarks] Simplify and refactor the RemarkParser interface
Francis Visoiu Mistrih via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 16 08:25:06 PDT 2019
Author: thegameg
Date: Tue Jul 16 08:25:05 2019
New Revision: 366217
URL: http://llvm.org/viewvc/llvm-project?rev=366217&view=rev
Log:
[Remarks] Simplify and refactor the RemarkParser interface
Before, everything was based on some kind of type erased parser
implementation which container a lot of boilerplate code when multiple
formats were to be supported.
This simplifies it by:
* the remark now owns its arguments
* *always* returning an error from the implementation side
* working around the way the YAML parser reports errors: catch them through
callbacks and re-insert them in a proper llvm::Error
* add a CParser wrapper that is used when implementing the C API to
avoid cluttering the C++ API with useless state
* LLVMRemarkParserGetNext now returns an object that needs to be
released to avoid leaking resources
* add a new API to dispose of a remark entry: LLVMRemarkEntryDispose
Removed:
llvm/trunk/lib/Remarks/RemarkParserImpl.h
Modified:
llvm/trunk/docs/Remarks.rst
llvm/trunk/include/llvm-c/Remarks.h
llvm/trunk/include/llvm/IR/RemarkStreamer.h
llvm/trunk/include/llvm/Remarks/Remark.h
llvm/trunk/include/llvm/Remarks/RemarkParser.h
llvm/trunk/include/llvm/Support/SourceMgr.h
llvm/trunk/lib/IR/RemarkStreamer.cpp
llvm/trunk/lib/Remarks/Remark.cpp
llvm/trunk/lib/Remarks/RemarkParser.cpp
llvm/trunk/lib/Remarks/YAMLRemarkParser.cpp
llvm/trunk/lib/Remarks/YAMLRemarkParser.h
llvm/trunk/tools/llvm-opt-report/OptReport.cpp
llvm/trunk/tools/remarks-shlib/Remarks.exports
llvm/trunk/unittests/Remarks/YAMLRemarksParsingTest.cpp
Modified: llvm/trunk/docs/Remarks.rst
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/docs/Remarks.rst?rev=366217&r1=366216&r2=366217&view=diff
==============================================================================
--- llvm/trunk/docs/Remarks.rst (original)
+++ llvm/trunk/docs/Remarks.rst Tue Jul 16 08:25:05 2019
@@ -295,6 +295,7 @@ The typical usage through the C API is l
LLVMRemarkEntryRef Remark = NULL;
while ((Remark = LLVMRemarkParserGetNext(Parser))) {
// use Remark
+ LLVMRemarkEntryDispose(Remark); // Release memory.
}
bool HasError = LLVMRemarkParserHasError(Parser);
LLVMRemarkParserDispose(Parser);
Modified: llvm/trunk/include/llvm-c/Remarks.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm-c/Remarks.h?rev=366217&r1=366216&r2=366217&view=diff
==============================================================================
--- llvm/trunk/include/llvm-c/Remarks.h (original)
+++ llvm/trunk/include/llvm-c/Remarks.h Tue Jul 16 08:25:05 2019
@@ -137,6 +137,13 @@ extern LLVMRemarkDebugLocRef LLVMRemarkA
typedef struct LLVMRemarkOpaqueEntry *LLVMRemarkEntryRef;
/**
+ * Free the resources used by the remark entry.
+ *
+ * \since REMARKS_API_VERSION=0
+ */
+extern void LLVMRemarkEntryDispose(LLVMRemarkEntryRef Remark);
+
+/**
* The type of the remark. For example, it can allow users to only keep the
* missed optimizations from the compiler.
*
@@ -161,7 +168,7 @@ extern LLVMRemarkStringRef
LLVMRemarkEntryGetRemarkName(LLVMRemarkEntryRef Remark);
/**
- * Get the name of the function being processsed when the remark was emitted.
+ * Get the name of the function being processed when the remark was emitted.
*
* \since REMARKS_API_VERSION=0
*/
@@ -199,6 +206,8 @@ extern uint32_t LLVMRemarkEntryGetNumArg
*
* If there are no arguments in \p Remark, the return value will be `NULL`.
*
+ * The lifetime of the returned value is bound to the lifetime of \p Remark.
+ *
* \since REMARKS_API_VERSION=0
*/
extern LLVMRemarkArgRef LLVMRemarkEntryGetFirstArg(LLVMRemarkEntryRef Remark);
@@ -208,6 +217,8 @@ extern LLVMRemarkArgRef LLVMRemarkEntryG
*
* Returns `NULL` if there are no more arguments available.
*
+ * The lifetime of the returned value is bound to the lifetime of \p Remark.
+ *
* \since REMARKS_API_VERSION=0
*/
extern LLVMRemarkArgRef LLVMRemarkEntryGetNextArg(LLVMRemarkArgRef It,
@@ -232,8 +243,11 @@ extern LLVMRemarkParserRef LLVMRemarkPar
/**
* Returns the next remark in the file.
*
- * The value pointed to by the return value is invalidated by the next call to
- * LLVMRemarkParserGetNext().
+ * The value pointed to by the return value needs to be disposed using a call to
+ * LLVMRemarkEntryDispose().
+ *
+ * All the entries in the returned value that are of LLVMRemarkStringRef type
+ * will become invalidated once a call to LLVMRemarkParserDispose is made.
*
* If the parser reaches the end of the buffer, the return value will be `NULL`.
*
@@ -258,8 +272,9 @@ extern LLVMRemarkParserRef LLVMRemarkPar
* ```
* LLVMRemarkParserRef Parser = LLVMRemarkParserCreateYAML(Buf, Size);
* LLVMRemarkEntryRef Remark = NULL;
- * while ((Remark == LLVMRemarkParserGetNext(Parser))) {
+ * while ((Remark = LLVMRemarkParserGetNext(Parser))) {
* // use Remark
+ * LLVMRemarkEntryDispose(Remark); // Release memory.
* }
* bool HasError = LLVMRemarkParserHasError(Parser);
* LLVMRemarkParserDispose(Parser);
Modified: llvm/trunk/include/llvm/IR/RemarkStreamer.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/RemarkStreamer.h?rev=366217&r1=366216&r2=366217&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/RemarkStreamer.h (original)
+++ llvm/trunk/include/llvm/IR/RemarkStreamer.h Tue Jul 16 08:25:05 2019
@@ -32,15 +32,9 @@ class RemarkStreamer {
/// The object used to serialize the remarks to a specific format.
std::unique_ptr<remarks::Serializer> Serializer;
- /// Temporary buffer for converting diagnostics into remark objects. This is
- /// used for the remark arguments that are converted from a vector of
- /// diagnostic arguments to a vector of remark arguments.
- SmallVector<remarks::Argument, 8> TmpArgs;
- /// Convert diagnostics into remark objects. The result uses \p TmpArgs as a
- /// temporary buffer for the remark arguments, and relies on all the strings
- /// to be kept in memory until the next call to `toRemark`.
- /// The lifetime of the members of the result is bound to the lifetime of both
- /// the remark streamer and the LLVM diagnostics.
+ /// Convert diagnostics into remark objects.
+ /// The lifetime of the members of the result is bound to the lifetime of
+ /// the LLVM diagnostics.
remarks::Remark toRemark(const DiagnosticInfoOptimizationBase &Diag);
public:
Modified: llvm/trunk/include/llvm/Remarks/Remark.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Remarks/Remark.h?rev=366217&r1=366216&r2=366217&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Remarks/Remark.h (original)
+++ llvm/trunk/include/llvm/Remarks/Remark.h Tue Jul 16 08:25:05 2019
@@ -85,10 +85,23 @@ struct Remark {
Optional<uint64_t> Hotness;
/// Arguments collected via the streaming interface.
- ArrayRef<Argument> Args;
+ SmallVector<Argument, 5> Args;
+
+ Remark() = default;
+ Remark(Remark &&) = default;
+ Remark &operator=(Remark &&) = default;
/// Return a message composed from the arguments as a string.
std::string getArgsAsMsg() const;
+
+ /// Clone this remark to explicitly ask for a copy.
+ Remark clone() const { return *this; }
+
+private:
+ /// In order to avoid unwanted copies, "delete" the copy constructor.
+ /// If a copy is needed, it should be done through `Remark::clone()`.
+ Remark(const Remark &) = default;
+ Remark& operator=(const Remark &) = default;
};
// Create wrappers for C Binding types (see CBindingWrapping.h).
Modified: llvm/trunk/include/llvm/Remarks/RemarkParser.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Remarks/RemarkParser.h?rev=366217&r1=366216&r2=366217&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Remarks/RemarkParser.h (original)
+++ llvm/trunk/include/llvm/Remarks/RemarkParser.h Tue Jul 16 08:25:05 2019
@@ -26,27 +26,33 @@ namespace remarks {
struct ParserImpl;
struct ParsedStringTable;
+class EndOfFileError : public ErrorInfo<EndOfFileError> {
+public:
+ static char ID;
+
+ EndOfFileError() {}
+
+ void log(raw_ostream &OS) const override { OS << "End of file reached."; }
+ std::error_code convertToErrorCode() const override {
+ return inconvertibleErrorCode();
+ }
+};
+
/// Parser used to parse a raw buffer to remarks::Remark objects.
struct Parser {
- /// The hidden implementation of the parser.
- std::unique_ptr<ParserImpl> Impl;
+ /// The format of the parser.
+ Format ParserFormat;
+
+ Parser(Format ParserFormat) : ParserFormat(ParserFormat) {}
- /// Create a parser parsing \p Buffer to Remark objects.
- /// This constructor should be only used for parsing remarks without a string
- /// table.
- Parser(Format ParserFormat, StringRef Buffer);
-
- /// Create a parser parsing \p Buffer to Remark objects, using \p StrTab as a
- /// string table.
- Parser(Format ParserFormat, StringRef Buffer,
- const ParsedStringTable &StrTab);
-
- // Needed because ParserImpl is an incomplete type.
- ~Parser();
-
- /// Returns an empty Optional if it reached the end.
- /// Returns a valid remark otherwise.
- Expected<const Remark *> getNext() const;
+ /// If no error occurs, this returns a valid Remark object.
+ /// If an error of type EndOfFileError occurs, it is safe to recover from it
+ /// by stopping the parsing.
+ /// If any other error occurs, it should be propagated to the user.
+ /// The pointer should never be null.
+ virtual Expected<std::unique_ptr<Remark>> next() = 0;
+
+ virtual ~Parser() = default;
};
/// In-memory representation of the string table parsed from a buffer (e.g. the
@@ -61,6 +67,10 @@ struct ParsedStringTable {
ParsedStringTable(StringRef Buffer);
};
+Expected<std::unique_ptr<Parser>>
+createRemarkParser(Format ParserFormat, StringRef Buf,
+ Optional<const ParsedStringTable *> StrTab = None);
+
} // end namespace remarks
} // end namespace llvm
Modified: llvm/trunk/include/llvm/Support/SourceMgr.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/SourceMgr.h?rev=366217&r1=366216&r2=366217&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/SourceMgr.h (original)
+++ llvm/trunk/include/llvm/Support/SourceMgr.h Tue Jul 16 08:25:05 2019
@@ -106,6 +106,8 @@ public:
SourceMgr() = default;
SourceMgr(const SourceMgr &) = delete;
SourceMgr &operator=(const SourceMgr &) = delete;
+ SourceMgr(SourceMgr &&) = default;
+ SourceMgr &operator=(SourceMgr &&) = default;
~SourceMgr() = default;
void setIncludeDirs(const std::vector<std::string> &Dirs) {
Modified: llvm/trunk/lib/IR/RemarkStreamer.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/RemarkStreamer.cpp?rev=366217&r1=366216&r2=366217&view=diff
==============================================================================
--- llvm/trunk/lib/IR/RemarkStreamer.cpp (original)
+++ llvm/trunk/lib/IR/RemarkStreamer.cpp Tue Jul 16 08:25:05 2019
@@ -72,9 +72,6 @@ toRemarkLocation(const DiagnosticLocatio
/// LLVM Diagnostic -> Remark
remarks::Remark
RemarkStreamer::toRemark(const DiagnosticInfoOptimizationBase &Diag) {
- // Re-use the buffer.
- TmpArgs.clear();
-
remarks::Remark R; // The result.
R.RemarkType = toRemarkType(static_cast<DiagnosticKind>(Diag.getKind()));
R.PassName = Diag.getPassName();
@@ -84,15 +81,12 @@ RemarkStreamer::toRemark(const Diagnosti
R.Loc = toRemarkLocation(Diag.getLocation());
R.Hotness = Diag.getHotness();
- // Use TmpArgs to build the list of arguments and re-use the memory allocated
- // from previous remark conversions.
for (const DiagnosticInfoOptimizationBase::Argument &Arg : Diag.getArgs()) {
- TmpArgs.emplace_back();
- TmpArgs.back().Key = Arg.Key;
- TmpArgs.back().Val = Arg.Val;
- TmpArgs.back().Loc = toRemarkLocation(Arg.Loc);
+ R.Args.emplace_back();
+ R.Args.back().Key = Arg.Key;
+ R.Args.back().Val = Arg.Val;
+ R.Args.back().Loc = toRemarkLocation(Arg.Loc);
}
- R.Args = TmpArgs; // This is valid until the next call to this function.
return R;
}
Modified: llvm/trunk/lib/Remarks/Remark.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Remarks/Remark.cpp?rev=366217&r1=366216&r2=366217&view=diff
==============================================================================
--- llvm/trunk/lib/Remarks/Remark.cpp (original)
+++ llvm/trunk/lib/Remarks/Remark.cpp Tue Jul 16 08:25:05 2019
@@ -66,6 +66,10 @@ LLVMRemarkArgGetDebugLoc(LLVMRemarkArgRe
return nullptr;
}
+extern "C" void LLVMRemarkEntryDispose(LLVMRemarkEntryRef Remark) {
+ delete unwrap(Remark);
+}
+
extern "C" LLVMRemarkType LLVMRemarkEntryGetType(LLVMRemarkEntryRef Remark) {
// Assume here that the enums can be converted both ways.
return static_cast<LLVMRemarkType>(unwrap(Remark)->RemarkType);
Modified: llvm/trunk/lib/Remarks/RemarkParser.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Remarks/RemarkParser.cpp?rev=366217&r1=366216&r2=366217&view=diff
==============================================================================
--- llvm/trunk/lib/Remarks/RemarkParser.cpp (original)
+++ llvm/trunk/lib/Remarks/RemarkParser.cpp Tue Jul 16 08:25:05 2019
@@ -20,69 +20,7 @@
using namespace llvm;
using namespace llvm::remarks;
-static std::unique_ptr<ParserImpl> formatToParserImpl(Format ParserFormat,
- StringRef Buf) {
- switch (ParserFormat) {
- case Format::YAML:
- return llvm::make_unique<YAMLParserImpl>(Buf);
- case Format::Unknown:
- llvm_unreachable("Unhandled llvm::remarks::ParserFormat enum");
- return nullptr;
- };
-}
-
-static std::unique_ptr<ParserImpl>
-formatToParserImpl(Format ParserFormat, StringRef Buf,
- const ParsedStringTable &StrTab) {
- switch (ParserFormat) {
- case Format::YAML:
- return llvm::make_unique<YAMLParserImpl>(Buf, &StrTab);
- case Format::Unknown:
- llvm_unreachable("Unhandled llvm::remarks::ParserFormat enum");
- return nullptr;
- };
-}
-
-Parser::Parser(Format ParserFormat, StringRef Buf)
- : Impl(formatToParserImpl(ParserFormat, Buf)) {}
-
-Parser::Parser(Format ParserFormat, StringRef Buf,
- const ParsedStringTable &StrTab)
- : Impl(formatToParserImpl(ParserFormat, Buf, StrTab)) {}
-
-Parser::~Parser() = default;
-
-static Expected<const Remark *> getNextYAML(YAMLParserImpl &Impl) {
- YAMLRemarkParser &YAMLParser = Impl.YAMLParser;
- // Check for EOF.
- if (Impl.YAMLIt == Impl.YAMLParser.Stream.end())
- return nullptr;
-
- auto CurrentIt = Impl.YAMLIt;
-
- // Try to parse an entry.
- if (Error E = YAMLParser.parseYAMLElement(*CurrentIt)) {
- // Set the iterator to the end, in case the user calls getNext again.
- Impl.YAMLIt = Impl.YAMLParser.Stream.end();
- return std::move(E);
- }
-
- // Move on.
- ++Impl.YAMLIt;
-
- // Return the just-parsed remark.
- if (const Optional<YAMLRemarkParser::ParseState> &State = YAMLParser.State)
- return &State->TheRemark;
- else
- return createStringError(std::make_error_code(std::errc::invalid_argument),
- "unexpected error while parsing.");
-}
-
-Expected<const Remark *> Parser::getNext() const {
- if (auto *Impl = dyn_cast<YAMLParserImpl>(this->Impl.get()))
- return getNextYAML(*Impl);
- llvm_unreachable("Get next called with an unknown parsing implementation.");
-}
+char EndOfFileError::ID = 0;
ParsedStringTable::ParsedStringTable(StringRef InBuffer) : Buffer(InBuffer) {
while (!InBuffer.empty()) {
@@ -109,59 +47,70 @@ Expected<StringRef> ParsedStringTable::o
return StringRef(Buffer.data() + Offset, NextOffset - Offset - 1);
}
+Expected<std::unique_ptr<Parser>>
+llvm::remarks::createRemarkParser(Format ParserFormat, StringRef Buf,
+ Optional<const ParsedStringTable *> StrTab) {
+ switch (ParserFormat) {
+ case Format::YAML:
+ return llvm::make_unique<YAMLRemarkParser>(Buf, StrTab);
+ case Format::Unknown:
+ return createStringError(std::make_error_code(std::errc::invalid_argument),
+ "Unknown remark parser format.");
+ }
+}
+
+// Wrapper that holds the state needed to interact with the C API.
+struct CParser {
+ std::unique_ptr<Parser> TheParser;
+ Optional<std::string> Err;
+
+ CParser(Format ParserFormat, StringRef Buf,
+ Optional<const ParsedStringTable *> StrTab = None)
+ : TheParser(cantFail(createRemarkParser(ParserFormat, Buf, StrTab))) {}
+
+ void handleError(Error E) { Err.emplace(toString(std::move(E))); }
+ bool hasError() const { return Err.hasValue(); }
+ const char *getMessage() const { return Err ? Err->c_str() : nullptr; };
+};
+
// Create wrappers for C Binding types (see CBindingWrapping.h).
-DEFINE_SIMPLE_CONVERSION_FUNCTIONS(remarks::Parser, LLVMRemarkParserRef)
+DEFINE_SIMPLE_CONVERSION_FUNCTIONS(CParser, LLVMRemarkParserRef)
extern "C" LLVMRemarkParserRef LLVMRemarkParserCreateYAML(const void *Buf,
uint64_t Size) {
- return wrap(new remarks::Parser(
- remarks::Format::YAML, StringRef(static_cast<const char *>(Buf), Size)));
-}
-
-static void handleYAMLError(remarks::YAMLParserImpl &Impl, Error E) {
- handleAllErrors(
- std::move(E),
- [&](const YAMLParseError &PE) {
- Impl.YAMLParser.Stream.printError(&PE.getNode(),
- Twine(PE.getMessage()) + Twine('\n'));
- },
- [&](const ErrorInfoBase &EIB) { EIB.log(Impl.YAMLParser.ErrorStream); });
- Impl.HasErrors = true;
+ return wrap(new CParser(Format::YAML,
+ StringRef(static_cast<const char *>(Buf), Size)));
}
extern "C" LLVMRemarkEntryRef
LLVMRemarkParserGetNext(LLVMRemarkParserRef Parser) {
- remarks::Parser &TheParser = *unwrap(Parser);
+ CParser &TheCParser = *unwrap(Parser);
+ remarks::Parser &TheParser = *TheCParser.TheParser;
- Expected<const remarks::Remark *> RemarkOrErr = TheParser.getNext();
- if (!RemarkOrErr) {
- // Error during parsing.
- if (auto *Impl = dyn_cast<remarks::YAMLParserImpl>(TheParser.Impl.get()))
- handleYAMLError(*Impl, RemarkOrErr.takeError());
- else
- llvm_unreachable("unkown parser implementation.");
+ Expected<std::unique_ptr<Remark>> MaybeRemark = TheParser.next();
+ if (Error E = MaybeRemark.takeError()) {
+ if (E.isA<EndOfFileError>()) {
+ consumeError(std::move(E));
+ return nullptr;
+ }
+
+ // Handle the error. Allow it to be checked through HasError and
+ // GetErrorMessage.
+ TheCParser.handleError(std::move(E));
return nullptr;
}
- if (*RemarkOrErr == nullptr)
- return nullptr;
// Valid remark.
- return wrap(*RemarkOrErr);
+ return wrap(MaybeRemark->release());
}
extern "C" LLVMBool LLVMRemarkParserHasError(LLVMRemarkParserRef Parser) {
- if (auto *Impl =
- dyn_cast<remarks::YAMLParserImpl>(unwrap(Parser)->Impl.get()))
- return Impl->HasErrors;
- llvm_unreachable("unkown parser implementation.");
+ return unwrap(Parser)->hasError();
}
extern "C" const char *
LLVMRemarkParserGetErrorMessage(LLVMRemarkParserRef Parser) {
- if (auto *Impl =
- dyn_cast<remarks::YAMLParserImpl>(unwrap(Parser)->Impl.get()))
- return Impl->YAMLParser.ErrorStream.str().c_str();
- llvm_unreachable("unkown parser implementation.");
+ return unwrap(Parser)->getMessage();
}
extern "C" void LLVMRemarkParserDispose(LLVMRemarkParserRef Parser) {
Removed: llvm/trunk/lib/Remarks/RemarkParserImpl.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Remarks/RemarkParserImpl.h?rev=366216&view=auto
==============================================================================
--- llvm/trunk/lib/Remarks/RemarkParserImpl.h (original)
+++ llvm/trunk/lib/Remarks/RemarkParserImpl.h (removed)
@@ -1,33 +0,0 @@
-//===-- RemarkParserImpl.h - Implementation details -------------*- C++/-*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-//
-// This file provides implementation details for the remark parser.
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLVM_REMARKS_REMARK_PARSER_IMPL_H
-#define LLVM_REMARKS_REMARK_PARSER_IMPL_H
-
-#include "llvm/Remarks/RemarkParser.h"
-
-namespace llvm {
-namespace remarks {
-/// This is used as a base for any parser implementation.
-struct ParserImpl {
- explicit ParserImpl(Format ParserFormat) : ParserFormat(ParserFormat) {}
- // Virtual destructor prevents mismatched deletes
- virtual ~ParserImpl() {}
-
- // The parser format. This is used as a tag to safely cast between
- // implementations.
- Format ParserFormat;
-};
-} // end namespace remarks
-} // end namespace llvm
-
-#endif /* LLVM_REMARKS_REMARK_PARSER_IMPL_H */
Modified: llvm/trunk/lib/Remarks/YAMLRemarkParser.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Remarks/YAMLRemarkParser.cpp?rev=366217&r1=366216&r2=366217&view=diff
==============================================================================
--- llvm/trunk/lib/Remarks/YAMLRemarkParser.cpp (original)
+++ llvm/trunk/lib/Remarks/YAMLRemarkParser.cpp Tue Jul 16 08:25:05 2019
@@ -20,255 +20,308 @@ using namespace llvm::remarks;
char YAMLParseError::ID = 0;
-Error YAMLRemarkParser::parseKey(StringRef &Result, yaml::KeyValueNode &Node) {
- if (auto *Key = dyn_cast<yaml::ScalarNode>(Node.getKey())) {
- Result = Key->getRawValue();
+static void handleDiagnostic(const SMDiagnostic &Diag, void *Ctx) {
+ assert(Ctx && "Expected non-null Ctx in diagnostic handler.");
+ std::string &Message = *static_cast<std::string *>(Ctx);
+ assert(Message.empty() && "Expected an empty string.");
+ raw_string_ostream OS(Message);
+ Diag.print(/*ProgName=*/nullptr, OS, /*ShowColors*/ false,
+ /*ShowKindLabels*/ true);
+ OS << '\n';
+ OS.flush();
+}
+
+YAMLParseError::YAMLParseError(StringRef Msg, SourceMgr &SM,
+ yaml::Stream &Stream, yaml::Node &Node) {
+ // 1) Set up a diagnostic handler to avoid errors being printed out to
+ // stderr.
+ // 2) Use the stream to print the error with the associated node.
+ // 3) The stream will use the source manager to print the error, which will
+ // call the diagnostic handler.
+ // 4) The diagnostic handler will stream the error directly into this object's
+ // Message member, which is used when logging is asked for.
+ auto OldDiagHandler = SM.getDiagHandler();
+ auto OldDiagCtx = SM.getDiagContext();
+ SM.setDiagHandler(handleDiagnostic, &Message);
+ Stream.printError(&Node, Twine(Msg) + Twine('\n'));
+ // Restore the old handlers.
+ SM.setDiagHandler(OldDiagHandler, OldDiagCtx);
+}
+
+static SourceMgr setupSM(std::string &LastErrorMessage) {
+ SourceMgr SM;
+ SM.setDiagHandler(handleDiagnostic, &LastErrorMessage);
+ return SM;
+}
+
+YAMLRemarkParser::YAMLRemarkParser(StringRef Buf,
+ Optional<const ParsedStringTable *> StrTab)
+ : Parser{Format::YAML}, StrTab(StrTab), LastErrorMessage(),
+ SM(setupSM(LastErrorMessage)), Stream(Buf, SM), YAMLIt(Stream.begin()) {}
+
+Error YAMLRemarkParser::error(StringRef Message, yaml::Node &Node) {
+ return make_error<YAMLParseError>(Message, SM, Stream, Node);
+}
+
+Error YAMLRemarkParser::error() {
+ if (LastErrorMessage.empty())
return Error::success();
+ Error E = make_error<YAMLParseError>(LastErrorMessage);
+ LastErrorMessage.clear();
+ return E;
+}
+
+Expected<std::unique_ptr<Remark>>
+YAMLRemarkParser::parseRemark(yaml::Document &RemarkEntry) {
+ if (Error E = error())
+ return std::move(E);
+
+ yaml::Node *YAMLRoot = RemarkEntry.getRoot();
+ if (!YAMLRoot) {
+ return createStringError(std::make_error_code(std::errc::invalid_argument),
+ "not a valid YAML file.");
+ }
+
+ auto *Root = dyn_cast<yaml::MappingNode>(YAMLRoot);
+ if (!Root)
+ return error("document root is not of mapping type.", *YAMLRoot);
+
+ std::unique_ptr<Remark> Result = llvm::make_unique<Remark>();
+ Remark &TheRemark = *Result;
+
+ // First, the type. It needs special handling since is not part of the
+ // key-value stream.
+ Expected<Type> T = parseType(*Root);
+ if (!T)
+ return T.takeError();
+ else
+ TheRemark.RemarkType = *T;
+
+ // Then, parse the fields, one by one.
+ for (yaml::KeyValueNode &RemarkField : *Root) {
+ Expected<StringRef> MaybeKey = parseKey(RemarkField);
+ if (!MaybeKey)
+ return MaybeKey.takeError();
+ StringRef KeyName = *MaybeKey;
+
+ if (KeyName == "Pass") {
+ if (Expected<StringRef> MaybeStr = parseStr(RemarkField))
+ TheRemark.PassName = *MaybeStr;
+ else
+ return MaybeStr.takeError();
+ } else if (KeyName == "Name") {
+ if (Expected<StringRef> MaybeStr = parseStr(RemarkField))
+ TheRemark.RemarkName = *MaybeStr;
+ else
+ return MaybeStr.takeError();
+ } else if (KeyName == "Function") {
+ if (Expected<StringRef> MaybeStr = parseStr(RemarkField))
+ TheRemark.FunctionName = *MaybeStr;
+ else
+ return MaybeStr.takeError();
+ } else if (KeyName == "Hotness") {
+ if (Expected<unsigned> MaybeU = parseUnsigned(RemarkField))
+ TheRemark.Hotness = *MaybeU;
+ else
+ return MaybeU.takeError();
+ } else if (KeyName == "DebugLoc") {
+ if (Expected<RemarkLocation> MaybeLoc = parseDebugLoc(RemarkField))
+ TheRemark.Loc = *MaybeLoc;
+ else
+ return MaybeLoc.takeError();
+ } else if (KeyName == "Args") {
+ auto *Args = dyn_cast<yaml::SequenceNode>(RemarkField.getValue());
+ if (!Args)
+ return error("wrong value type for key.", RemarkField);
+
+ for (yaml::Node &Arg : *Args) {
+ if (Expected<Argument> MaybeArg = parseArg(Arg))
+ TheRemark.Args.push_back(*MaybeArg);
+ else
+ return MaybeArg.takeError();
+ }
+ } else {
+ return error("unknown key.", RemarkField);
+ }
}
- return make_error<YAMLParseError>("key is not a string.", Node);
+ // Check if any of the mandatory fields are missing.
+ if (TheRemark.RemarkType == Type::Unknown || TheRemark.PassName.empty() ||
+ TheRemark.RemarkName.empty() || TheRemark.FunctionName.empty())
+ return error("Type, Pass, Name or Function missing.",
+ *RemarkEntry.getRoot());
+
+ return std::move(Result);
+}
+
+Expected<Type> YAMLRemarkParser::parseType(yaml::MappingNode &Node) {
+ auto Type = StringSwitch<remarks::Type>(Node.getRawTag())
+ .Case("!Passed", remarks::Type::Passed)
+ .Case("!Missed", remarks::Type::Missed)
+ .Case("!Analysis", remarks::Type::Analysis)
+ .Case("!AnalysisFPCommute", remarks::Type::AnalysisFPCommute)
+ .Case("!AnalysisAliasing", remarks::Type::AnalysisAliasing)
+ .Case("!Failure", remarks::Type::Failure)
+ .Default(remarks::Type::Unknown);
+ if (Type == remarks::Type::Unknown)
+ return error("expected a remark tag.", Node);
+ return Type;
+}
+
+Expected<StringRef> YAMLRemarkParser::parseKey(yaml::KeyValueNode &Node) {
+ if (auto *Key = dyn_cast<yaml::ScalarNode>(Node.getKey()))
+ return Key->getRawValue();
+
+ return error("key is not a string.", Node);
}
-template <typename T>
-Error YAMLRemarkParser::parseStr(T &Result, yaml::KeyValueNode &Node) {
+Expected<StringRef> YAMLRemarkParser::parseStr(yaml::KeyValueNode &Node) {
auto *Value = dyn_cast<yaml::ScalarNode>(Node.getValue());
if (!Value)
- return make_error<YAMLParseError>("expected a value of scalar type.", Node);
- StringRef Tmp;
+ return error("expected a value of scalar type.", Node);
+ StringRef Result;
if (!StrTab) {
- Tmp = Value->getRawValue();
+ Result = Value->getRawValue();
} else {
// If we have a string table, parse it as an unsigned.
unsigned StrID = 0;
- if (Error E = parseUnsigned(StrID, Node))
- return E;
+ if (Expected<unsigned> MaybeStrID = parseUnsigned(Node))
+ StrID = *MaybeStrID;
+ else
+ return MaybeStrID.takeError();
+
if (Expected<StringRef> Str = (**StrTab)[StrID])
- Tmp = *Str;
+ Result = *Str;
else
return Str.takeError();
}
- if (Tmp.front() == '\'')
- Tmp = Tmp.drop_front();
-
- if (Tmp.back() == '\'')
- Tmp = Tmp.drop_back();
+ if (Result.front() == '\'')
+ Result = Result.drop_front();
- Result = Tmp;
+ if (Result.back() == '\'')
+ Result = Result.drop_back();
- return Error::success();
+ return Result;
}
-template <typename T>
-Error YAMLRemarkParser::parseUnsigned(T &Result, yaml::KeyValueNode &Node) {
+Expected<unsigned> YAMLRemarkParser::parseUnsigned(yaml::KeyValueNode &Node) {
SmallVector<char, 4> Tmp;
auto *Value = dyn_cast<yaml::ScalarNode>(Node.getValue());
if (!Value)
- return make_error<YAMLParseError>("expected a value of scalar type.", Node);
+ return error("expected a value of scalar type.", Node);
unsigned UnsignedValue = 0;
if (Value->getValue(Tmp).getAsInteger(10, UnsignedValue))
- return make_error<YAMLParseError>("expected a value of integer type.",
- *Value);
- Result = UnsignedValue;
- return Error::success();
-}
-
-Error YAMLRemarkParser::parseType(Type &Result, yaml::MappingNode &Node) {
- auto Type = StringSwitch<remarks::Type>(Node.getRawTag())
- .Case("!Passed", remarks::Type::Passed)
- .Case("!Missed", remarks::Type::Missed)
- .Case("!Analysis", remarks::Type::Analysis)
- .Case("!AnalysisFPCommute", remarks::Type::AnalysisFPCommute)
- .Case("!AnalysisAliasing", remarks::Type::AnalysisAliasing)
- .Case("!Failure", remarks::Type::Failure)
- .Default(remarks::Type::Unknown);
- if (Type == remarks::Type::Unknown)
- return make_error<YAMLParseError>("expected a remark tag.", Node);
- Result = Type;
- return Error::success();
+ return error("expected a value of integer type.", *Value);
+ return UnsignedValue;
}
-Error YAMLRemarkParser::parseDebugLoc(Optional<RemarkLocation> &Result,
- yaml::KeyValueNode &Node) {
+Expected<RemarkLocation>
+YAMLRemarkParser::parseDebugLoc(yaml::KeyValueNode &Node) {
auto *DebugLoc = dyn_cast<yaml::MappingNode>(Node.getValue());
if (!DebugLoc)
- return make_error<YAMLParseError>("expected a value of mapping type.",
- Node);
+ return error("expected a value of mapping type.", Node);
Optional<StringRef> File;
Optional<unsigned> Line;
Optional<unsigned> Column;
for (yaml::KeyValueNode &DLNode : *DebugLoc) {
- StringRef KeyName;
- if (Error E = parseKey(KeyName, DLNode))
- return E;
+ Expected<StringRef> MaybeKey = parseKey(DLNode);
+ if (!MaybeKey)
+ return MaybeKey.takeError();
+ StringRef KeyName = *MaybeKey;
+
if (KeyName == "File") {
- if (Error E = parseStr(File, DLNode))
- return E;
+ if (Expected<StringRef> MaybeStr = parseStr(DLNode))
+ File = *MaybeStr;
+ else
+ return MaybeStr.takeError();
} else if (KeyName == "Column") {
- if (Error E = parseUnsigned(Column, DLNode))
- return E;
+ if (Expected<unsigned> MaybeU = parseUnsigned(DLNode))
+ Column = *MaybeU;
+ else
+ return MaybeU.takeError();
} else if (KeyName == "Line") {
- if (Error E = parseUnsigned(Line, DLNode))
- return E;
+ if (Expected<unsigned> MaybeU = parseUnsigned(DLNode))
+ Line = *MaybeU;
+ else
+ return MaybeU.takeError();
} else {
- return make_error<YAMLParseError>("unknown entry in DebugLoc map.",
- DLNode);
+ return error("unknown entry in DebugLoc map.", DLNode);
}
}
// If any of the debug loc fields is missing, return an error.
if (!File || !Line || !Column)
- return make_error<YAMLParseError>("DebugLoc node incomplete.", Node);
+ return error("DebugLoc node incomplete.", Node);
- Result = RemarkLocation{*File, *Line, *Column};
-
- return Error::success();
+ return RemarkLocation{*File, *Line, *Column};
}
-Error YAMLRemarkParser::parseRemarkField(yaml::KeyValueNode &RemarkField) {
-
- StringRef KeyName;
- if (Error E = parseKey(KeyName, RemarkField))
- return E;
-
- if (KeyName == "Pass") {
- if (Error E = parseStr(State->TheRemark.PassName, RemarkField))
- return E;
- } else if (KeyName == "Name") {
- if (Error E = parseStr(State->TheRemark.RemarkName, RemarkField))
- return E;
- } else if (KeyName == "Function") {
- if (Error E = parseStr(State->TheRemark.FunctionName, RemarkField))
- return E;
- } else if (KeyName == "Hotness") {
- State->TheRemark.Hotness = 0;
- if (Error E = parseUnsigned(*State->TheRemark.Hotness, RemarkField))
- return E;
- } else if (KeyName == "DebugLoc") {
- if (Error E = parseDebugLoc(State->TheRemark.Loc, RemarkField))
- return E;
- } else if (KeyName == "Args") {
- auto *Args = dyn_cast<yaml::SequenceNode>(RemarkField.getValue());
- if (!Args)
- return make_error<YAMLParseError>("wrong value type for key.",
- RemarkField);
-
- for (yaml::Node &Arg : *Args)
- if (Error E = parseArg(State->Args, Arg))
- return E;
-
- State->TheRemark.Args = State->Args;
- } else {
- return make_error<YAMLParseError>("unknown key.", RemarkField);
- }
-
- return Error::success();
-}
-
-Error YAMLRemarkParser::parseArg(SmallVectorImpl<Argument> &Args,
- yaml::Node &Node) {
+Expected<Argument> YAMLRemarkParser::parseArg(yaml::Node &Node) {
auto *ArgMap = dyn_cast<yaml::MappingNode>(&Node);
if (!ArgMap)
- return make_error<YAMLParseError>("expected a value of mapping type.",
- Node);
+ return error("expected a value of mapping type.", Node);
- StringRef KeyStr;
- StringRef ValueStr;
+ Optional<StringRef> KeyStr;
+ Optional<StringRef> ValueStr;
Optional<RemarkLocation> Loc;
- for (yaml::KeyValueNode &ArgEntry : *ArgMap)
- if (Error E = parseArgEntry(ArgEntry, KeyStr, ValueStr, Loc))
- return E;
-
- if (KeyStr.empty())
- return make_error<YAMLParseError>("argument key is missing.", *ArgMap);
- if (ValueStr.empty())
- return make_error<YAMLParseError>("argument value is missing.", *ArgMap);
-
- Args.push_back(Argument{KeyStr, ValueStr, Loc});
-
- return Error::success();
-}
-
-Error YAMLRemarkParser::parseArgEntry(yaml::KeyValueNode &ArgEntry,
- StringRef &KeyStr, StringRef &ValueStr,
- Optional<RemarkLocation> &Loc) {
- StringRef KeyName;
- if (Error E = parseKey(KeyName, ArgEntry))
- return E;
-
- // Try to parse debug locs.
- if (KeyName == "DebugLoc") {
- // Can't have multiple DebugLoc entries per argument.
- if (Loc)
- return make_error<YAMLParseError>(
- "only one DebugLoc entry is allowed per argument.", ArgEntry);
-
- if (Error E = parseDebugLoc(Loc, ArgEntry))
- return E;
- return Error::success();
- }
+ for (yaml::KeyValueNode &ArgEntry : *ArgMap) {
+ Expected<StringRef> MaybeKey = parseKey(ArgEntry);
+ if (!MaybeKey)
+ return MaybeKey.takeError();
+ StringRef KeyName = *MaybeKey;
+
+ // Try to parse debug locs.
+ if (KeyName == "DebugLoc") {
+ // Can't have multiple DebugLoc entries per argument.
+ if (Loc)
+ return error("only one DebugLoc entry is allowed per argument.",
+ ArgEntry);
+
+ if (Expected<RemarkLocation> MaybeLoc = parseDebugLoc(ArgEntry)) {
+ Loc = *MaybeLoc;
+ continue;
+ } else
+ return MaybeLoc.takeError();
+ }
- // If we already have a string, error out.
- if (!ValueStr.empty())
- return make_error<YAMLParseError>(
- "only one string entry is allowed per argument.", ArgEntry);
-
- // Try to parse a string.
- if (Error E = parseStr(ValueStr, ArgEntry))
- return E;
-
- // Keep the key from the string.
- KeyStr = KeyName;
- return Error::success();
-}
-
-Error YAMLRemarkParser::parseYAMLElement(yaml::Document &Remark) {
- // Parsing a new remark, clear the previous one by re-constructing the state
- // in-place in the Optional.
- State.emplace(TmpArgs);
+ // If we already have a string, error out.
+ if (ValueStr)
+ return error("only one string entry is allowed per argument.", ArgEntry);
+
+ // Try to parse the value.
+ if (Expected<StringRef> MaybeStr = parseStr(ArgEntry))
+ ValueStr = *MaybeStr;
+ else
+ return MaybeStr.takeError();
- yaml::Node *YAMLRoot = Remark.getRoot();
- if (!YAMLRoot)
- return createStringError(std::make_error_code(std::errc::invalid_argument),
- "not a valid YAML file.");
+ // Keep the key from the string.
+ KeyStr = KeyName;
+ }
- auto *Root = dyn_cast<yaml::MappingNode>(YAMLRoot);
- if (!Root)
- return make_error<YAMLParseError>("document root is not of mapping type.",
- *YAMLRoot);
+ if (!KeyStr)
+ return error("argument key is missing.", *ArgMap);
+ if (!ValueStr)
+ return error("argument value is missing.", *ArgMap);
- if (Error E = parseType(State->TheRemark.RemarkType, *Root))
- return E;
+ return Argument{*KeyStr, *ValueStr, Loc};
+}
- for (yaml::KeyValueNode &RemarkField : *Root)
- if (Error E = parseRemarkField(RemarkField))
- return E;
-
- // If the YAML parsing failed, don't even continue parsing. We might
- // encounter malformed YAML.
- if (Stream.failed())
- return make_error<YAMLParseError>("YAML parsing failed.",
- *Remark.getRoot());
+Expected<std::unique_ptr<Remark>> YAMLRemarkParser::next() {
+ if (YAMLIt == Stream.end())
+ return make_error<EndOfFileError>();
+
+ Expected<std::unique_ptr<Remark>> MaybeResult = parseRemark(*YAMLIt);
+ if (!MaybeResult) {
+ // Avoid garbage input, set the iterator to the end.
+ YAMLIt = Stream.end();
+ return MaybeResult.takeError();
+ }
- // Check if any of the mandatory fields are missing.
- if (State->TheRemark.RemarkType == Type::Unknown ||
- State->TheRemark.PassName.empty() ||
- State->TheRemark.RemarkName.empty() ||
- State->TheRemark.FunctionName.empty())
- return make_error<YAMLParseError>("Type, Pass, Name or Function missing.",
- *Remark.getRoot());
-
- return Error::success();
-}
+ ++YAMLIt;
-/// Handle a diagnostic from the YAML stream. Records the error in the
-/// YAMLRemarkParser class.
-void YAMLRemarkParser::HandleDiagnostic(const SMDiagnostic &Diag, void *Ctx) {
- assert(Ctx && "Expected non-null Ctx in diagnostic handler.");
- auto *Parser = static_cast<YAMLRemarkParser *>(Ctx);
- Diag.print(/*ProgName=*/nullptr, Parser->ErrorStream, /*ShowColors*/ false,
- /*ShowKindLabels*/ true);
+ return std::move(*MaybeResult);
}
Modified: llvm/trunk/lib/Remarks/YAMLRemarkParser.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Remarks/YAMLRemarkParser.h?rev=366217&r1=366216&r2=366217&view=diff
==============================================================================
--- llvm/trunk/lib/Remarks/YAMLRemarkParser.h (original)
+++ llvm/trunk/lib/Remarks/YAMLRemarkParser.h Tue Jul 16 08:25:05 2019
@@ -13,7 +13,6 @@
#ifndef LLVM_REMARKS_YAML_REMARK_PARSER_H
#define LLVM_REMARKS_YAML_REMARK_PARSER_H
-#include "RemarkParserImpl.h"
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/Remarks/Remark.h"
@@ -27,112 +26,69 @@
namespace llvm {
namespace remarks {
-/// Parses and holds the state of the latest parsed remark.
-struct YAMLRemarkParser {
- /// Source manager for better error messages.
- SourceMgr SM;
- /// Stream for yaml parsing.
- yaml::Stream Stream;
- /// Storage for the error stream.
- std::string ErrorString;
- /// The error stream.
- raw_string_ostream ErrorStream;
- /// Temporary parsing buffer for the arguments.
- SmallVector<Argument, 8> TmpArgs;
- /// The string table used for parsing strings.
- Optional<const ParsedStringTable *> StrTab;
- /// The state used by the parser to parse a remark entry. Invalidated with
- /// every call to `parseYAMLElement`.
- struct ParseState {
- /// Temporary parsing buffer for the arguments.
- /// The parser itself is owning this buffer in order to reduce the number of
- /// allocations.
- SmallVectorImpl<Argument> &Args;
- Remark TheRemark;
-
- ParseState(SmallVectorImpl<Argument> &Args) : Args(Args) {}
- /// Use Args only as a **temporary** buffer.
- ~ParseState() { Args.clear(); }
- };
-
- /// The current state of the parser. If the parsing didn't start yet, it will
- /// not be containing any value.
- Optional<ParseState> State;
-
- YAMLRemarkParser(StringRef Buf,
- Optional<const ParsedStringTable *> StrTab = None)
- : SM(), Stream(Buf, SM), ErrorString(), ErrorStream(ErrorString),
- TmpArgs(), StrTab(StrTab) {
- SM.setDiagHandler(YAMLRemarkParser::HandleDiagnostic, this);
- }
-
- /// Parse a YAML element.
- Error parseYAMLElement(yaml::Document &Remark);
-
-private:
- /// Parse one key to a string.
- /// otherwise.
- Error parseKey(StringRef &Result, yaml::KeyValueNode &Node);
- /// Parse one value to a string.
- template <typename T> Error parseStr(T &Result, yaml::KeyValueNode &Node);
- /// Parse one value to an unsigned.
- template <typename T>
- Error parseUnsigned(T &Result, yaml::KeyValueNode &Node);
- /// Parse the type of a remark to an enum type.
- Error parseType(Type &Result, yaml::MappingNode &Node);
- /// Parse a debug location.
- Error parseDebugLoc(Optional<RemarkLocation> &Result,
- yaml::KeyValueNode &Node);
- /// Parse a remark field and update the parsing state.
- Error parseRemarkField(yaml::KeyValueNode &RemarkField);
- /// Parse an argument.
- Error parseArg(SmallVectorImpl<Argument> &TmpArgs, yaml::Node &Node);
- /// Parse an entry from the contents of an argument.
- Error parseArgEntry(yaml::KeyValueNode &ArgEntry, StringRef &KeyStr,
- StringRef &ValueStr, Optional<RemarkLocation> &Loc);
-
- /// Handle a diagnostic from the YAML stream. Records the error in the
- /// YAMLRemarkParser class.
- static void HandleDiagnostic(const SMDiagnostic &Diag, void *Ctx);
-};
class YAMLParseError : public ErrorInfo<YAMLParseError> {
public:
static char ID;
- YAMLParseError(StringRef Message, yaml::Node &Node)
- : Message(Message), Node(Node) {}
+ YAMLParseError(StringRef Message, SourceMgr &SM, yaml::Stream &Stream,
+ yaml::Node &Node);
+
+ YAMLParseError(StringRef Message) : Message(Message) {}
void log(raw_ostream &OS) const override { OS << Message; }
std::error_code convertToErrorCode() const override {
return inconvertibleErrorCode();
}
- StringRef getMessage() const { return Message; }
- yaml::Node &getNode() const { return Node; }
-
private:
- StringRef Message; // No need to hold a full copy of the buffer.
- yaml::Node &Node;
+ std::string Message;
};
/// Regular YAML to Remark parser.
-struct YAMLParserImpl : public ParserImpl {
- /// The object parsing the YAML.
- YAMLRemarkParser YAMLParser;
+struct YAMLRemarkParser : public Parser {
+ /// The string table used for parsing strings.
+ Optional<const ParsedStringTable *> StrTab;
+ /// Last error message that can come from the YAML parser diagnostics.
+ /// We need this for catching errors in the constructor.
+ std::string LastErrorMessage;
+ /// Source manager for better error messages.
+ SourceMgr SM;
+ /// Stream for yaml parsing.
+ yaml::Stream Stream;
/// Iterator in the YAML stream.
yaml::document_iterator YAMLIt;
- /// Set to `true` if we had any errors during parsing.
- bool HasErrors = false;
- YAMLParserImpl(StringRef Buf,
- Optional<const ParsedStringTable *> StrTab = None)
- : ParserImpl{Format::YAML}, YAMLParser(Buf, StrTab),
- YAMLIt(YAMLParser.Stream.begin()), HasErrors(false) {}
+ YAMLRemarkParser(StringRef Buf,
+ Optional<const ParsedStringTable *> StrTab = None);
+
+ Expected<std::unique_ptr<Remark>> next() override;
- static bool classof(const ParserImpl *PI) {
- return PI->ParserFormat == Format::YAML;
+ static bool classof(const Parser *P) {
+ return P->ParserFormat == Format::YAML;
}
+
+private:
+ /// Create a YAMLParseError error from an existing error generated by the YAML
+ /// parser.
+ /// If there is no error, this returns Success.
+ Error error();
+ /// Create a YAMLParseError error referencing a specific node.
+ Error error(StringRef Message, yaml::Node &Node);
+ /// Parse a YAML remark to a remarks::Remark object.
+ Expected<std::unique_ptr<Remark>> parseRemark(yaml::Document &Remark);
+ /// Parse the type of a remark to an enum type.
+ Expected<Type> parseType(yaml::MappingNode &Node);
+ /// Parse one key to a string.
+ Expected<StringRef> parseKey(yaml::KeyValueNode &Node);
+ /// Parse one value to a string.
+ Expected<StringRef> parseStr(yaml::KeyValueNode &Node);
+ /// Parse one value to an unsigned.
+ Expected<unsigned> parseUnsigned(yaml::KeyValueNode &Node);
+ /// Parse a debug location.
+ Expected<RemarkLocation> parseDebugLoc(yaml::KeyValueNode &Node);
+ /// Parse an argument.
+ Expected<Argument> parseArg(yaml::Node &Node);
};
} // end namespace remarks
} // end namespace llvm
Modified: llvm/trunk/tools/llvm-opt-report/OptReport.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-opt-report/OptReport.cpp?rev=366217&r1=366216&r2=366217&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-opt-report/OptReport.cpp (original)
+++ llvm/trunk/tools/llvm-opt-report/OptReport.cpp Tue Jul 16 08:25:05 2019
@@ -150,20 +150,32 @@ static bool readLocationInfo(LocationInf
return false;
}
- remarks::Parser Parser(remarks::Format::YAML, (*Buf)->getBuffer());
+ Expected<std::unique_ptr<remarks::Parser>> MaybeParser =
+ remarks::createRemarkParser(remarks::Format::YAML, (*Buf)->getBuffer());
+ if (!MaybeParser) {
+ handleAllErrors(MaybeParser.takeError(), [&](const ErrorInfoBase &PE) {
+ PE.log(WithColor::error());
+ });
+ return false;
+ }
+ remarks::Parser &Parser = **MaybeParser;
while (true) {
- Expected<const remarks::Remark *> RemarkOrErr = Parser.getNext();
- if (!RemarkOrErr) {
- handleAllErrors(RemarkOrErr.takeError(), [&](const ErrorInfoBase &PE) {
+ Expected<std::unique_ptr<remarks::Remark>> MaybeRemark = Parser.next();
+ if (!MaybeRemark) {
+ Error E = MaybeRemark.takeError();
+ if (E.isA<remarks::EndOfFileError>()) {
+ // EOF.
+ consumeError(std::move(E));
+ break;
+ }
+ handleAllErrors(MaybeRemark.takeError(), [&](const ErrorInfoBase &PE) {
PE.log(WithColor::error());
});
return false;
}
- if (!*RemarkOrErr) // End of file.
- break;
- const remarks::Remark &Remark = **RemarkOrErr;
+ const remarks::Remark &Remark = **MaybeRemark;
bool Transformed = Remark.RemarkType == remarks::Type::Passed;
Modified: llvm/trunk/tools/remarks-shlib/Remarks.exports
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/remarks-shlib/Remarks.exports?rev=366217&r1=366216&r2=366217&view=diff
==============================================================================
--- llvm/trunk/tools/remarks-shlib/Remarks.exports (original)
+++ llvm/trunk/tools/remarks-shlib/Remarks.exports Tue Jul 16 08:25:05 2019
@@ -6,6 +6,7 @@ LLVMRemarkDebugLocGetSourceColumn
LLVMRemarkArgGetKey
LLVMRemarkArgGetValue
LLVMRemarkArgGetDebugLoc
+LLVMRemarkEntryDispose
LLVMRemarkEntryGetType
LLVMRemarkEntryGetPassName
LLVMRemarkEntryGetRemarkName
Modified: llvm/trunk/unittests/Remarks/YAMLRemarksParsingTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Remarks/YAMLRemarksParsingTest.cpp?rev=366217&r1=366216&r2=366217&view=diff
==============================================================================
--- llvm/trunk/unittests/Remarks/YAMLRemarksParsingTest.cpp (original)
+++ llvm/trunk/unittests/Remarks/YAMLRemarksParsingTest.cpp Tue Jul 16 08:25:05 2019
@@ -14,20 +14,31 @@
using namespace llvm;
template <size_t N> void parseGood(const char (&Buf)[N]) {
- remarks::Parser Parser(remarks::Format::YAML, {Buf, N - 1});
- Expected<const remarks::Remark *> Remark = Parser.getNext();
+ Expected<std::unique_ptr<remarks::Parser>> MaybeParser =
+ remarks::createRemarkParser(remarks::Format::YAML, {Buf, N - 1});
+ EXPECT_FALSE(errorToBool(MaybeParser.takeError()));
+ EXPECT_TRUE(*MaybeParser != nullptr);
+
+ remarks::Parser &Parser = **MaybeParser;
+ Expected<std::unique_ptr<remarks::Remark>> Remark = Parser.next();
EXPECT_FALSE(errorToBool(Remark.takeError())); // Check for parsing errors.
EXPECT_TRUE(*Remark != nullptr); // At least one remark.
- Remark = Parser.getNext();
- EXPECT_FALSE(errorToBool(Remark.takeError())); // Check for parsing errors.
- EXPECT_TRUE(*Remark == nullptr); // Check that there are no more remarks.
+ Remark = Parser.next();
+ Error E = Remark.takeError();
+ EXPECT_TRUE(E.isA<remarks::EndOfFileError>());
+ EXPECT_TRUE(errorToBool(std::move(E))); // Check for parsing errors.
}
template <size_t N>
bool parseExpectError(const char (&Buf)[N], const char *Error) {
- remarks::Parser Parser(remarks::Format::YAML, {Buf, N - 1});
- Expected<const remarks::Remark *> Remark = Parser.getNext();
- EXPECT_FALSE(Remark); // Expect an error here.
+ Expected<std::unique_ptr<remarks::Parser>> MaybeParser =
+ remarks::createRemarkParser(remarks::Format::YAML, {Buf, N - 1});
+ EXPECT_FALSE(errorToBool(MaybeParser.takeError()));
+ EXPECT_TRUE(*MaybeParser != nullptr);
+
+ remarks::Parser &Parser = **MaybeParser;
+ Expected<std::unique_ptr<remarks::Remark>> Remark = Parser.next();
+ EXPECT_FALSE(Remark); // Check for parsing errors.
std::string ErrorStr;
raw_string_ostream Stream(ErrorStr);
@@ -42,7 +53,7 @@ TEST(YAMLRemarks, ParsingEmpty) {
TEST(YAMLRemarks, ParsingNotYAML) {
EXPECT_TRUE(
- parseExpectError("\x01\x02\x03\x04\x05\x06", "not a valid YAML file."));
+ parseExpectError("\x01\x02\x03\x04\x05\x06", "Got empty plain scalar"));
}
TEST(YAMLRemarks, ParsingGood) {
@@ -315,17 +326,6 @@ TEST(YAMLRemarks, ParsingWrongArgs) {
"Name: NoDefinition\n"
"Function: foo\n"
"Args:\n"
- " - Callee: ''\n"
- " - DebugLoc: { File: a, Line: 1, Column: 2 }\n"
- "",
- "argument value is missing."));
- // No arg value.
- EXPECT_TRUE(parseExpectError("\n"
- "--- !Missed\n"
- "Pass: inline\n"
- "Name: NoDefinition\n"
- "Function: foo\n"
- "Args:\n"
" - DebugLoc: { File: a, Line: 1, Column: 2 }\n"
"",
"argument key is missing."));
@@ -354,12 +354,18 @@ TEST(YAMLRemarks, Contents) {
" - String: ' because its definition is unavailable'\n"
"\n";
- remarks::Parser Parser(remarks::Format::YAML, Buf);
- Expected<const remarks::Remark *> RemarkOrErr = Parser.getNext();
- EXPECT_FALSE(errorToBool(RemarkOrErr.takeError()));
- EXPECT_TRUE(*RemarkOrErr != nullptr);
+ Expected<std::unique_ptr<remarks::Parser>> MaybeParser =
+ remarks::createRemarkParser(remarks::Format::YAML, Buf);
+ EXPECT_FALSE(errorToBool(MaybeParser.takeError()));
+ EXPECT_TRUE(*MaybeParser != nullptr);
+
+ remarks::Parser &Parser = **MaybeParser;
+ Expected<std::unique_ptr<remarks::Remark>> MaybeRemark = Parser.next();
+ EXPECT_FALSE(
+ errorToBool(MaybeRemark.takeError())); // Check for parsing errors.
+ EXPECT_TRUE(*MaybeRemark != nullptr); // At least one remark.
- const remarks::Remark &Remark = **RemarkOrErr;
+ const remarks::Remark &Remark = **MaybeRemark;
EXPECT_EQ(Remark.RemarkType, remarks::Type::Missed);
EXPECT_EQ(checkStr(Remark.PassName, 6), "inline");
EXPECT_EQ(checkStr(Remark.RemarkName, 12), "NoDefinition");
@@ -408,9 +414,10 @@ TEST(YAMLRemarks, Contents) {
++ArgID;
}
- RemarkOrErr = Parser.getNext();
- EXPECT_FALSE(errorToBool(RemarkOrErr.takeError()));
- EXPECT_EQ(*RemarkOrErr, nullptr);
+ MaybeRemark = Parser.next();
+ Error E = MaybeRemark.takeError();
+ EXPECT_TRUE(E.isA<remarks::EndOfFileError>());
+ EXPECT_TRUE(errorToBool(std::move(E))); // Check for parsing errors.
}
static inline StringRef checkStr(LLVMRemarkStringRef Str,
@@ -487,6 +494,8 @@ TEST(YAMLRemarks, ContentsCAPI) {
++ArgID;
} while ((Arg = LLVMRemarkEntryGetNextArg(Arg, Remark)));
+ LLVMRemarkEntryDispose(Remark);
+
EXPECT_EQ(LLVMRemarkParserGetNext(Parser), nullptr);
EXPECT_FALSE(LLVMRemarkParserHasError(Parser));
@@ -516,12 +525,18 @@ TEST(YAMLRemarks, ContentsStrTab) {
115);
remarks::ParsedStringTable StrTab(StrTabBuf);
- remarks::Parser Parser(remarks::Format::YAML, Buf, StrTab);
- Expected<const remarks::Remark *> RemarkOrErr = Parser.getNext();
- EXPECT_FALSE(errorToBool(RemarkOrErr.takeError()));
- EXPECT_TRUE(*RemarkOrErr != nullptr);
+ Expected<std::unique_ptr<remarks::Parser>> MaybeParser =
+ remarks::createRemarkParser(remarks::Format::YAML, Buf, &StrTab);
+ EXPECT_FALSE(errorToBool(MaybeParser.takeError()));
+ EXPECT_TRUE(*MaybeParser != nullptr);
+
+ remarks::Parser &Parser = **MaybeParser;
+ Expected<std::unique_ptr<remarks::Remark>> MaybeRemark = Parser.next();
+ EXPECT_FALSE(
+ errorToBool(MaybeRemark.takeError())); // Check for parsing errors.
+ EXPECT_TRUE(*MaybeRemark != nullptr); // At least one remark.
- const remarks::Remark &Remark = **RemarkOrErr;
+ const remarks::Remark &Remark = **MaybeRemark;
EXPECT_EQ(Remark.RemarkType, remarks::Type::Missed);
EXPECT_EQ(checkStr(Remark.PassName, 6), "inline");
EXPECT_EQ(checkStr(Remark.RemarkName, 12), "NoDefinition");
@@ -570,9 +585,10 @@ TEST(YAMLRemarks, ContentsStrTab) {
++ArgID;
}
- RemarkOrErr = Parser.getNext();
- EXPECT_FALSE(errorToBool(RemarkOrErr.takeError()));
- EXPECT_EQ(*RemarkOrErr, nullptr);
+ MaybeRemark = Parser.next();
+ Error E = MaybeRemark.takeError();
+ EXPECT_TRUE(E.isA<remarks::EndOfFileError>());
+ EXPECT_TRUE(errorToBool(std::move(E))); // Check for parsing errors.
}
TEST(YAMLRemarks, ParsingBadStringTableIndex) {
@@ -584,13 +600,18 @@ TEST(YAMLRemarks, ParsingBadStringTableI
StringRef StrTabBuf = StringRef("inline");
remarks::ParsedStringTable StrTab(StrTabBuf);
- remarks::Parser Parser(remarks::Format::YAML, Buf, StrTab);
- Expected<const remarks::Remark *> Remark = Parser.getNext();
- EXPECT_FALSE(Remark); // Expect an error here.
+ Expected<std::unique_ptr<remarks::Parser>> MaybeParser =
+ remarks::createRemarkParser(remarks::Format::YAML, Buf, &StrTab);
+ EXPECT_FALSE(errorToBool(MaybeParser.takeError()));
+ EXPECT_TRUE(*MaybeParser != nullptr);
+
+ remarks::Parser &Parser = **MaybeParser;
+ Expected<std::unique_ptr<remarks::Remark>> MaybeRemark = Parser.next();
+ EXPECT_FALSE(MaybeRemark); // Expect an error here.
std::string ErrorStr;
raw_string_ostream Stream(ErrorStr);
- handleAllErrors(Remark.takeError(),
+ handleAllErrors(MaybeRemark.takeError(),
[&](const ErrorInfoBase &EIB) { EIB.log(Stream); });
EXPECT_TRUE(
StringRef(Stream.str())
More information about the llvm-commits
mailing list