From 0847532edcd31e36bddb7e811115a6280657cb7d Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Wed, 13 Sep 2023 16:43:24 -0700 Subject: [PATCH] Refactor file_test autoupdating into a class. (#3228) I'm finding the current autoupdate difficult to reason about. What I'm trying to do here is use the class to make it easier to add helper functions. For example, I merge the vector+cursor for stdout/stderr into an object, passed to helpers together instead of as two parameters. ShouldAddCheckLine can check against output_file_number_ without passing that through a couple levels of function calls. In turn, ShouldAddCheckLine is shared with the end-of-file logic instead of that having its own comparison from what AddCheckLines does. Also, I'm trying to get the pre-AUTOUPDATE edits in their own loop, distinct from the main code. The class means AddRemappedNonCheckLine is a helper function to share code, instead of a lambda (which I was thinking would just confuse the flow further). I'm also changing the input non_check_lines to a single vector to match stdout/stderr. Because of the AUTOUPDATE + SPLIT lines, we're guaranteed to have at least one line per file. I realized file_offset_in_new_lines is redundant with output_line_number so code now uses the latter (because it's older -- I think one's as good as the other, otherwise). Note, this change deliberately does not affect output. I'm only trying to make it easier to read for the next changes, using the lack of change in results as a good indicator that this is getting it right. --- testing/file_test/autoupdate.cpp | 477 +++++++++++---------------- testing/file_test/autoupdate.h | 202 ++++++++++-- testing/file_test/file_test_base.cpp | 26 +- testing/file_test/file_test_base.h | 15 +- testing/file_test/line.h | 11 +- 5 files changed, 406 insertions(+), 325 deletions(-) diff --git a/testing/file_test/autoupdate.cpp b/testing/file_test/autoupdate.cpp index b7a87f369c8d..96e3cccc03aa 100644 --- a/testing/file_test/autoupdate.cpp +++ b/testing/file_test/autoupdate.cpp @@ -10,15 +10,11 @@ #include "common/check.h" #include "common/ostream.h" #include "llvm/ADT/DenseMap.h" -#include "llvm/ADT/STLFunctionalExtras.h" #include "llvm/ADT/StringExtras.h" #include "llvm/Support/FormatVariadic.h" namespace Carbon::Testing { -// Put helper classes in an anonymous namespace. -namespace { - // Converts a matched line number to an int, trimming whitespace. static auto ParseLineNumber(absl::string_view matched_line_number) -> int { llvm::StringRef trimmed = matched_line_number; @@ -30,138 +26,87 @@ static auto ParseLineNumber(absl::string_view matched_line_number) -> int { return val; } -// The file and line number that a CHECK line refers to, and the -// replacement from which they were determined, if any. -struct FileAndLineNumber { - explicit FileAndLineNumber(int file_number) : file_number(file_number) {} +FileTestAutoupdater::FileAndLineNumber::FileAndLineNumber( + const LineNumberReplacement* replacement, int file_number, + absl::string_view line_number) + : replacement(replacement), + file_number(file_number), + line_number(ParseLineNumber(line_number)) {} - explicit FileAndLineNumber(const FileTestLineNumberReplacement* replacement, - int file_number, absl::string_view line_number) - : replacement(replacement), - file_number(file_number), - line_number(ParseLineNumber(line_number)) {} - - const FileTestLineNumberReplacement* replacement = nullptr; - int file_number; - int line_number = -1; -}; - -class CheckLine : public FileTestLineBase { - public: - // RE2 is passed by a pointer because it doesn't support std::optional. - explicit CheckLine(FileAndLineNumber file_and_line_number, std::string line) - : FileTestLineBase(file_and_line_number.line_number), - file_number_(file_and_line_number.file_number), - replacement_(file_and_line_number.replacement), - line_(std::move(line)) {} - - auto Print(llvm::raw_ostream& out) const -> void override { - out << indent_ << line_; +auto FileTestAutoupdater::CheckLine::RemapLineNumbers( + const llvm::DenseMap, int>& output_line_remap, + const llvm::SmallVector& new_last_line_numbers) -> void { + // Only need to do remappings when there's a line number replacement. + if (!replacement_) { + return; } - // When the location of the CHECK in output is known, we can set the indent - // and its line. - auto SetOutputLine(llvm::StringRef indent, int output_file_number, - int output_line_number) -> void { - indent_ = indent; - output_file_number_ = output_file_number; - output_line_number_ = output_line_number; - } - - // When the location of all lines in a file are known, we can set the line - // offset based on the target line. - auto RemapLineNumbers( - const llvm::DenseMap, int>& output_line_remap, - const llvm::SmallVector& new_last_line_numbers) -> void { - // Only need to do remappings when there's a line number replacement. - if (!replacement_) { + bool found_one = false; + // Use a cursor for the line so that we can't keep matching the same + // content, which may occur when we keep a literal line number. + int line_offset = 0; + while (true) { + // Rebuild the cursor each time because we're editing the line, which + // could cause a reallocation. + absl::string_view line_cursor = line_; + line_cursor.remove_prefix(line_offset); + // Look for a line number to replace. There may be multiple, so we + // repeatedly check. + absl::string_view matched_line_number; + if (replacement_->has_file) { + RE2::PartialMatch(line_cursor, *replacement_->re, nullptr, + &matched_line_number); + } else { + RE2::PartialMatch(line_cursor, *replacement_->re, &matched_line_number); + } + if (matched_line_number.empty()) { + CARBON_CHECK(found_one) << line_; return; } + found_one = true; - bool found_one = false; - // Use a cursor for the line so that we can't keep matching the same - // content, which may occur when we keep a literal line number. - int line_offset = 0; - while (true) { - // Rebuild the cursor each time because we're editing the line, which - // could cause a reallocation. - absl::string_view line_cursor = line_; - line_cursor.remove_prefix(line_offset); - // Look for a line number to replace. There may be multiple, so we - // repeatedly check. - absl::string_view matched_line_number; - if (replacement_->has_file) { - RE2::PartialMatch(line_cursor, *replacement_->re, nullptr, - &matched_line_number); - } else { - RE2::PartialMatch(line_cursor, *replacement_->re, &matched_line_number); - } - if (matched_line_number.empty()) { - CARBON_CHECK(found_one) << line_; - return; - } - found_one = true; + // Update the cursor offset from the match. + line_offset = matched_line_number.begin() - line_.c_str(); - // Update the cursor offset from the match. - line_offset = matched_line_number.begin() - line_.c_str(); - - // Calculate the new line number (possibly with new CHECK lines added, or - // some removed). - int old_line_number = ParseLineNumber(matched_line_number); - int new_line_number = -1; - if (auto remapped = - output_line_remap.find({file_number_, old_line_number}); - remapped != output_line_remap.end()) { - // Map old non-check lines to their new line numbers. - new_line_number = remapped->second; - } else { - // We assume unmapped references point to the end-of-file. - new_line_number = new_last_line_numbers[file_number_]; - } - - std::string replacement; - if (output_file_number_ == file_number_) { - int offset = new_line_number - output_line_number_; - // Update the line offset in the CHECK line. - const char* offset_prefix = offset < 0 ? "" : "+"; - replacement = llvm::formatv( - replacement_->line_formatv.c_str(), - llvm::formatv("[[@LINE{0}{1}]]", offset_prefix, offset)); - } else { - // If the CHECK was written to a different file from the file that it - // refers to, leave behind an absolute line reference rather than a - // cross-file offset. - replacement = - llvm::formatv(replacement_->line_formatv.c_str(), new_line_number); - } - line_.replace(matched_line_number.data() - line_.data(), - matched_line_number.size(), replacement); + // Calculate the new line number (possibly with new CHECK lines added, or + // some removed). + int old_line_number = ParseLineNumber(matched_line_number); + int new_line_number = -1; + if (auto remapped = + output_line_remap.find({file_number(), old_line_number}); + remapped != output_line_remap.end()) { + // Map old non-check lines to their new line numbers. + new_line_number = remapped->second; + } else { + // We assume unmapped references point to the end-of-file. + new_line_number = new_last_line_numbers[file_number()]; } + + std::string replacement; + if (output_file_number_ == file_number()) { + int offset = new_line_number - output_line_number_; + // Update the line offset in the CHECK line. + const char* offset_prefix = offset < 0 ? "" : "+"; + replacement = llvm::formatv( + replacement_->line_formatv.c_str(), + llvm::formatv("[[@LINE{0}{1}]]", offset_prefix, offset)); + } else { + // If the CHECK was written to a different file from the file that it + // refers to, leave behind an absolute line reference rather than a + // cross-file offset. + replacement = + llvm::formatv(replacement_->line_formatv.c_str(), new_line_number); + } + line_.replace(matched_line_number.data() - line_.data(), + matched_line_number.size(), replacement); } +} - auto file_number() const -> int { return file_number_; } - - auto is_blank() const -> bool override { return false; } - - private: - int file_number_; - const FileTestLineNumberReplacement* replacement_; - std::string line_; - llvm::StringRef indent_; - int output_file_number_ = -1; - int output_line_number_ = -1; -}; - -} // namespace - -// Looks for the patterns in the line. Returns the first match, or defaulted -// information if not found. -static auto GetFileAndLineNumber( - const llvm::SmallVector& replacements, +auto FileTestAutoupdater::GetFileAndLineNumber( llvm::DenseMap file_to_number_map, int default_file_number, const std::string& check_line) -> FileAndLineNumber { - for (const auto& replacement : replacements) { + for (const auto& replacement : line_number_replacements_) { if (replacement.has_file) { absl::string_view filename; absl::string_view line_number; @@ -187,22 +132,15 @@ static auto GetFileAndLineNumber( return FileAndLineNumber(default_file_number); } -// Builds CheckLine lists for autoupdate. -static auto BuildCheckLines( - llvm::StringRef output, const char* label, - const llvm::SmallVector& filenames, - const std::optional& default_file_re, - const llvm::SmallVector& replacements, - std::function do_extra_check_replacements) - -> llvm::SmallVector { - llvm::SmallVector check_lines; +auto FileTestAutoupdater::BuildCheckLines(llvm::StringRef output, + const char* label) -> CheckLines { if (output.empty()) { - return check_lines; + return CheckLines({}); } // Prepare to look for filenames in lines. llvm::DenseMap file_to_number_map; - for (auto [number, name] : llvm::enumerate(filenames)) { + for (auto [number, name] : llvm::enumerate(filenames_)) { file_to_number_map.insert({name, number}); } @@ -227,6 +165,7 @@ static auto BuildCheckLines( // The default file number for when no specific file is found. int default_file_number = 0; + llvm::SmallVector check_lines; for (const auto& line : lines) { std::string check_line = llvm::formatv("// CHECK:{0}:{1}{2}", label, line.empty() ? "" : " ", line); @@ -239,187 +178,159 @@ static auto BuildCheckLines( check_line.replace(pos, tmpdir.size(), "{{.+}}"); } - do_extra_check_replacements(check_line); + do_extra_check_replacements_(check_line); - if (default_file_re) { + if (default_file_re_) { absl::string_view filename; - if (RE2::PartialMatch(line, *default_file_re, &filename)) { + if (RE2::PartialMatch(line, *default_file_re_, &filename)) { auto it = file_to_number_map.find(filename); CARBON_CHECK(it != file_to_number_map.end()) << "default_file_re had unexpected match in '" << line << "' (`" - << default_file_re->pattern() << "`)"; + << default_file_re_->pattern() << "`)"; default_file_number = it->second; } } - auto file_and_line = GetFileAndLineNumber(replacements, file_to_number_map, + auto file_and_line = GetFileAndLineNumber(file_to_number_map, default_file_number, check_line); check_lines.push_back(CheckLine(file_and_line, check_line)); } - - return check_lines; + return CheckLines(check_lines); } -auto AutoupdateFileTest( - const std::filesystem::path& file_test_path, llvm::StringRef input_content, - const llvm::SmallVector& filenames, - int autoupdate_line_number, - const llvm::SmallVector>& non_check_lines, - llvm::StringRef stdout, llvm::StringRef stderr, - const std::optional& default_file_re, - const llvm::SmallVector& - line_number_replacements, - std::function do_extra_check_replacements) -> bool { - for (const auto& replacement : line_number_replacements) { - CARBON_CHECK(replacement.has_file || default_file_re) - << "For replacement with pattern `" << replacement.re->pattern() - << "` to have has_file=false, override GetDefaultFileRE."; - CARBON_CHECK(replacement.re->ok()) - << "Invalid line replacement RE2: " << replacement.re->error(); +auto FileTestAutoupdater::AddRemappedNonCheckLine() -> void { + new_lines_.push_back(non_check_line_); + CARBON_CHECK(output_line_remap_ + .insert({{non_check_line_->file_number(), + non_check_line_->line_number()}, + ++output_line_number_}) + .second); +} + +auto FileTestAutoupdater::ShouldAddCheckLine(const CheckLines& check_lines, + int to_line_number) const -> bool { + return check_lines.cursor != check_lines.lines.end() && + (check_lines.cursor->file_number() < output_file_number_ || + (check_lines.cursor->file_number() == output_file_number_ && + check_lines.cursor->line_number() <= to_line_number)); +} + +auto FileTestAutoupdater::AddCheckLines(CheckLines& check_lines, + int to_line_number, + llvm::StringRef indent) -> void { + for (; ShouldAddCheckLine(check_lines, to_line_number); + ++check_lines.cursor) { + new_lines_.push_back(check_lines.cursor); + check_lines.cursor->SetOutputLine(indent, output_file_number_, + ++output_line_number_); + } +} + +auto FileTestAutoupdater::FinishFile() -> void { + // At the end of each file, print any remaining lines which are associated + // with the file. + if (ShouldAddCheckLine(stderr_, INT_MAX) || + ShouldAddCheckLine(stdout_, INT_MAX)) { + // Ensure there's a blank line before any trailing CHECKs. + if (!new_lines_.empty() && !new_lines_.back()->is_blank()) { + new_lines_.push_back(&blank_line_); + ++output_line_number_; + } + + AddCheckLines(stderr_, INT_MAX, ""); + AddCheckLines(stdout_, INT_MAX, ""); } - // Prepare CHECK lines. - llvm::SmallVector stdout_check_lines = - BuildCheckLines(stdout, "STDOUT", filenames, default_file_re, - line_number_replacements, do_extra_check_replacements); - llvm::SmallVector stderr_check_lines = - BuildCheckLines(stderr, "STDERR", filenames, default_file_re, - line_number_replacements, do_extra_check_replacements); - auto* stdout_check_line = stdout_check_lines.begin(); - auto* stderr_check_line = stderr_check_lines.begin(); + new_last_line_numbers_.push_back(output_line_number_); +} +auto FileTestAutoupdater::StartSplitFile() -> void { + // Advance the file. + ++output_file_number_; + output_line_number_ = 0; + CARBON_CHECK(output_file_number_ == non_check_line_->file_number()) + << "Non-sequential file: " << non_check_line_->file_number(); + + // Each following file has precisely one split line. + CARBON_CHECK(non_check_line_->line_number() < 1) + << "Expected a split line, got " << *non_check_line_; + // The split line is ignored when calculating line counts. + new_lines_.push_back(non_check_line_); + ++non_check_line_; +} + +auto FileTestAutoupdater::Run() -> bool { bool any_attached_stdout_lines = std::any_of( - stdout_check_lines.begin(), stdout_check_lines.end(), + stdout_.lines.begin(), stdout_.lines.end(), [&](const CheckLine& line) { return line.line_number() != -1; }); - // All CHECK lines are suppressed until we reach AUTOUPDATE. - bool reached_autoupdate = false; - - const FileTestLine blank_line(-1, ""); - - // Maps {file_number, original line number} to a new line number. - llvm::DenseMap, int> output_line_remap; - - // Tracks the new last line numbers for each file. - llvm::SmallVector new_last_line_numbers; - new_last_line_numbers.reserve(filenames.size()); - - // Stitch together content. - llvm::SmallVector new_lines; - for (auto [file_number_as_size_t, filename, non_check_file] : - llvm::enumerate(filenames, non_check_lines)) { - auto file_number = static_cast(file_number_as_size_t); - int output_line_number = 0; - - // Track the offset in new_lines to later determine the line count. - int file_offset_in_new_lines = new_lines.size(); - - // Add all check lines from the given vector until we reach a check line - // attached to a line later than `to_line_number`. - auto add_check_lines = [&](const llvm::SmallVector& lines, - CheckLine*& line, int to_line_number, - llvm::StringRef indent) { - for (; line != lines.end() && (line->file_number() < file_number || - (line->file_number() == file_number && - line->line_number() <= to_line_number)); - ++line) { - new_lines.push_back(line); - line->SetOutputLine(indent, file_number, ++output_line_number); - } - }; - - // Looping through the original file, print check lines preceding each - // original line. - for (const auto& non_check_line : non_check_file) { - // If there are any non-check lines with an invalid line_number, it's - // something like a split directive which shouldn't increment - // output_line_number. - if (non_check_line.line_number() < 1) { - // These are ignored when calculating line counts. - CARBON_CHECK(file_offset_in_new_lines == - static_cast(new_lines.size())); - ++file_offset_in_new_lines; - new_lines.push_back(&non_check_line); - continue; - } - - // STDERR check lines are placed before the line they refer to, or as - // early as possible if they don't refer to a line. Include all STDERR - // lines until we find one that wants to go later in the file. - if (reached_autoupdate) { - add_check_lines(stderr_check_lines, stderr_check_line, - non_check_line.line_number(), non_check_line.indent()); - } else if (autoupdate_line_number == non_check_line.line_number()) { - // This is the AUTOUPDATE line, so we'll print it, then start printing - // CHECK lines. - reached_autoupdate = true; - } - - new_lines.push_back(&non_check_line); - CARBON_CHECK(output_line_remap - .insert({{file_number, non_check_line.line_number()}, - ++output_line_number}) - .second); - - // If we just added the AUTOUPDATE line, include any early STDERR lines - // now, so that the initial batch of CHECK lines have STDERR before - // STDOUT. This also ensures we don't insert a blank line before the - // STDERR checks if there are no more lines after AUTOUPDATE. - if (autoupdate_line_number == non_check_line.line_number()) { - add_check_lines(stderr_check_lines, stderr_check_line, - non_check_line.line_number(), non_check_line.indent()); - } - - // STDOUT check lines are placed after the line they refer to, or at the - // end of the file if none of them refers to a line. - if (reached_autoupdate && any_attached_stdout_lines) { - add_check_lines(stdout_check_lines, stdout_check_line, - non_check_line.line_number(), non_check_line.indent()); - } - } - - // This should always be true after the first file is processed. - CARBON_CHECK(reached_autoupdate); - - // At the end of each file, print any remaining lines which are associated - // with the file. - if ((stderr_check_line != stderr_check_lines.end() && - stderr_check_line->file_number() == file_number) || - (stdout_check_line != stdout_check_lines.end() && - stdout_check_line->file_number() == file_number)) { - // Ensure there's a blank line before any trailing CHECKs. - if (!new_lines.empty() && !new_lines.back()->is_blank()) { - new_lines.push_back(&blank_line); - ++output_line_number; - } - - add_check_lines(stderr_check_lines, stderr_check_line, INT_MAX, ""); - add_check_lines(stdout_check_lines, stdout_check_line, INT_MAX, ""); - } - - new_last_line_numbers.push_back(new_lines.size() - - file_offset_in_new_lines); + // Print everything until the autoupdate line. + while (non_check_line_->line_number() != autoupdate_line_number_) { + CARBON_CHECK(non_check_line_ != non_check_lines_.end() && + non_check_line_->file_number() == 0) + << "Missed autoupdate?"; + AddRemappedNonCheckLine(); + ++non_check_line_; } - for (auto& check_line : stdout_check_lines) { - check_line.RemapLineNumbers(output_line_remap, new_last_line_numbers); + // Add the AUTOUPDATE line along with any early STDERR lines, so that the + // initial batch of CHECK lines have STDERR before STDOUT. This also ensures + // we don't insert a blank line before the STDERR checks if there are no more + // lines after AUTOUPDATE. + AddRemappedNonCheckLine(); + AddCheckLines(stderr_, non_check_line_->line_number(), + non_check_line_->indent()); + if (any_attached_stdout_lines) { + AddCheckLines(stdout_, non_check_line_->line_number(), + non_check_line_->indent()); } - for (auto& check_line : stderr_check_lines) { - check_line.RemapLineNumbers(output_line_remap, new_last_line_numbers); + ++non_check_line_; + + // Loop through remaining content. + while (non_check_line_ != non_check_lines_.end()) { + if (output_file_number_ < non_check_line_->file_number()) { + FinishFile(); + StartSplitFile(); + continue; + } + + // STDERR check lines are placed before the line they refer to, or as + // early as possible if they don't refer to a line. Include all STDERR + // lines until we find one that wants to go later in the file. + AddCheckLines(stderr_, non_check_line_->line_number(), + non_check_line_->indent()); + AddRemappedNonCheckLine(); + + // STDOUT check lines are placed after the line they refer to, or at the + // end of the file if none of them refers to a line. + if (any_attached_stdout_lines) { + AddCheckLines(stdout_, non_check_line_->line_number(), + non_check_line_->indent()); + } + + ++non_check_line_; + } + + FinishFile(); + + for (auto& check_line : stdout_.lines) { + check_line.RemapLineNumbers(output_line_remap_, new_last_line_numbers_); + } + for (auto& check_line : stderr_.lines) { + check_line.RemapLineNumbers(output_line_remap_, new_last_line_numbers_); } // Generate the autoupdated file. std::string new_content; llvm::raw_string_ostream new_content_stream(new_content); - for (const auto& line : new_lines) { - line->Print(new_content_stream); - new_content_stream << '\n'; + for (const auto& line : new_lines_) { + new_content_stream << *line << '\n'; } // Update the file on disk if needed. - if (new_content == input_content) { + if (new_content == input_content_) { return false; } - std::ofstream out(file_test_path); + std::ofstream out(file_test_path_); out << new_content; return true; } diff --git a/testing/file_test/autoupdate.h b/testing/file_test/autoupdate.h index cdc4956cfc6d..2d9971cc0e4e 100644 --- a/testing/file_test/autoupdate.h +++ b/testing/file_test/autoupdate.h @@ -6,7 +6,10 @@ #define CARBON_TESTING_FILE_TEST_AUTOUPDATE_H_ #include +#include +#include "common/check.h" +#include "llvm/ADT/DenseMap.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" #include "re2/re2.h" @@ -14,32 +17,187 @@ namespace Carbon::Testing { -struct FileTestLineNumberReplacement { - bool has_file; +class FileTestAutoupdater { + public: + struct LineNumberReplacement { + bool has_file; - // The line replacement. The pattern should match lines. If has_file, pattern - // should have a file and line group; otherwise, only a line group, but - // default_file_re should be provided. - // - // Uses shared_ptr for storage in SmallVector. - std::shared_ptr re; + // The line replacement. The pattern should match lines. If has_file, + // pattern should have a file and line group; otherwise, only a line group, + // but default_file_re should be provided. + // + // Uses shared_ptr for storage in SmallVector. + std::shared_ptr re; - // line_formatv should provide {0} to substitute with [[@LINE...]] deltas. - std::string line_formatv; + // line_formatv should provide {0} to substitute with [[@LINE...]] deltas. + std::string line_formatv; + }; + + explicit FileTestAutoupdater( + const std::filesystem::path& file_test_path, + llvm::StringRef input_content, + const llvm::SmallVector& filenames, + int autoupdate_line_number, + const llvm::SmallVector& non_check_lines, + llvm::StringRef stdout, llvm::StringRef stderr, + const std::optional& default_file_re, + const llvm::SmallVector& line_number_replacements, + std::function do_extra_check_replacements) + : file_test_path_(file_test_path), + input_content_(input_content), + filenames_(filenames), + autoupdate_line_number_(autoupdate_line_number), + non_check_lines_(non_check_lines), + default_file_re_(default_file_re), + line_number_replacements_(line_number_replacements), + do_extra_check_replacements_(std::move(do_extra_check_replacements)), + // BuildCheckLines should only be called after other member + // initialization. + stdout_(BuildCheckLines(stdout, "STDOUT")), + stderr_(BuildCheckLines(stderr, "STDERR")), + non_check_line_(non_check_lines_.begin()) { + for (const auto& replacement : line_number_replacements_) { + CARBON_CHECK(replacement.has_file || default_file_re_) + << "For replacement with pattern `" << replacement.re->pattern() + << "` to have has_file=false, override GetDefaultFileRE."; + CARBON_CHECK(replacement.re->ok()) + << "Invalid line replacement RE2: " << replacement.re->error(); + } + } + + // Automatically updates CHECKs in the provided file. Returns true if updated. + auto Run() -> bool; + + private: + // The file and line number that a CHECK line refers to, and the + // replacement from which they were determined, if any. + struct FileAndLineNumber { + explicit FileAndLineNumber(int file_number) : file_number(file_number) {} + + explicit FileAndLineNumber(const LineNumberReplacement* replacement, + int file_number, absl::string_view line_number); + + const LineNumberReplacement* replacement = nullptr; + int file_number; + int line_number = -1; + }; + + // A CHECK line which is integrated into autoupdate output. + class CheckLine : public FileTestLineBase { + public: + // RE2 is passed by a pointer because it doesn't support std::optional. + explicit CheckLine(FileAndLineNumber file_and_line_number, std::string line) + : FileTestLineBase(file_and_line_number.file_number, + file_and_line_number.line_number), + replacement_(file_and_line_number.replacement), + line_(std::move(line)) {} + + auto Print(llvm::raw_ostream& out) const -> void override { + out << indent_ << line_; + } + + // When the location of the CHECK in output is known, we can set the indent + // and its line. + auto SetOutputLine(llvm::StringRef indent, int output_file_number, + int output_line_number) -> void { + indent_ = indent; + output_file_number_ = output_file_number; + output_line_number_ = output_line_number; + } + + // When the location of all lines in a file are known, we can set the line + // offset based on the target line. + auto RemapLineNumbers( + const llvm::DenseMap, int>& output_line_remap, + const llvm::SmallVector& new_last_line_numbers) -> void; + + auto is_blank() const -> bool override { return false; } + + private: + const LineNumberReplacement* replacement_; + std::string line_; + llvm::StringRef indent_; + int output_file_number_ = -1; + int output_line_number_ = -1; + }; + + // Clusters information for stdout and stderr. + struct CheckLines { + explicit CheckLines(llvm::SmallVector lines) + : lines(std::move(lines)), cursor(this->lines.begin()) {} + + // The full list of check lines. + llvm::SmallVector lines; + // An iterator into check_lines. + CheckLine* cursor; + }; + + // Looks for the patterns in the line. Returns the first match, or defaulted + // information if not found. + auto GetFileAndLineNumber( + llvm::DenseMap file_to_number_map, + int default_file_number, const std::string& check_line) + -> FileAndLineNumber; + + // Builds CheckLine lists for autoupdate. + auto BuildCheckLines(llvm::StringRef output, const char* label) -> CheckLines; + + // Adds a non-check line to the new_lines and output_line_remap. The caller + // still needs to advance the cursor when ready. + auto AddRemappedNonCheckLine() -> void; + + // Returns true if there's a CheckLine that should be added at + // `to_line_number`. + auto ShouldAddCheckLine(const CheckLines& check_lines, + int to_line_number) const -> bool; + + // Add all check lines from the given vector until we reach a check line + // attached to a line later than `to_line_number`. + auto AddCheckLines(CheckLines& check_lines, int to_line_number, + llvm::StringRef indent) -> void; + + // Adds remaining check lines for the current file. + auto FinishFile() -> void; + + // Starts a new split file, updating file and line numbers. Advances past the + // split line. + auto StartSplitFile() -> void; + + // Passed-in state. + const std::filesystem::path& file_test_path_; + llvm::StringRef input_content_; + const llvm::SmallVector& filenames_; + int autoupdate_line_number_; + const llvm::SmallVector& non_check_lines_; + const std::optional& default_file_re_; + const llvm::SmallVector& line_number_replacements_; + std::function do_extra_check_replacements_; + + // The constructed CheckLine list and cursor. + CheckLines stdout_; + CheckLines stderr_; + + // Iterators for the main Run loop. + const FileTestLine* non_check_line_; + + // Tracks the new last line numbers for each file. + llvm::SmallVector new_last_line_numbers_; + + // A reusable blank line. new_lines_ can contain a reference back to it. + const FileTestLine blank_line_ = FileTestLine(-1, -1, ""); + + // Stitched-together content. + llvm::SmallVector new_lines_; + + // Maps {file_number, original line number} to a new line number. + llvm::DenseMap, int> output_line_remap_; + + // The current output file number; mainly used for tracking progression. + int output_file_number_ = 0; + // The current output line number in stitched content. + int output_line_number_ = 0; }; -// Automatically updates CHECKs in the provided file. Returns true if updated. -auto AutoupdateFileTest( - const std::filesystem::path& file_test_path, llvm::StringRef input_content, - const llvm::SmallVector& filenames, - int autoupdate_line_number, - const llvm::SmallVector>& non_check_lines, - llvm::StringRef stdout, llvm::StringRef stderr, - const std::optional& default_file_re, - const llvm::SmallVector& - line_number_replacements, - std::function do_extra_check_replacements) -> bool; - } // namespace Carbon::Testing #endif // CARBON_TESTING_FILE_TEST_AUTOUPDATE_H_ diff --git a/testing/file_test/file_test_base.cpp b/testing/file_test/file_test_base.cpp index d4bc682e0d48..e020132d61a4 100644 --- a/testing/file_test/file_test_base.cpp +++ b/testing/file_test/file_test_base.cpp @@ -19,6 +19,7 @@ #include "llvm/Support/InitLLVM.h" #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/PrettyStackTrace.h" +#include "testing/file_test/autoupdate.h" ABSL_FLAG(std::vector, file_tests, {}, "A comma-separated list of repo-relative names of test files. " @@ -111,7 +112,7 @@ auto FileTestBase::Autoupdate() -> ErrorOr { llvm::SmallVector filenames; filenames.reserve(context.non_check_lines.size()); - if (context.non_check_lines.size() > 1) { + if (context.has_splits) { // There are splits, so we provide an empty name for the first file. filenames.push_back({}); } @@ -124,12 +125,14 @@ auto FileTestBase::Autoupdate() -> ErrorOr { expected_filenames = expected_filenames.drop_front(); } - return AutoupdateFileTest( - std::filesystem::absolute(test_name_.str()), context.input_content, - filenames, *context.autoupdate_line_number, context.non_check_lines, - context.stdout, context.stderr, GetDefaultFileRE(expected_filenames), - GetLineNumberReplacements(expected_filenames), - [&](std::string& line) { DoExtraCheckReplacements(line); }); + return FileTestAutoupdater( + std::filesystem::absolute(test_name_.str()), context.input_content, + filenames, *context.autoupdate_line_number, + context.non_check_lines, context.stdout, context.stderr, + GetDefaultFileRE(expected_filenames), + GetLineNumberReplacements(expected_filenames), + [&](std::string& line) { DoExtraCheckReplacements(line); }) + .Run(); } auto FileTestBase::GetLineNumberReplacements( @@ -253,7 +256,7 @@ auto FileTestBase::ProcessTestFile(TestContext& context) -> ErrorOr { // The current file's start. const char* current_file_start = nullptr; - context.non_check_lines.resize(1); + int file_number = 0; while (!cursor.empty()) { auto [line, next_cursor] = cursor.split("\n"); cursor = next_cursor; @@ -269,7 +272,9 @@ auto FileTestBase::ProcessTestFile(TestContext& context) -> ErrorOr { << "AUTOUPDATE/NOAUTOUPDATE setting must be in the first file."; } - context.non_check_lines.push_back({FileTestLine(0, line)}); + context.has_splits = true; + ++file_number; + context.non_check_lines.push_back(FileTestLine(file_number, 0, line)); // On a file split, add the previous file, then start a new one. if (current_file_start) { context.test_files.push_back(TestFile( @@ -311,7 +316,8 @@ auto FileTestBase::ProcessTestFile(TestContext& context) -> ErrorOr { expected->push_back(check_matcher); } } else { - context.non_check_lines.back().push_back(FileTestLine(line_index, line)); + context.non_check_lines.push_back( + FileTestLine(file_number, line_index, line)); if (line_trimmed.consume_front("// ARGS: ")) { if (context.test_args.empty()) { // Split the line into arguments. diff --git a/testing/file_test/file_test_base.h b/testing/file_test/file_test_base.h index 9d008dfc7b34..37fd9fb088ed 100644 --- a/testing/file_test/file_test_base.h +++ b/testing/file_test/file_test_base.h @@ -40,7 +40,7 @@ class FileTestBase : public testing::Test { }; // Provided for child class convenience. - using LineNumberReplacement = FileTestLineNumberReplacement; + using LineNumberReplacement = FileTestAutoupdater::LineNumberReplacement; explicit FileTestBase(llvm::StringRef test_name) : test_name_(test_name) {} @@ -100,12 +100,15 @@ class FileTestBase : public testing::Test { std::string input_content; // Lines which don't contain CHECKs, and thus need to be retained by - // autoupdate. Their line number in the file is attached. + // autoupdate. Their file and line numbers are attached. // - // If there are splits, then the line is in the respective file. For N - // splits, there will be one vector for the parts of the input file which - // are not in any split, plus one vector per split file. - llvm::SmallVector> non_check_lines; + // If there are splits, then the splitting line is in the respective file. + // For N splits, the 0th file is the parts of the input file which are not + // in any split, plus one file per split file. + llvm::SmallVector non_check_lines; + + // Whether there are splits. + bool has_splits = false; // Arguments for the test, generated from ARGS. llvm::SmallVector test_args; diff --git a/testing/file_test/line.h b/testing/file_test/line.h index 01fe13c62dc5..e0783fed3c4f 100644 --- a/testing/file_test/line.h +++ b/testing/file_test/line.h @@ -11,9 +11,10 @@ namespace Carbon::Testing { // Interface for lines. -class FileTestLineBase { +class FileTestLineBase : public Printable { public: - explicit FileTestLineBase(int line_number) : line_number_(line_number) {} + explicit FileTestLineBase(int file_number, int line_number) + : file_number_(file_number), line_number_(line_number) {} virtual ~FileTestLineBase() {} // Prints the autoupdated line. @@ -21,17 +22,19 @@ class FileTestLineBase { virtual auto is_blank() const -> bool = 0; + auto file_number() const -> int { return file_number_; } auto line_number() const -> int { return line_number_; } private: + int file_number_; int line_number_; }; // A line in the original file test. class FileTestLine : public FileTestLineBase { public: - explicit FileTestLine(int line_number, llvm::StringRef line) - : FileTestLineBase(line_number), line_(line) {} + explicit FileTestLine(int file_number, int line_number, llvm::StringRef line) + : FileTestLineBase(file_number, line_number), line_(line) {} auto Print(llvm::raw_ostream& out) const -> void override { out << line_; }