diff --git a/src/debug.cc b/src/debug.cc index cc88b6d96b..d63f902ef5 100644 --- a/src/debug.cc +++ b/src/debug.cc @@ -14,6 +14,8 @@ #include "src/debug.h" +#include + namespace tint { namespace { @@ -32,9 +34,9 @@ InternalCompilerError::InternalCompilerError(const char* file, : file_(file), line_(line), system_(system), diagnostics_(diagnostics) {} InternalCompilerError::~InternalCompilerError() { - Source source{Source::Range{{line_}}, new Source::File{file_, ""}}; - diagnostics_.own_file(source.file); - diagnostics_.add_ice(system_, msg_.str(), source); + auto file = std::make_shared(file_, ""); + Source source{Source::Range{{line_}}, file.get()}; + diagnostics_.add_ice(system_, msg_.str(), source, std::move(file)); if (ice_reporter) { ice_reporter(diagnostics_); diff --git a/src/diagnostic/diagnostic.cc b/src/diagnostic/diagnostic.cc index 62df006f0f..5d2a8533e1 100644 --- a/src/diagnostic/diagnostic.cc +++ b/src/diagnostic/diagnostic.cc @@ -21,39 +21,20 @@ namespace tint { namespace diag { +Diagnostic::Diagnostic() = default; +Diagnostic::Diagnostic(const Diagnostic&) = default; +Diagnostic::~Diagnostic() = default; +Diagnostic& Diagnostic::operator=(const Diagnostic&) = default; + List::List() = default; List::List(std::initializer_list list) : entries_(list) {} -List::List(const List& rhs) { - *this = rhs; -} +List::List(const List& rhs) = default; List::List(List&& rhs) = default; List::~List() = default; -List& List::operator=(const List& rhs) { - // Create copies of any of owned files, maintaining a map of rhs-file to - // new-file. - std::unordered_map files; - owned_files_.reserve(rhs.owned_files_.size()); - files.reserve(rhs.owned_files_.size()); - for (auto& rhs_file : rhs.owned_files_) { - auto file = std::make_unique(*rhs_file); - files.emplace(rhs_file.get(), file.get()); - owned_files_.emplace_back(std::move(file)); - } - - // Copy the diagnostic entries, then fix up pointers to the file copies. - entries_ = rhs.entries_; - for (auto& entry : entries_) { - if (auto it = files.find(entry.source.file); it != files.end()) { - entry.source.file = it->second; - } - } - - error_count_ = rhs.error_count_; - return *this; -} +List& List::operator=(const List& rhs) = default; List& List::operator=(List&& rhs) = default; diff --git a/src/diagnostic/diagnostic.h b/src/diagnostic/diagnostic.h index 9d7c6011ea..c2c30fcb8d 100644 --- a/src/diagnostic/diagnostic.h +++ b/src/diagnostic/diagnostic.h @@ -55,6 +55,17 @@ enum class System { /// message. class Diagnostic { public: + /// Constructor + Diagnostic(); + /// Copy constructor + Diagnostic(const Diagnostic&); + /// Destructor + ~Diagnostic(); + + /// Copy assignment operator + /// @return this diagnostic + Diagnostic& operator=(const Diagnostic&); + /// severity is the severity of the diagnostic message. Severity severity = Severity::Error; /// source is the location of the diagnostic. @@ -66,6 +77,10 @@ class Diagnostic { /// code is the error code, for example a validation error might have the code /// `"v-0001"`. const char* code = nullptr; + /// A shared pointer to a Source::File. Only used if the diagnostic Source + /// points to a file that was created specifically for this diagnostic + /// (usually an ICE). + std::shared_ptr owned_file = nullptr; }; /// List is a container of Diagnostic messages. @@ -197,24 +212,20 @@ class List { /// @param system the system raising the error message /// @param err_msg the error message /// @param source the source of the internal compiler error + /// @param file the Source::File owned by this diagnostic void add_ice(System system, const std::string& err_msg, - const Source& source) { + const Source& source, + std::shared_ptr file) { diag::Diagnostic ice{}; ice.severity = diag::Severity::InternalCompilerError; ice.system = system; ice.source = source; ice.message = err_msg; + ice.owned_file = std::move(file); add(std::move(ice)); } - /// Adds the file to the list of files owned by this diagnostic list. - /// When this list is destructed, all the owned files will be deleted. - /// @param file the file that this List should own - void own_file(const Source::File* file) { - owned_files_.emplace_back(std::unique_ptr(file)); - } - /// @returns true iff the diagnostic list contains errors diagnostics (or of /// higher severity). bool contains_errors() const { return error_count_ > 0; } @@ -232,7 +243,6 @@ class List { private: std::vector entries_; - std::vector> owned_files_; size_t error_count_ = 0; }; diff --git a/src/diagnostic/diagnostic_test.cc b/src/diagnostic/diagnostic_test.cc index e123a0bace..b904b6fd64 100644 --- a/src/diagnostic/diagnostic_test.cc +++ b/src/diagnostic/diagnostic_test.cc @@ -21,43 +21,20 @@ namespace tint { namespace diag { namespace { -TEST(DiagListTest, UnownedFilesNotCopied) { - Source::File file{"path", "content"}; +TEST(DiagListTest, OwnedFilesShared) { + auto file = std::make_shared("path", "content"); diag::List list_a, list_b; { diag::Diagnostic diag{}; - diag.source = Source{Source::Range{{0, 0}}, &file}; + diag.source = Source{Source::Range{{0, 0}}, file.get()}; list_a.add(std::move(diag)); } list_b = list_a; ASSERT_EQ(list_b.count(), list_a.count()); - EXPECT_EQ(list_b.begin()->source.file, &file); -} - -TEST(DiagListTest, OwnedFilesCopied) { - auto* file = new Source::File{"path", "content"}; - - diag::List list_a, list_b; - { - diag::Diagnostic diag{}; - diag.source = Source{Source::Range{{0, 0}}, file}; - list_a.add(std::move(diag)); - list_a.own_file(file); - } - - list_b = list_a; - - ASSERT_EQ(list_b.count(), list_a.count()); - EXPECT_NE(list_b.begin()->source.file, file); - ASSERT_NE(list_b.begin()->source.file, nullptr); - EXPECT_EQ(list_b.begin()->source.file->path, file->path); - EXPECT_EQ(list_b.begin()->source.file->content.data, file->content.data); - EXPECT_EQ(list_b.begin()->source.file->content.data_view, - file->content.data_view); - EXPECT_EQ(list_b.begin()->source.file->content.lines, file->content.lines); + EXPECT_EQ(list_b.begin()->source.file, file.get()); } } // namespace diff --git a/src/diagnostic/formatter_test.cc b/src/diagnostic/formatter_test.cc index f77b847a6b..966b888956 100644 --- a/src/diagnostic/formatter_test.cc +++ b/src/diagnostic/formatter_test.cc @@ -14,6 +14,8 @@ #include "src/diagnostic/formatter.h" +#include + #include "gtest/gtest.h" #include "src/diagnostic/diagnostic.h" @@ -21,6 +23,20 @@ namespace tint { namespace diag { namespace { +Diagnostic Diag(Severity severity, + Source source, + std::string message, + System system, + const char* code = nullptr) { + Diagnostic d; + d.severity = severity; + d.source = source; + d.message = std::move(message); + d.system = system; + d.code = code; + return d; +} + constexpr const char* content = // Note: words are tab-delimited R"(the cat says meow the dog says woof @@ -31,21 +47,28 @@ the snail says ??? class DiagFormatterTest : public testing::Test { public: Source::File file{"file.name", content}; - Diagnostic diag_note{Severity::Note, - Source{Source::Range{Source::Location{1, 14}}, &file}, - "purr", System::Test}; - Diagnostic diag_warn{Severity::Warning, - Source{Source::Range{{2, 14}, {2, 18}}, &file}, "grrr", - System::Test}; - Diagnostic diag_err{Severity::Error, - Source{Source::Range{{3, 16}, {3, 21}}, &file}, "hiss", - System::Test, "abc123"}; - Diagnostic diag_ice{Severity::InternalCompilerError, - Source{Source::Range{{4, 16}, {4, 19}}, &file}, - "unreachable", System::Test}; - Diagnostic diag_fatal{Severity::Fatal, - Source{Source::Range{{4, 16}, {4, 19}}, &file}, - "nothing", System::Test}; + Diagnostic diag_note = + Diag(Severity::Note, + Source{Source::Range{Source::Location{1, 14}}, &file}, + "purr", + System::Test); + Diagnostic diag_warn = Diag(Severity::Warning, + Source{Source::Range{{2, 14}, {2, 18}}, &file}, + "grrr", + System::Test); + Diagnostic diag_err = Diag(Severity::Error, + Source{Source::Range{{3, 16}, {3, 21}}, &file}, + "hiss", + System::Test, + "abc123"); + Diagnostic diag_ice = Diag(Severity::InternalCompilerError, + Source{Source::Range{{4, 16}, {4, 19}}, &file}, + "unreachable", + System::Test); + Diagnostic diag_fatal = Diag(Severity::Fatal, + Source{Source::Range{{4, 16}, {4, 19}}, &file}, + "nothing", + System::Test); }; TEST_F(DiagFormatterTest, Simple) { @@ -69,7 +92,7 @@ TEST_F(DiagFormatterTest, SimpleNewlineAtEnd) { TEST_F(DiagFormatterTest, SimpleNoSource) { Formatter fmt{{false, false, false, false}}; - Diagnostic diag{Severity::Note, Source{}, "no source!", System::Test}; + auto diag = Diag(Severity::Note, Source{}, "no source!", System::Test); auto got = fmt.format(List{diag}); auto* expect = "no source!"; ASSERT_EQ(expect, got); @@ -130,9 +153,9 @@ the snake says quack } TEST_F(DiagFormatterTest, BasicWithMultiLine) { - Diagnostic multiline{Severity::Warning, - Source{Source::Range{{2, 9}, {4, 15}}, &file}, - "multiline", System::Test}; + auto multiline = + Diag(Severity::Warning, Source{Source::Range{{2, 9}, {4, 15}}, &file}, + "multiline", System::Test); Formatter fmt{{false, false, true, false}}; auto got = fmt.format(List{multiline}); auto* expect = R"(2:9: multiline @@ -165,9 +188,9 @@ the snake says quack } TEST_F(DiagFormatterTest, BasicWithMultiLineTab4) { - Diagnostic multiline{Severity::Warning, - Source{Source::Range{{2, 9}, {4, 15}}, &file}, - "multiline", System::Test}; + auto multiline = + Diag(Severity::Warning, Source{Source::Range{{2, 9}, {4, 15}}, &file}, + "multiline", System::Test); Formatter fmt{{false, false, true, false, 4u}}; auto got = fmt.format(List{multiline}); auto* expect = R"(2:9: multiline