From 7bbf4fbba3b35d8ea1d15d8296f9195b58be8b4b Mon Sep 17 00:00:00 2001 From: Jon Meow Date: Tue, 15 Mar 2022 15:43:53 -0700 Subject: [PATCH] Accessor renames on diagnostics+source buffer (#1133) --- toolchain/diagnostics/diagnostic_emitter.h | 6 +++--- toolchain/lexer/numeric_literal_test.cpp | 21 ++++++++++--------- toolchain/lexer/string_literal_test.cpp | 14 ++++++------- toolchain/lexer/tokenized_buffer.cpp | 24 +++++++++++----------- toolchain/parser/parse_tree_test.cpp | 14 ++++++------- toolchain/source/source_buffer.h | 4 ++-- toolchain/source/source_buffer_test.cpp | 16 +++++++-------- 7 files changed, 50 insertions(+), 49 deletions(-) diff --git a/toolchain/diagnostics/diagnostic_emitter.h b/toolchain/diagnostics/diagnostic_emitter.h index 98ced89a5936..2985b940441c 100644 --- a/toolchain/diagnostics/diagnostic_emitter.h +++ b/toolchain/diagnostics/diagnostic_emitter.h @@ -184,12 +184,12 @@ class ErrorTrackingDiagnosticConsumer : public DiagnosticConsumer { next_consumer_->HandleDiagnostic(diagnostic); } - // Returns whether we've seen an error since the last reset. - auto SeenError() const -> bool { return seen_error_; } - // Reset whether we've seen an error. auto Reset() -> void { seen_error_ = false; } + // Returns whether we've seen an error since the last reset. + auto seen_error() const -> bool { return seen_error_; } + private: DiagnosticConsumer* next_consumer_; bool seen_error_ = false; diff --git a/toolchain/lexer/numeric_literal_test.cpp b/toolchain/lexer/numeric_literal_test.cpp index d4b4734ca39c..e5f51abb15a4 100644 --- a/toolchain/lexer/numeric_literal_test.cpp +++ b/toolchain/lexer/numeric_literal_test.cpp @@ -101,7 +101,7 @@ TEST_F(NumericLiteralTest, HandlesIntegerLiteral) { EXPECT_THAT(Parse(testcase.token), HasIntValue(IsUnsignedInteger(testcase.value))) << testcase.token; - EXPECT_FALSE(error_tracker.SeenError()) << testcase.token; + EXPECT_FALSE(error_tracker.seen_error()) << testcase.token; } } @@ -123,7 +123,7 @@ TEST_F(NumericLiteralTest, ValidatesBaseSpecifier) { for (llvm::StringLiteral literal : valid) { error_tracker.Reset(); EXPECT_THAT(Parse(literal), HasIntValue(_)) << literal; - EXPECT_FALSE(error_tracker.SeenError()) << literal; + EXPECT_FALSE(error_tracker.seen_error()) << literal; } llvm::StringLiteral invalid[] = { @@ -135,7 +135,7 @@ TEST_F(NumericLiteralTest, ValidatesBaseSpecifier) { for (llvm::StringLiteral literal : invalid) { error_tracker.Reset(); EXPECT_THAT(Parse(literal), HasUnrecoverableError()) << literal; - EXPECT_TRUE(error_tracker.SeenError()) << literal; + EXPECT_TRUE(error_tracker.seen_error()) << literal; } } @@ -158,7 +158,7 @@ TEST_F(NumericLiteralTest, ValidatesIntegerDigitSeparators) { for (llvm::StringLiteral literal : valid) { error_tracker.Reset(); EXPECT_THAT(Parse(literal), HasIntValue(_)) << literal; - EXPECT_FALSE(error_tracker.SeenError()) << literal; + EXPECT_FALSE(error_tracker.seen_error()) << literal; } llvm::StringLiteral invalid[] = { @@ -185,7 +185,7 @@ TEST_F(NumericLiteralTest, ValidatesIntegerDigitSeparators) { for (llvm::StringLiteral literal : invalid) { error_tracker.Reset(); EXPECT_THAT(Parse(literal), HasIntValue(_)) << literal; - EXPECT_TRUE(error_tracker.SeenError()) << literal; + EXPECT_TRUE(error_tracker.seen_error()) << literal; } } @@ -242,7 +242,8 @@ TEST_F(NumericLiteralTest, HandlesRealLiteral) { .mantissa = IsUnsignedInteger(testcase.mantissa), .exponent = IsSignedInteger(testcase.exponent)})) << testcase.token; - EXPECT_EQ(error_tracker.SeenError(), testcase.radix == 2) << testcase.token; + EXPECT_EQ(error_tracker.seen_error(), testcase.radix == 2) + << testcase.token; } } @@ -257,7 +258,7 @@ TEST_F(NumericLiteralTest, HandlesRealLiteralOverflow) { return (exponent + 9223372036854775800).getSExtValue() == -24; })})); - EXPECT_FALSE(error_tracker.SeenError()); + EXPECT_FALSE(error_tracker.seen_error()); } TEST_F(NumericLiteralTest, ValidatesRealLiterals) { @@ -269,7 +270,7 @@ TEST_F(NumericLiteralTest, ValidatesRealLiterals) { for (llvm::StringLiteral literal : invalid_digit_separators) { error_tracker.Reset(); EXPECT_THAT(Parse(literal), HasRealValue({})) << literal; - EXPECT_TRUE(error_tracker.SeenError()) << literal; + EXPECT_TRUE(error_tracker.seen_error()) << literal; } llvm::StringLiteral invalid[] = { @@ -320,14 +321,14 @@ TEST_F(NumericLiteralTest, ValidatesRealLiterals) { for (llvm::StringLiteral literal : invalid) { error_tracker.Reset(); EXPECT_THAT(Parse(literal), HasUnrecoverableError()) << literal; - EXPECT_TRUE(error_tracker.SeenError()) << literal; + EXPECT_TRUE(error_tracker.seen_error()) << literal; } } TEST_F(NumericLiteralTest, TooManyDigits) { std::string long_number(2000, '1'); EXPECT_THAT(Parse(long_number), HasUnrecoverableError()); - EXPECT_TRUE(error_tracker.SeenError()); + EXPECT_TRUE(error_tracker.seen_error()); } } // namespace diff --git a/toolchain/lexer/string_literal_test.cpp b/toolchain/lexer/string_literal_test.cpp index e3244252b5a6..b38e7155b27d 100644 --- a/toolchain/lexer/string_literal_test.cpp +++ b/toolchain/lexer/string_literal_test.cpp @@ -202,7 +202,7 @@ TEST_F(StringLiteralTest, StringLiteralContents) { for (auto [test, contents] : testcases) { error_tracker.Reset(); auto value = Parse(test.trim()); - EXPECT_FALSE(error_tracker.SeenError()) << "`" << test << "`"; + EXPECT_FALSE(error_tracker.seen_error()) << "`" << test << "`"; EXPECT_EQ(value, contents); } } @@ -226,7 +226,7 @@ TEST_F(StringLiteralTest, StringLiteralBadIndent) { for (auto [test, contents] : testcases) { error_tracker.Reset(); auto value = Parse(test); - EXPECT_TRUE(error_tracker.SeenError()) << "`" << test << "`"; + EXPECT_TRUE(error_tracker.seen_error()) << "`" << test << "`"; EXPECT_EQ(value, contents); } } @@ -275,26 +275,26 @@ TEST_F(StringLiteralTest, StringLiteralBadEscapeSequence) { for (llvm::StringLiteral test : testcases) { error_tracker.Reset(); auto value = Parse(test); - EXPECT_TRUE(error_tracker.SeenError()) << "`" << test << "`"; + EXPECT_TRUE(error_tracker.seen_error()) << "`" << test << "`"; // TODO: Test value produced by error recovery. } } TEST_F(StringLiteralTest, TabInString) { auto value = Parse("\"x\ty\""); - EXPECT_TRUE(error_tracker.SeenError()); + EXPECT_TRUE(error_tracker.seen_error()); EXPECT_EQ(value, "x\ty"); } TEST_F(StringLiteralTest, TabAtEndOfString) { auto value = Parse("\"\t\t\t\""); - EXPECT_TRUE(error_tracker.SeenError()); + EXPECT_TRUE(error_tracker.seen_error()); EXPECT_EQ(value, "\t\t\t"); } TEST_F(StringLiteralTest, TabInBlockString) { auto value = Parse("\"\"\"\nx\ty\n\"\"\""); - EXPECT_TRUE(error_tracker.SeenError()); + EXPECT_TRUE(error_tracker.seen_error()); EXPECT_EQ(value, "x\ty\n"); } @@ -303,7 +303,7 @@ TEST_F(StringLiteralTest, UnicodeTooManyDigits) { text.append(10000, '9'); text.append("}"); auto value = Parse("\"\\" + text + "\""); - EXPECT_TRUE(error_tracker.SeenError()); + EXPECT_TRUE(error_tracker.seen_error()); EXPECT_EQ(value, text); } diff --git a/toolchain/lexer/tokenized_buffer.cpp b/toolchain/lexer/tokenized_buffer.cpp index 52764419c47d..ddd5c69c5c14 100644 --- a/toolchain/lexer/tokenized_buffer.cpp +++ b/toolchain/lexer/tokenized_buffer.cpp @@ -573,7 +573,7 @@ auto TokenizedBuffer::Lex(SourceBuffer& source, DiagnosticConsumer& consumer) ErrorTrackingDiagnosticConsumer error_tracking_consumer(consumer); Lexer lexer(buffer, error_tracking_consumer); - llvm::StringRef source_text = source.Text(); + llvm::StringRef source_text = source.text(); while (lexer.SkipWhitespace(source_text)) { // Each time we find non-whitespace characters, try each kind of token we // support lexing, from simplest to most complex. @@ -599,7 +599,7 @@ auto TokenizedBuffer::Lex(SourceBuffer& source, DiagnosticConsumer& consumer) lexer.CloseInvalidOpenGroups(TokenKind::Error()); lexer.AddEndOfFileToken(); - if (error_tracking_consumer.SeenError()) { + if (error_tracking_consumer.seen_error()) { buffer.has_errors_ = true; } @@ -632,7 +632,7 @@ auto TokenizedBuffer::GetTokenText(Token token) const -> llvm::StringRef { if (token_info.kind == TokenKind::Error()) { auto& line_info = GetLineInfo(token_info.token_line); int64_t token_start = line_info.start + token_info.column; - return source_->Text().substr(token_start, token_info.error_length); + return source_->text().substr(token_start, token_info.error_length); } // Refer back to the source text to preserve oddities like radix or digit @@ -642,7 +642,7 @@ auto TokenizedBuffer::GetTokenText(Token token) const -> llvm::StringRef { auto& line_info = GetLineInfo(token_info.token_line); int64_t token_start = line_info.start + token_info.column; llvm::Optional relexed_token = - LexedNumericLiteral::Lex(source_->Text().substr(token_start)); + LexedNumericLiteral::Lex(source_->text().substr(token_start)); CHECK(relexed_token) << "Could not reform numeric literal token."; return relexed_token->text(); } @@ -653,7 +653,7 @@ auto TokenizedBuffer::GetTokenText(Token token) const -> llvm::StringRef { auto& line_info = GetLineInfo(token_info.token_line); int64_t token_start = line_info.start + token_info.column; llvm::Optional relexed_token = - LexedStringLiteral::Lex(source_->Text().substr(token_start)); + LexedStringLiteral::Lex(source_->text().substr(token_start)); CHECK(relexed_token) << "Could not reform string literal token."; return relexed_token->text(); } @@ -664,7 +664,7 @@ auto TokenizedBuffer::GetTokenText(Token token) const -> llvm::StringRef { auto& line_info = GetLineInfo(token_info.token_line); int64_t token_start = line_info.start + token_info.column; llvm::StringRef suffix = - source_->Text().substr(token_start + 1).take_while(IsDecimalDigit); + source_->text().substr(token_start + 1).take_while(IsDecimalDigit); return llvm::StringRef(suffix.data() - 1, suffix.size() + 1); } @@ -702,7 +702,7 @@ auto TokenizedBuffer::GetRealLiteral(Token token) const -> RealLiteralValue { // decimal or hexadecimal literal. auto& line_info = GetLineInfo(token_info.token_line); int64_t token_start = line_info.start + token_info.column; - char second_char = source_->Text()[token_start + 1]; + char second_char = source_->text()[token_start + 1]; bool is_decimal = second_char != 'x' && second_char != 'b'; return RealLiteralValue(this, token_info.literal_index, is_decimal); @@ -910,9 +910,9 @@ auto TokenizedBuffer::TokenIterator::Print(llvm::raw_ostream& output) const auto TokenizedBuffer::SourceBufferLocationTranslator::GetLocation( const char* loc) -> Diagnostic::Location { - CHECK(StringRefContainsPointer(buffer_->source_->Text(), loc)) + CHECK(StringRefContainsPointer(buffer_->source_->text(), loc)) << "location not within buffer"; - int64_t offset = loc - buffer_->source_->Text().begin(); + int64_t offset = loc - buffer_->source_->text().begin(); // Find the first line starting after the given location. Note that we can't // inspect `line.length` here because it is not necessarily correct for the @@ -937,7 +937,7 @@ auto TokenizedBuffer::SourceBufferLocationTranslator::GetLocation( column_number = *last_line_lexed_to_column_; for (int64_t i = line_it->start + *last_line_lexed_to_column_; i != offset; ++i) { - if (buffer_->source_->Text()[i] == '\n') { + if (buffer_->source_->text()[i] == '\n') { ++line_number; column_number = 0; } else { @@ -946,7 +946,7 @@ auto TokenizedBuffer::SourceBufferLocationTranslator::GetLocation( } } - return {.file_name = buffer_->source_->Filename().str(), + return {.file_name = buffer_->source_->filename().str(), .line_number = line_number + 1, .column_number = column_number + 1}; } @@ -957,7 +957,7 @@ auto TokenizedBuffer::TokenLocationTranslator::GetLocation(Token token) auto& token_info = buffer_->GetTokenInfo(token); auto& line_info = buffer_->GetLineInfo(token_info.token_line); const char* token_start = - buffer_->source_->Text().begin() + line_info.start + token_info.column; + buffer_->source_->text().begin() + line_info.start + token_info.column; // Find the corresponding file location. // TODO: Should we somehow indicate in the diagnostic location if this token diff --git a/toolchain/parser/parse_tree_test.cpp b/toolchain/parser/parse_tree_test.cpp index 814e37fd3bf2..f13a1d1b1d2c 100644 --- a/toolchain/parser/parse_tree_test.cpp +++ b/toolchain/parser/parse_tree_test.cpp @@ -672,7 +672,7 @@ TEST_F(ParseTreeTest, OperatorWhitespaceErrors) { ErrorTrackingDiagnosticConsumer error_tracker(consumer); ParseTree tree = ParseTree::Parse(tokens, error_tracker); EXPECT_THAT(tree.has_errors(), Eq(kind == Failed)) << input; - EXPECT_THAT(error_tracker.SeenError(), Eq(kind != Valid)) << input; + EXPECT_THAT(error_tracker.seen_error(), Eq(kind != Valid)) << input; } } @@ -719,7 +719,7 @@ TEST_F(ParseTreeTest, IfNoElse) { ErrorTrackingDiagnosticConsumer error_tracker(consumer); ParseTree tree = ParseTree::Parse(tokens, error_tracker); EXPECT_FALSE(tree.has_errors()); - EXPECT_FALSE(error_tracker.SeenError()); + EXPECT_FALSE(error_tracker.seen_error()); EXPECT_THAT( tree, @@ -754,7 +754,7 @@ TEST_F(ParseTreeTest, IfNoElseUnbraced) { ParseTree tree = ParseTree::Parse(tokens, error_tracker); // The missing braces are invalid, but we should be able to recover. EXPECT_FALSE(tree.has_errors()); - EXPECT_TRUE(error_tracker.SeenError()); + EXPECT_TRUE(error_tracker.seen_error()); EXPECT_THAT( tree, @@ -789,7 +789,7 @@ TEST_F(ParseTreeTest, IfElse) { ErrorTrackingDiagnosticConsumer error_tracker(consumer); ParseTree tree = ParseTree::Parse(tokens, error_tracker); EXPECT_FALSE(tree.has_errors()); - EXPECT_FALSE(error_tracker.SeenError()); + EXPECT_FALSE(error_tracker.seen_error()); EXPECT_THAT( tree, @@ -856,7 +856,7 @@ TEST_F(ParseTreeTest, IfElseUnbraced) { ParseTree tree = ParseTree::Parse(tokens, error_tracker); // The missing braces are invalid, but we should be able to recover. EXPECT_FALSE(tree.has_errors()); - EXPECT_TRUE(error_tracker.SeenError()); + EXPECT_TRUE(error_tracker.seen_error()); EXPECT_THAT( tree, @@ -940,7 +940,7 @@ TEST_F(ParseTreeTest, WhileBreakContinue) { ErrorTrackingDiagnosticConsumer error_tracker(consumer); ParseTree tree = ParseTree::Parse(tokens, error_tracker); EXPECT_FALSE(tree.has_errors()); - EXPECT_FALSE(error_tracker.SeenError()); + EXPECT_FALSE(error_tracker.seen_error()); EXPECT_THAT( tree, @@ -971,7 +971,7 @@ TEST_F(ParseTreeTest, WhileUnbraced) { ErrorTrackingDiagnosticConsumer error_tracker(consumer); ParseTree tree = ParseTree::Parse(tokens, error_tracker); EXPECT_FALSE(tree.has_errors()); - EXPECT_TRUE(error_tracker.SeenError()); + EXPECT_TRUE(error_tracker.seen_error()); EXPECT_THAT( tree, diff --git a/toolchain/source/source_buffer.h b/toolchain/source/source_buffer.h index 99c3f4cf945c..417335249cb5 100644 --- a/toolchain/source/source_buffer.h +++ b/toolchain/source/source_buffer.h @@ -50,9 +50,9 @@ class SourceBuffer { ~SourceBuffer(); - [[nodiscard]] auto Filename() const -> llvm::StringRef { return filename_; } + [[nodiscard]] auto filename() const -> llvm::StringRef { return filename_; } - [[nodiscard]] auto Text() const -> llvm::StringRef { return text_; } + [[nodiscard]] auto text() const -> llvm::StringRef { return text_; } private: enum class ContentMode { diff --git a/toolchain/source/source_buffer_test.cpp b/toolchain/source/source_buffer_test.cpp index 7f6e206557f9..c7b305c3ea15 100644 --- a/toolchain/source/source_buffer_test.cpp +++ b/toolchain/source/source_buffer_test.cpp @@ -16,16 +16,16 @@ namespace { TEST(SourceBufferTest, StringRep) { auto buffer = SourceBuffer::CreateFromText(llvm::Twine("Hello") + " World"); - EXPECT_EQ("/text", buffer->Filename()); - EXPECT_EQ("Hello World", buffer->Text()); + EXPECT_EQ("/text", buffer->filename()); + EXPECT_EQ("Hello World", buffer->text()); } TEST(SourceBufferText, StringRepWithFilename) { // Give a custom filename. auto buffer = SourceBuffer::CreateFromText("Hello World Again!", "/custom/text"); - EXPECT_EQ("/custom/text", buffer->Filename()); - EXPECT_EQ("Hello World Again!", buffer->Text()); + EXPECT_EQ("/custom/text", buffer->filename()); + EXPECT_EQ("Hello World Again!", buffer->text()); } auto CreateTestFile(llvm::StringRef text) -> std::string { @@ -54,8 +54,8 @@ TEST(SourceBufferTest, FileRep) { SourceBuffer& buffer = *expected_buffer; - EXPECT_EQ(test_file_path, buffer.Filename()); - EXPECT_EQ("Hello World", buffer.Text()); + EXPECT_EQ(test_file_path, buffer.filename()); + EXPECT_EQ("Hello World", buffer.text()); } TEST(SourceBufferTest, FileRepEmpty) { @@ -67,8 +67,8 @@ TEST(SourceBufferTest, FileRepEmpty) { SourceBuffer& buffer = *expected_buffer; - EXPECT_EQ(test_file_path, buffer.Filename()); - EXPECT_EQ("", buffer.Text()); + EXPECT_EQ(test_file_path, buffer.filename()); + EXPECT_EQ("", buffer.text()); } } // namespace