JSSpecCompiler: Start converting crashes to error messages

I got fed up with looking at error messages that tell me "VERIFICATION
FAILED: !is_error()". So this commit introduces DiagnosticEngine class
whose purpose is to accumulate and print more user-friendly errors.
This commit is contained in:
Dan Klishch 2024-01-18 21:09:53 -05:00 committed by Andrew Kaster
parent 3aec6952a2
commit 0806ccaeec
8 changed files with 262 additions and 4 deletions

View file

@ -17,6 +17,7 @@ set(SOURCES
Parser/SpecParser.cpp
Parser/TextParser.cpp
Parser/XMLUtils.cpp
DiagnosticEngine.cpp
Function.cpp
main.cpp
)

View file

@ -0,0 +1,74 @@
/*
* Copyright (c) 2024, Dan Klishch <danilklishch@gmail.com>
*
* SPDX-License-Identifier: BSD-2-Clause
*/
#include "DiagnosticEngine.h"
namespace JSSpecCompiler {
bool DiagnosticEngine::has_fatal_errors() const
{
return m_has_fatal_errors;
}
void DiagnosticEngine::print_diagnostics()
{
auto use_color = isatty(STDERR_FILENO) ? UseColor::Yes : UseColor::No;
StringBuilder builder;
for (auto const& diagnostic : m_diagnostics)
diagnostic.format_into(builder, use_color);
out(stderr, "{}", builder.string_view());
}
void DiagnosticEngine::Diagnostic::format_into(StringBuilder& builder, UseColor use_color) const
{
if (!location.filename.is_empty())
builder.appendff("{}:{}:{}: ", location.filename, location.line + 1, location.column + 1);
static constexpr Array<StringView, 4> colored_diagnostic_levels = { {
"\e[1mnote\e[0m"sv,
"\e[1;33mwarning\e[0m"sv,
"\e[1;31merror\e[0m"sv,
"\e[1;31mfatal error\e[0m"sv,
} };
static constexpr Array<StringView, 4> diagnostic_levels = { {
"note"sv,
"warning"sv,
"error"sv,
"fatal error"sv,
} };
auto diagnostic_level_text = (use_color == UseColor::Yes ? colored_diagnostic_levels : diagnostic_levels);
builder.appendff("{}: ", diagnostic_level_text[to_underlying(level)]);
if (auto logical_location = location.logical_location) {
if (!logical_location->section.is_empty()) {
builder.appendff("in {}", logical_location->section);
if (!logical_location->step.is_empty())
builder.appendff(" step {}", logical_location->step);
builder.appendff(": ");
}
}
builder.append(message);
builder.append('\n');
for (auto const& note : notes)
note.format_into(builder, use_color);
}
void DiagnosticEngine::add_diagnostic(Diagnostic&& diagnostic)
{
if (diagnostic.level == DiagnosticLevel::FatalError)
m_has_fatal_errors = true;
if (diagnostic.level != DiagnosticLevel::Note)
m_diagnostics.append(move(diagnostic));
else
m_diagnostics.last().notes.append(move(diagnostic));
}
}

View file

@ -0,0 +1,90 @@
/*
* Copyright (c) 2024, Dan Klishch <danilklishch@gmail.com>
*
* SPDX-License-Identifier: BSD-2-Clause
*/
#pragma once
#include <AK/ByteString.h>
#include <AK/FlyString.h>
#include <AK/QuickSort.h>
#include <AK/String.h>
#include <LibXML/DOM/Node.h>
namespace JSSpecCompiler {
struct LogicalLocation : RefCounted<LogicalLocation> {
String section;
String step;
};
struct Location {
StringView filename;
size_t line = 0;
size_t column = 0;
RefPtr<LogicalLocation> logical_location;
static Location global_scope() { return {}; }
};
class DiagnosticEngine {
AK_MAKE_NONCOPYABLE(DiagnosticEngine);
AK_MAKE_NONMOVABLE(DiagnosticEngine);
public:
DiagnosticEngine() = default;
#define DEFINE_DIAGNOSTIC_FUNCTION(name_, level_) \
template<typename... Parameters> \
void name_(Location const& location, AK::CheckedFormatString<Parameters...>&& fmtstr, Parameters const&... parameters) \
{ \
add_diagnostic({ \
.location = location, \
.level = DiagnosticLevel::level_, \
.message = MUST(String::formatted(move(fmtstr), parameters...)), \
}); \
}
DEFINE_DIAGNOSTIC_FUNCTION(note, Note)
DEFINE_DIAGNOSTIC_FUNCTION(warn, Warning)
DEFINE_DIAGNOSTIC_FUNCTION(error, Error)
DEFINE_DIAGNOSTIC_FUNCTION(fatal_error, FatalError)
#undef DEFINE_DIAGNOSTIC_FUNCTION
bool has_fatal_errors() const;
void print_diagnostics();
private:
enum class DiagnosticLevel {
Note,
Warning,
Error,
FatalError,
};
enum class UseColor {
No,
Yes,
};
struct Diagnostic {
Location location;
DiagnosticLevel level;
String message;
Vector<Diagnostic> notes;
bool operator<(Diagnostic const& other) const;
void format_into(StringBuilder& builder, UseColor) const;
};
void add_diagnostic(Diagnostic&& diagnostic);
Vector<Diagnostic> m_diagnostics;
bool m_has_fatal_errors = false;
};
}

View file

