Skip to content

Commit

Permalink
refactor(diag): create new Diag_List type; use in one spot
Browse files Browse the repository at this point in the history
I don't like Buffering_Diag_Reporter. Create a new Diag_List type,
decoupled from Diag_Reporter, which we can use to "just" store a list
of diagnostics (hence its name).

To test the waters, use Diag_List in place of Buffering_Diag_Reporter
in one spot in the Lexer.
  • Loading branch information
strager committed Jan 3, 2024
1 parent bdfccbd commit 3ddd74d
Show file tree
Hide file tree
Showing 11 changed files with 398 additions and 21 deletions.
2 changes: 2 additions & 0 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,8 @@ quick_lint_js_add_library(
quick-lint-js/diag/buffering-diag-reporter.h
quick-lint-js/diag/diag-code-list.cpp
quick-lint-js/diag/diag-code-list.h
quick-lint-js/diag/diag-list.cpp
quick-lint-js/diag/diag-list.h
quick-lint-js/diag/diag-reporter.cpp
quick-lint-js/diag/diag-reporter.h
quick-lint-js/diag/diagnostic-formatter.cpp
Expand Down
95 changes: 95 additions & 0 deletions src/quick-lint-js/diag/diag-list.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
// Copyright (C) 2020 Matthew "strager" Glazar
// See end of file for extended copyright information.

#include <quick-lint-js/diag/diag-list.h>
#include <quick-lint-js/port/memory-resource.h>
#include <quick-lint-js/util/algorithm.h>

namespace quick_lint_js {
struct Diag_List::Node : public Node_Base {
union Underlying_Diag {
explicit Underlying_Diag() {}

#define QLJS_DIAG_TYPE_NAME(name) \
::quick_lint_js::name name; \
static_assert(std::is_trivially_copyable_v<::quick_lint_js::name>, \
#name " should be trivially copyable");
QLJS_X_DIAG_TYPE_NAMES
#undef QLJS_DIAG_TYPE_NAME
};

Underlying_Diag data;
};
static_assert(alignof(Diag_List::Node) == alignof(Diag_List::Node_Base),
"Node_Base alignment should align any Diag type");

Diag_List::Diag_List(Memory_Resource *memory) : memory_(memory) {}

Diag_List::~Diag_List() {
// Leak. this->memory should be a Linked_Bump_Allocator managed by the caller.
}

Diag_List::Rewind_State Diag_List::prepare_for_rewind() {
return Rewind_State{
.first_ = this->first_,
.last_ = this->last_,
};
}

void Diag_List::rewind(Rewind_State &&r) {
// Leak nodes between r.last and this->last_. this->memory should be a
// Linked_Bump_Allocator managed by the caller.
this->first_ = r.first_;
this->last_ = r.last_;
}

#define QLJS_DIAG_TYPE_NAME(name) \
void Diag_List::add(name diag) { \
this->add_impl<sizeof(diag)>(Diag_Type::name, &diag); \
}
QLJS_X_DIAG_TYPE_NAMES
#undef QLJS_DIAG_TYPE_NAME

template <std::size_t Diag_Size>
void Diag_List::add_impl(Diag_Type type, void *diag) {
Node *node = this->memory_->new_object<Node>();
node->next = nullptr;
node->type = type;
std::memcpy(&node->data, diag, Diag_Size);
if (this->last_ == nullptr) {
this->first_ = node;
} else {
this->last_->next = node;
}
this->last_ = node;
}

bool Diag_List::reported_any_diagnostic_except_since(
std::initializer_list<Diag_Type> ignored_types, const Rewind_State &r) {
for (Node_Base *node = r.last_ == nullptr ? this->first_ : r.last_;
node != nullptr; node = node->next) {
if (!contains(ignored_types, node->type)) {
return true;
}
}
return false;
}
}

// quick-lint-js finds bugs in JavaScript programs.
// Copyright (C) 2020 Matthew "strager" Glazar
//
// This file is part of quick-lint-js.
//
// quick-lint-js is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// quick-lint-js is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with quick-lint-js. If not, see <https://www.gnu.org/licenses/>.
90 changes: 90 additions & 0 deletions src/quick-lint-js/diag/diag-list.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
// Copyright (C) 2020 Matthew "strager" Glazar
// See end of file for extended copyright information.

#pragma once

#include <quick-lint-js/diag/diagnostic-types.h>
#include <quick-lint-js/port/memory-resource.h>

namespace quick_lint_js {
// Internally, Diag_List is implemented as a singly linked list.
class Diag_List {
public:
// Private to Diag_List. Do not use.
//
// Node_Base is split from Node to improve compile times. The union inside
// Node compiles slowly.
//
// Node_Base needs to be in the header for an efficient implementation of
// for_each.
struct Node_Base {
Node_Base *next;
Diag_Type type;

// Returns &static_cast<Node*>(this)->data.
void *get_data() { return &this[1]; }
};
// Private to Diag_List. Do not use.
struct Node;

struct Rewind_State {
// Private to Diag_List. Do not use.
Node_Base *first_;
Node_Base *last_;
};

explicit Diag_List(Memory_Resource *);

Diag_List(Diag_List &&) = delete;
Diag_List &operator=(Diag_List &&) = delete;

~Diag_List();

#define QLJS_DIAG_TYPE_NAME(name) void add(name diag);
QLJS_X_DIAG_TYPE_NAMES
#undef QLJS_DIAG_TYPE_NAME

Rewind_State prepare_for_rewind();

void rewind(Rewind_State &&r);

// Func must have the following signature:
//
// void(Diag_Type type, const void* diag);
template <class Func>
void for_each(Func &&callback) const {
for (Node_Base *node = this->first_; node != nullptr; node = node->next) {
callback(node->type, node->get_data());
}
}

bool reported_any_diagnostic_except_since(
std::initializer_list<Diag_Type> ignored_types, const Rewind_State &);

private:
template <std::size_t Diag_Size>
void add_impl(Diag_Type type, void *diag);

Memory_Resource *memory_;
Node_Base *first_ = nullptr;
Node_Base *last_ = nullptr;
};
}

// quick-lint-js finds bugs in JavaScript programs.
// Copyright (C) 2020 Matthew "strager" Glazar
//
// This file is part of quick-lint-js.
//
// quick-lint-js is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// quick-lint-js is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with quick-lint-js. If not, see <https://www.gnu.org/licenses/>.
7 changes: 7 additions & 0 deletions src/quick-lint-js/diag/diag-reporter.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (C) 2020 Matthew "strager" Glazar
// See end of file for extended copyright information.

#include <quick-lint-js/diag/diag-list.h>
#include <quick-lint-js/diag/diag-reporter.h>
#include <quick-lint-js/diag/diagnostic-types.h>

Expand All @@ -12,6 +13,12 @@ namespace quick_lint_js {
QLJS_X_DIAG_TYPE_NAMES
#undef QLJS_DIAG_TYPE_NAME

void Diag_Reporter::report(const Diag_List& diags) {
diags.for_each([&](Diag_Type type, void* diag) -> void {
this->report_impl(type, diag);
});
}

Null_Diag_Reporter Null_Diag_Reporter::instance;
}

Expand Down
5 changes: 5 additions & 0 deletions src/quick-lint-js/diag/diag-reporter.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
#include <quick-lint-js/diag/diagnostic-types.h>

namespace quick_lint_js {
class Diag_List;

class Diag_Reporter {
public:
Diag_Reporter() = default;
Expand All @@ -22,6 +24,9 @@ class Diag_Reporter {
QLJS_X_DIAG_TYPE_NAMES
#undef QLJS_DIAG_TYPE_NAME

virtual void report(const Diag_List &);

// TODO(#1154): Delete this in favor of report(const Diag_List&).
virtual void report_impl(Diag_Type type, void *diag) = 0;
};

Expand Down
35 changes: 20 additions & 15 deletions src/quick-lint-js/fe/lex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <quick-lint-js/container/string-view.h>
#include <quick-lint-js/container/vector.h>
#include <quick-lint-js/diag/buffering-diag-reporter.h>
#include <quick-lint-js/diag/diag-list.h>
#include <quick-lint-js/diag/diagnostic-types.h>
#include <quick-lint-js/fe/lex.h>
#include <quick-lint-js/fe/token.h>
Expand Down Expand Up @@ -725,11 +726,13 @@ const Char8* Lexer::parse_string_literal() {
}
}
break;
case 'u':
c = this->parse_unicode_escape(escape_sequence_start,
this->diag_reporter_)
.end;
case 'u': {
// TODO(#1154): Remove this temporary Diag_List.
Diag_List diags(&this->allocator_);
c = this->parse_unicode_escape(escape_sequence_start, &diags).end;
this->diag_reporter_->report(diags);
break;
}
default:
++c;
break;
Expand Down Expand Up @@ -841,7 +844,7 @@ void Lexer::skip_in_template(const Char8* template_begin) {
Lexer::Parsed_Template_Body Lexer::parse_template_body(
const Char8* input, const Char8* template_begin,
Diag_Reporter* diag_reporter) {
Buffering_Diag_Reporter* escape_sequence_diagnostics = nullptr;
Diag_List* escape_sequence_diagnostics = nullptr;
const Char8* c = input;
for (;;) {
switch (*c) {
Expand Down Expand Up @@ -878,8 +881,7 @@ Lexer::Parsed_Template_Body Lexer::parse_template_body(
case 'u': {
if (!escape_sequence_diagnostics) {
escape_sequence_diagnostics =
this->allocator_.new_object<Buffering_Diag_Reporter>(
&this->allocator_);
this->allocator_.new_object<Diag_List>(&this->allocator_);
}
c = this->parse_unicode_escape(escape_sequence_start,
escape_sequence_diagnostics)
Expand Down Expand Up @@ -1527,8 +1529,8 @@ const Char8* Lexer::parse_hex_digits_and_underscores(const Char8* input) {

QLJS_WARNING_PUSH
QLJS_WARNING_IGNORE_GCC("-Wuseless-cast")
Lexer::Parsed_Unicode_Escape Lexer::parse_unicode_escape(
const Char8* input, Diag_Reporter* reporter) {
Lexer::Parsed_Unicode_Escape Lexer::parse_unicode_escape(const Char8* input,
Diag_List* out_diags) {
const Char8* escape_sequence_begin = input;
auto get_escape_span = [escape_sequence_begin, &input]() {
return Source_Code_Span(escape_sequence_begin, input);
Expand All @@ -1545,7 +1547,7 @@ Lexer::Parsed_Unicode_Escape Lexer::parse_unicode_escape(
// TODO: Add an enum to Diag_Unclosed_Identifier_Escape_Sequence to
// indicate whether the token is a template literal, a string literal
// or an identifier.
reporter->report(Diag_Unclosed_Identifier_Escape_Sequence{
out_diags->add(Diag_Unclosed_Identifier_Escape_Sequence{
.escape_sequence = get_escape_span()});
return Parsed_Unicode_Escape{.end = input, .code_point = std::nullopt};
}
Expand All @@ -1557,7 +1559,7 @@ Lexer::Parsed_Unicode_Escape Lexer::parse_unicode_escape(
code_point_hex_end = input;
++input; // Skip "}".
if (found_non_hex_digit || code_point_hex_begin == code_point_hex_end) {
reporter->report(Diag_Expected_Hex_Digits_In_Unicode_Escape{
out_diags->add(Diag_Expected_Hex_Digits_In_Unicode_Escape{
.escape_sequence = get_escape_span()});
return Parsed_Unicode_Escape{.end = input, .code_point = std::nullopt};
}
Expand All @@ -1569,12 +1571,12 @@ Lexer::Parsed_Unicode_Escape Lexer::parse_unicode_escape(
// TODO: Add an enum to Diag_Expected_Hex_Digits_In_Unicode_Escape to
// indicate whether the token is a template literal, a string literal
// or an identifier.
reporter->report(Diag_Expected_Hex_Digits_In_Unicode_Escape{
out_diags->add(Diag_Expected_Hex_Digits_In_Unicode_Escape{
.escape_sequence = get_escape_span()});
return Parsed_Unicode_Escape{.end = input, .code_point = std::nullopt};
}
if (!is_hex_digit(*input)) {
reporter->report(Diag_Expected_Hex_Digits_In_Unicode_Escape{
out_diags->add(Diag_Expected_Hex_Digits_In_Unicode_Escape{
.escape_sequence =
Source_Code_Span(escape_sequence_begin, input + 1)});
return Parsed_Unicode_Escape{.end = input, .code_point = std::nullopt};
Expand All @@ -1596,7 +1598,7 @@ Lexer::Parsed_Unicode_Escape Lexer::parse_unicode_escape(
break;
}
if (code_point >= 0x110000) {
reporter->report(Diag_Escaped_Code_Point_In_Unicode_Out_Of_Range{
out_diags->add(Diag_Escaped_Code_Point_In_Unicode_Out_Of_Range{
.escape_sequence = get_escape_span()});
}
return Parsed_Unicode_Escape{.end = input, .code_point = code_point};
Expand Down Expand Up @@ -1717,8 +1719,11 @@ Lexer::Parsed_Identifier Lexer::parse_identifier_slow(

auto parse_unicode_escape = [&]() {
const Char8* escape_begin = input;
// TODO(#1154): Remove this temporary Diag_List.
Diag_List diags(&this->allocator_);
Parsed_Unicode_Escape escape =
this->parse_unicode_escape(escape_begin, this->diag_reporter_);
this->parse_unicode_escape(escape_begin, &diags);
this->diag_reporter_->report(diags);

if (escape.code_point.has_value()) {
bool is_initial_identifier_character = escape_begin == identifier_begin;
Expand Down
4 changes: 2 additions & 2 deletions src/quick-lint-js/fe/lex.h
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ class Lexer {
Token_Type type;
const Char8* end;
// Might be null.
Buffering_Diag_Reporter* escape_sequence_diagnostics;
Diag_List* escape_sequence_diagnostics;
};

// The result of parsing an identifier.
Expand Down Expand Up @@ -284,7 +284,7 @@ class Lexer {
};

Parsed_Unicode_Escape parse_unicode_escape(const Char8* input,
Diag_Reporter*);
Diag_List* out_diags);

Parsed_Identifier parse_identifier(const Char8*, Identifier_Kind);
const Char8* parse_identifier_fast_only(const Char8*);
Expand Down
5 changes: 3 additions & 2 deletions src/quick-lint-js/fe/token.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
// See end of file for extended copyright information.

#include <quick-lint-js/assert.h>
#include <quick-lint-js/diag/buffering-diag-reporter.h>
#include <quick-lint-js/diag/diag-list.h>
#include <quick-lint-js/diag/diag-reporter.h>
#include <quick-lint-js/diag/diagnostic-types.h>
#include <quick-lint-js/fe/identifier.h>
#include <quick-lint-js/fe/token.h>
Expand Down Expand Up @@ -45,7 +46,7 @@ void Token::report_errors_for_escape_sequences_in_template(
QLJS_ASSERT(this->type == Token_Type::complete_template ||
this->type == Token_Type::incomplete_template);
if (this->template_escape_sequence_diagnostics) {
this->template_escape_sequence_diagnostics->move_into(reporter);
reporter->report(*this->template_escape_sequence_diagnostics);
}
}

Expand Down
Loading

0 comments on commit 3ddd74d

Please sign in to comment.