@ -64,6 +64,11 @@ class AlgorithmStepList;
class Algorithm;
class SpecFunction;
// DiagnosticEngine.h
struct LogicalLocation;
struct Location;
class DiagnosticEngine;
// Function.h
class TranslationUnit;
using TranslationUnitRef = TranslationUnit*;

View file

@ -11,6 +11,7 @@
#include <AK/RefPtr.h>
#include <AK/StringView.h>
#include "DiagnosticEngine.h"
#include "Forward.h"
namespace JSSpecCompiler {
@ -26,10 +27,12 @@ public:
FunctionDeclarationRef find_declaration_by_name(StringView name) const;
StringView filename() const { return m_filename; }
DiagnosticEngine& diag() { return m_diagnostic_engine; }
Vector<FunctionDefinitionRef> functions_to_compile() const { return m_functions_to_compile; }
private:
StringView m_filename;
DiagnosticEngine m_diagnostic_engine;
Vector<FunctionDefinitionRef> m_functions_to_compile;
Vector<NonnullRefPtr<FunctionDeclaration>> m_declarations_owner;
HashMap<StringView, FunctionDeclarationRef> m_function_index;

View file

@ -16,6 +16,38 @@
namespace JSSpecCompiler {
DiagnosticEngine& SpecificationParsingContext::diag()
{
return m_translation_unit->diag();
}
template<typename Func>
auto SpecificationParsingContext::with_new_logical_scope(Func&& func)
{
TemporaryChange<RefPtr<LogicalLocation>> change(m_current_logical_scope, make_ref_counted<LogicalLocation>());
return func();
}
LogicalLocation& SpecificationParsingContext::current_logical_scope()
{
return *m_current_logical_scope;
}
Location SpecificationParsingContext::file_scope() const
{
return { .filename = m_translation_unit->filename() };
}
Location SpecificationParsingContext::location_from_xml_offset(XML::Offset offset) const
{
return {
.filename = m_translation_unit->filename(),
.line = offset.line,
.column = offset.column,
.logical_location = m_current_logical_scope,
};
}
ParseErrorOr<AlgorithmStep> AlgorithmStep::create(XML::Node const* node)
{
VERIFY(node->as_element().name == tag_li);
@ -185,14 +217,35 @@ SpecParsingStep::~SpecParsingStep() = default;
void SpecParsingStep::run(TranslationUnitRef translation_unit)
{
SpecificationParsingContext ctx(translation_unit);
auto filename = translation_unit->filename();
auto file = Core::File::open_file_or_standard_stream(filename, Core::File::OpenMode::Read).release_value_but_fixme_should_propagate_errors();
m_input = file->read_until_eof().release_value_but_fixme_should_propagate_errors();
auto file_or_error = Core::File::open_file_or_standard_stream(filename, Core::File::OpenMode::Read);
if (file_or_error.is_error()) {
ctx.diag().fatal_error(Location::global_scope(),
"unable to open '{}': {}", filename, file_or_error.error());
return;
}
auto input_or_error = file_or_error.value()->read_until_eof();
if (input_or_error.is_error()) {
ctx.diag().fatal_error(Location::global_scope(),
"unable to read '{}': {}", filename, input_or_error.error());
return;
}
m_input = input_or_error.release_value();
XML::Parser parser { m_input };
auto document = parser.parse().release_value_but_fixme_should_propagate_errors();
m_document = AK::adopt_own_if_nonnull(new XML::Document(move(document)));
auto document_or_error = parser.parse();
if (document_or_error.is_error()) {
ctx.diag().fatal_error(ctx.file_scope(),
"XML::Parser failed to parse input: {}", document_or_error.error());
ctx.diag().note(ctx.file_scope(),
"since XML::Parser backtracks on error, the message above is likely to point to the "
"first tag in the input - use external XML verifier to find out the exact cause of error");
return;
}
m_document = make<XML::Document>(document_or_error.release_value());
auto spec_function = SpecFunction::create(&m_document->root()).release_value_but_fixme_should_propagate_errors();

View file

@ -15,6 +15,31 @@
#include "Parser/Token.h"
namespace JSSpecCompiler {
class SpecificationParsingContext {
AK_MAKE_NONCOPYABLE(SpecificationParsingContext);
AK_MAKE_NONMOVABLE(SpecificationParsingContext);
public:
SpecificationParsingContext(TranslationUnitRef translation_unit)
: m_translation_unit(translation_unit)
{
}
DiagnosticEngine& diag();
template<typename Func>
auto with_new_logical_scope(Func&& func);
LogicalLocation& current_logical_scope();
Location file_scope() const;
Location location_from_xml_offset(XML::Offset offset) const;
private:
TranslationUnitRef m_translation_unit;
RefPtr<LogicalLocation> m_current_logical_scope;
};
class AlgorithmStepList {
public:
static ParseErrorOr<AlgorithmStepList> create(XML::Node::Element const& element);

View file

@ -136,6 +136,11 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
for (auto const& step : pipeline.pipeline()) {
step.step->run(&translation_unit);
if (translation_unit.diag().has_fatal_errors()) {
translation_unit.diag().print_diagnostics();
return 1;
}
if (step.dump_ast) {
outln(stderr, "===== AST after {} =====", step.step->name());
for (auto const& function : translation_unit.functions_to_compile()) {
@ -152,5 +157,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
}
}
translation_unit.diag().print_diagnostics();
return 0;
}