From ecc7bdcb87c0912f0591954ccc6e004990e3b4b9 Mon Sep 17 00:00:00 2001 From: Henner Zeller Date: Sun, 12 Jan 2025 11:20:55 -0800 Subject: [PATCH] Don't assume string_view::iterator is const char* There were a few assumptions in the code that these types are equivalent, but they are not on all platforms. The fixes sometimes involve ugly `&*some_iterator` constructs to get to the underlying location. Some of these should be re-considered after switching to c++20. We also have a `string_view_null_iterator()` function to generate an iterator that is not initialized to be used in place where we previously used comparison against nullptr. Issues: #2323 --- MODULE.bazel | 6 ---- bazel/absl.patch | 18 ----------- verible/common/analysis/lint-rule-status.cc | 8 ++--- verible/common/formatting/align.h | 4 +-- verible/common/formatting/align_test.cc | 30 +++++++++++-------- verible/common/formatting/format-token.cc | 22 +++++++------- verible/common/formatting/format-token.h | 15 ++++++++-- .../common/formatting/format-token_test.cc | 7 +++-- verible/common/formatting/layout-optimizer.cc | 2 +- .../formatting/layout-optimizer_test.cc | 11 ++++--- .../formatting/unwrapped-line-test-utils.h | 6 ++-- verible/common/strings/range.cc | 6 ++-- verible/common/strings/range.h | 6 ++-- verible/common/strings/rebase_test.cc | 9 +++--- verible/common/text/text-structure.cc | 28 +++++++++-------- verible/common/text/text-structure.h | 3 +- verible/common/text/token-info.cc | 2 +- verible/common/text/token-info.h | 6 ++-- verible/common/text/token-stream-view.cc | 2 +- verible/common/text/tree-utils.cc | 13 ++++---- verible/common/text/tree-utils.h | 8 +++-- .../analysis/checkers/dff-name-style-rule.cc | 2 +- verible/verilog/analysis/symbol-table.h | 3 +- verible/verilog/formatting/formatter.cc | 3 +- verible/verilog/formatting/formatter_test.cc | 12 ++++---- verible/verilog/formatting/token-annotator.cc | 4 +-- verible/verilog/formatting/token-annotator.h | 3 +- .../formatting/token-annotator_test.cc | 3 +- verible/verilog/formatting/tree-unwrapper.cc | 2 +- verible/verilog/tools/ls/autoexpand.cc | 21 ++++++------- 30 files changed, 140 insertions(+), 125 deletions(-) delete mode 100644 bazel/absl.patch diff --git a/MODULE.bazel b/MODULE.bazel index 2bad50952..87cbfe863 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -20,12 +20,6 @@ bazel_dep(name = "rules_bison", version = "0.3") bazel_dep(name = "googletest", version = "1.14.0.bcr.1", dev_dependency = True) bazel_dep(name = "abseil-cpp", version = "20240116.2") -single_version_override( - module_name = "abseil-cpp", - patch_strip = 1, - patches = ["//bazel:absl.patch"], - version = "20240116.2", -) # Last protobuf version working with windows without strange linking errors. # This also means that we unfortunately can't use the @protobuf//bazel rules diff --git a/bazel/absl.patch b/bazel/absl.patch deleted file mode 100644 index 26d95967a..000000000 --- a/bazel/absl.patch +++ /dev/null @@ -1,18 +0,0 @@ -diff --git a/absl/base/options.h b/absl/base/options.h -index 4b70446b..6a839d94 100644 ---- a/absl/base/options.h -+++ b/absl/base/options.h -@@ -148,7 +148,13 @@ - // absl::string_view is a typedef of std::string_view, use the feature macro - // ABSL_USES_STD_STRING_VIEW. - -+#ifdef _WIN32 -+// On Windows, the std::string_view iterator is not const char*, which -+// is the expectation in a lot of Verible code. Use absl-impl. -+#define ABSL_OPTION_USE_STD_STRING_VIEW 0 -+#else - #define ABSL_OPTION_USE_STD_STRING_VIEW 2 -+#endif - - // ABSL_OPTION_USE_STD_VARIANT - // diff --git a/verible/common/analysis/lint-rule-status.cc b/verible/common/analysis/lint-rule-status.cc index 732bb19b8..6486e0680 100644 --- a/verible/common/analysis/lint-rule-status.cc +++ b/verible/common/analysis/lint-rule-status.cc @@ -43,16 +43,16 @@ std::string AutoFix::Apply(absl::string_view base) const { std::string result; auto prev_start = base.cbegin(); for (const auto &edit : edits_) { - CHECK_LE(base.cbegin(), edit.fragment.cbegin()); - CHECK_GE(base.cend(), edit.fragment.cend()); + CHECK(base.cbegin() <= edit.fragment.cbegin()); + CHECK(base.cend() >= edit.fragment.cend()); const absl::string_view text_before( - prev_start, std::distance(prev_start, edit.fragment.cbegin())); + &*prev_start, std::distance(prev_start, edit.fragment.cbegin())); absl::StrAppend(&result, text_before, edit.replacement); prev_start = edit.fragment.cend(); } - const absl::string_view text_after(prev_start, + const absl::string_view text_after(&*prev_start, std::distance(prev_start, base.cend())); return absl::StrCat(result, text_after); } diff --git a/verible/common/formatting/align.h b/verible/common/formatting/align.h index a746c29f7..af9a53ddf 100644 --- a/verible/common/formatting/align.h +++ b/verible/common/formatting/align.h @@ -396,8 +396,8 @@ ColumnPositionTree ScanPartitionForAlignmentCells_WithNonTreeTokens( // Collect tokens excluded from SyntaxTree (delimiters and comments) CHECK(!ftokens.empty()); - CHECK_LE(ftokens.front().Text().begin(), first_tree_token.text().begin()); - CHECK_GE(ftokens.back().Text().end(), last_tree_token.text().end()); + CHECK(ftokens.front().Text().begin() <= first_tree_token.text().begin()); + CHECK(ftokens.back().Text().end() >= last_tree_token.text().end()); FormatTokenRange::const_iterator ftoken_it = ftokens.begin(); // Find leading non-tree tokens range end diff --git a/verible/common/formatting/align_test.cc b/verible/common/formatting/align_test.cc index 5d9430e6f..d2c19cfc5 100644 --- a/verible/common/formatting/align_test.cc +++ b/verible/common/formatting/align_test.cc @@ -69,7 +69,8 @@ class AlignmentTestFixture : public ::testing::Test, public UnwrappedLineMemoryHandler { public: explicit AlignmentTestFixture(absl::string_view text) - : sample_(text), + : sample_backing_(text), + sample_(sample_backing_), tokens_(absl::StrSplit(sample_, absl::ByAnyChar(" \n"), absl::SkipEmpty())) { for (const auto token : tokens_) { @@ -80,7 +81,8 @@ class AlignmentTestFixture : public ::testing::Test, } protected: - const std::string sample_; + const std::string sample_backing_; + const absl::string_view sample_; const std::vector tokens_; std::vector ftokens_; }; @@ -271,7 +273,7 @@ TEST_F(Sparse3x3MatrixAlignmentTest, AlignmentPolicyFlushLeft) { TEST_F(Sparse3x3MatrixAlignmentTest, AlignmentPolicyPreserve) { // Set previous-token string pointers to preserve spaces. - ConnectPreFormatTokensPreservedSpaceStarts(sample_.data(), + ConnectPreFormatTokensPreservedSpaceStarts(sample_.begin(), &pre_format_tokens_); TabularAlignTokens(40, sample_, ByteOffsetSet(), kPreserveAlignmentHandler, @@ -391,7 +393,7 @@ TEST_F(Sparse3x3MatrixAlignmentTest, IgnoreCommentLine) { TEST_F(Sparse3x3MatrixAlignmentTest, CompletelyDisabledNoAlignment) { // Disabled ranges use original spacing - ConnectPreFormatTokensPreservedSpaceStarts(sample_.data(), + ConnectPreFormatTokensPreservedSpaceStarts(sample_.begin(), &pre_format_tokens_); // Require 1 space between tokens. @@ -413,7 +415,7 @@ TEST_F(Sparse3x3MatrixAlignmentTest, CompletelyDisabledNoAlignment) { TEST_F(Sparse3x3MatrixAlignmentTest, CompletelyDisabledNoAlignmentWithIndent) { // Disabled ranges use original spacing - ConnectPreFormatTokensPreservedSpaceStarts(sample_.data(), + ConnectPreFormatTokensPreservedSpaceStarts(sample_.begin(), &pre_format_tokens_); // Require 1 space between tokens. @@ -445,7 +447,7 @@ class Sparse3x3MatrixAlignmentMoreSpacesTest Sparse3x3MatrixAlignmentMoreSpacesTest() : Sparse3x3MatrixAlignmentTest("one two\nthree four\nfive six") { // This is needed for preservation of original spacing. - ConnectPreFormatTokensPreservedSpaceStarts(sample_.data(), + ConnectPreFormatTokensPreservedSpaceStarts(sample_.begin(), &pre_format_tokens_); } }; @@ -480,7 +482,7 @@ TEST_F(Sparse3x3MatrixAlignmentMoreSpacesTest, TEST_F(Sparse3x3MatrixAlignmentTest, PartiallyDisabledNoAlignment) { // Disabled ranges use original spacing - ConnectPreFormatTokensPreservedSpaceStarts(sample_.data(), + ConnectPreFormatTokensPreservedSpaceStarts(sample_.begin(), &pre_format_tokens_); // Require 1 space between tokens. @@ -865,7 +867,7 @@ class Dense2x2MatrixAlignmentTest : public MatrixTreeAlignmentTestFixture { CHECK_EQ(tokens_.size(), 4); // Need to know original spacing to be able to infer user-intent. - ConnectPreFormatTokensPreservedSpaceStarts(sample_.data(), + ConnectPreFormatTokensPreservedSpaceStarts(sample_.begin(), &pre_format_tokens_); // Require 1 space between tokens. @@ -1170,7 +1172,7 @@ TEST_F(SubcolumnsTreeAlignmentTest, AlignmentPolicyFlushLeft) { } TEST_F(SubcolumnsTreeAlignmentTest, AlignmentPolicyPreserve) { - ConnectPreFormatTokensPreservedSpaceStarts(sample_.data(), + ConnectPreFormatTokensPreservedSpaceStarts(sample_.begin(), &pre_format_tokens_); TabularAlignTokens(40, sample_, ByteOffsetSet(), @@ -1332,7 +1334,7 @@ class InferSubcolumnsTreeAlignmentTest : public SubcolumnsTreeAlignmentTest { "( eleven nineteen-ninety-nine 2k )\n") : SubcolumnsTreeAlignmentTest(text) { // Need to know original spacing to be able to infer user-intent. - ConnectPreFormatTokensPreservedSpaceStarts(sample_.data(), + ConnectPreFormatTokensPreservedSpaceStarts(sample_.begin(), &pre_format_tokens_); // Require 1 space between tokens. @@ -1497,7 +1499,8 @@ class FormatUsingOriginalSpacingTest : public ::testing::Test, "\n <1NL+7Spaces>\n <1nl+7spaces>" "\n\n <2NL+2Spaces>\n\n <2nl+2spaces>" "\n \n\n <1NL+1Space+2NL+2Spaces>\n \n\n <1nl+1space+2nl+2spaces>") - : sample_(text), + : sample_backing_(text), + sample_(sample_backing_), tokens_(absl::StrSplit(sample_, OutsideCharPairs('<', '>'), absl::SkipEmpty())) { for (const auto token : tokens_) { @@ -1505,7 +1508,7 @@ class FormatUsingOriginalSpacingTest : public ::testing::Test, } // sample_ is the memory-owning string buffer CreateTokenInfosExternalStringBuffer(ftokens_); - ConnectPreFormatTokensPreservedSpaceStarts(sample_.data(), + ConnectPreFormatTokensPreservedSpaceStarts(sample_.begin(), &pre_format_tokens_); } @@ -1518,7 +1521,8 @@ class FormatUsingOriginalSpacingTest : public ::testing::Test, EXPECT_PRED_FORMAT2(TokenPartitionTreesEqualPredFormat, nodes[0], expected); } - const std::string sample_; + const std::string sample_backing_; + const absl::string_view sample_; const std::vector tokens_; std::vector ftokens_; }; diff --git a/verible/common/formatting/format-token.cc b/verible/common/formatting/format-token.cc index a175152c8..f5f0f6104 100644 --- a/verible/common/formatting/format-token.cc +++ b/verible/common/formatting/format-token.cc @@ -77,8 +77,8 @@ std::ostream &operator<<(std::ostream &stream, const InterTokenInfo &t) { stream << "{\n spaces_required: " << t.spaces_required << "\n break_penalty: " << t.break_penalty << "\n break_decision: " << t.break_decision - << "\n preserve_space?: " << (t.preserved_space_start != nullptr) - << "\n}"; + << "\n preserve_space?: " + << (t.preserved_space_start != string_view_null_iterator()) << "\n}"; return stream; } @@ -143,9 +143,10 @@ InterTokenDecision::InterTokenDecision(const InterTokenInfo &info) action(ConvertSpacing(info.break_decision)), preserved_space_start(info.preserved_space_start) {} -static absl::string_view OriginalLeadingSpacesRange(const char *begin, - const char *end) { - if (begin == nullptr) { +static absl::string_view OriginalLeadingSpacesRange( + absl::string_view::const_iterator begin, + absl::string_view::const_iterator end) { + if (begin == string_view_null_iterator()) { VLOG(4) << "no original space range"; return make_string_view_range(end, end); // empty range } @@ -163,7 +164,7 @@ absl::string_view FormattedToken::OriginalLeadingSpaces() const { std::ostream &FormattedToken::FormattedText(std::ostream &stream) const { switch (before.action) { case SpacingDecision::kPreserve: { - if (before.preserved_space_start != nullptr) { + if (before.preserved_space_start != string_view_null_iterator()) { // Calculate string_view range of pre-existing spaces, and print that. stream << OriginalLeadingSpaces(); } else { @@ -196,7 +197,7 @@ absl::string_view PreFormatToken::OriginalLeadingSpaces() const { size_t PreFormatToken::LeadingSpacesLength() const { if (before.break_decision == SpacingOptions::kPreserve && - before.preserved_space_start != nullptr) { + before.preserved_space_start != string_view_null_iterator()) { return OriginalLeadingSpaces().length(); } // in other cases (append, wrap), take the spaces_required value. @@ -204,7 +205,7 @@ size_t PreFormatToken::LeadingSpacesLength() const { } int PreFormatToken::ExcessSpaces() const { - if (before.preserved_space_start == nullptr) return 0; + if (before.preserved_space_start == string_view_null_iterator()) return 0; const absl::string_view leading_spaces = OriginalLeadingSpaces(); int delta = 0; if (!absl::StrContains(leading_spaces, "\n")) { @@ -228,9 +229,10 @@ std::ostream &operator<<(std::ostream &stream, const PreFormatToken &t) { } void ConnectPreFormatTokensPreservedSpaceStarts( - const char *buffer_start, std::vector *format_tokens) { + absl::string_view::const_iterator buffer_start, + std::vector *format_tokens) { VLOG(4) << __FUNCTION__; - CHECK(buffer_start != nullptr); + CHECK(buffer_start != string_view_null_iterator()); for (auto &ftoken : *format_tokens) { ftoken.before.preserved_space_start = buffer_start; VLOG(4) << "space: " << VisualizeWhitespace(ftoken.OriginalLeadingSpaces()); diff --git a/verible/common/formatting/format-token.h b/verible/common/formatting/format-token.h index a54231337..8a6071160 100644 --- a/verible/common/formatting/format-token.h +++ b/verible/common/formatting/format-token.h @@ -28,6 +28,13 @@ namespace verible { +// TODO: find something platform independent that is less ugly. +// Or maybe revisit the place where we need this and see if they could +// be actually represented by a const char * nullptr. +inline absl::string_view::const_iterator string_view_null_iterator() { + return absl::string_view::const_iterator{}; +} + // Enumeration for options for formatting spaces between tokens. // This controls what to explore (if not pre-determined). // Related enum: SpacingDecision @@ -76,7 +83,8 @@ struct InterTokenInfo { // tokens, for the sake of preserving space. // Together with the current token, they can form a string_view representing // pre-existing space from the original buffer. - const char *preserved_space_start = nullptr; + absl::string_view::const_iterator preserved_space_start = + string_view_null_iterator(); InterTokenInfo() = default; InterTokenInfo(const InterTokenInfo &) = default; @@ -158,7 +166,7 @@ std::ostream &operator<<(std::ostream &stream, const PreFormatToken &token); // inter-token (space) text. // Note that this does not cover the space between the last token and EOF. void ConnectPreFormatTokensPreservedSpaceStarts( - const char *buffer_start, + absl::string_view::const_iterator buffer_start, std::vector *format_tokens); // Marks formatting-disabled ranges of tokens so that their original spacing is @@ -199,7 +207,8 @@ struct InterTokenDecision { SpacingDecision action = SpacingDecision::kPreserve; // When preserving spaces before this token, start from this offset. - const char *preserved_space_start = nullptr; + absl::string_view::const_iterator preserved_space_start = + string_view_null_iterator(); InterTokenDecision() = default; diff --git a/verible/common/formatting/format-token_test.cc b/verible/common/formatting/format-token_test.cc index dd381dc43..8dffc9dbe 100644 --- a/verible/common/formatting/format-token_test.cc +++ b/verible/common/formatting/format-token_test.cc @@ -218,9 +218,12 @@ class ConnectPreFormatTokensPreservedSpaceStartsTest public UnwrappedLineMemoryHandler {}; TEST_F(ConnectPreFormatTokensPreservedSpaceStartsTest, Empty) { - const char *text = ""; + // We do want to initialize the text, otherwise string_view wraps a nulllptr + // that is checked against downstream. + // NOLINTNEXTLINE(readability-redundant-string-init) + constexpr absl::string_view text(""); CreateTokenInfosExternalStringBuffer({}); - ConnectPreFormatTokensPreservedSpaceStarts(text, &pre_format_tokens_); + ConnectPreFormatTokensPreservedSpaceStarts(text.begin(), &pre_format_tokens_); EXPECT_TRUE(pre_format_tokens_.empty()); } diff --git a/verible/common/formatting/layout-optimizer.cc b/verible/common/formatting/layout-optimizer.cc index d502e4d27..f420831e4 100644 --- a/verible/common/formatting/layout-optimizer.cc +++ b/verible/common/formatting/layout-optimizer.cc @@ -192,7 +192,7 @@ LayoutFunction::const_iterator LayoutFunction::AtOrToTheLeftOf( begin(), end(), column, [](const LayoutFunctionSegment &s, int column) { return s.column <= column; }); - CHECK_NE(it, begin()); + CHECK(it != begin()); return it - 1; } diff --git a/verible/common/formatting/layout-optimizer_test.cc b/verible/common/formatting/layout-optimizer_test.cc index f670287cc..54f87c603 100644 --- a/verible/common/formatting/layout-optimizer_test.cc +++ b/verible/common/formatting/layout-optimizer_test.cc @@ -701,7 +701,8 @@ class LayoutFunctionFactoryTest : public ::testing::Test, // Setup pointers for OriginalLeadingSpaces() auto must_wrap_token = must_wrap_pre_format_tokens.begin(); auto joinable_token = joinable_pre_format_tokens.begin(); - const char *buffer_start = sample_.data(); + absl::string_view sample_view(sample_); + absl::string_view::const_iterator buffer_start = sample_view.begin(); for (size_t i = 0; i < number_of_tokens_in_set; ++i) { must_wrap_token->before.preserved_space_start = buffer_start; joinable_token->before.preserved_space_start = buffer_start; @@ -2387,10 +2388,11 @@ class TokenPartitionsLayoutOptimizerTest : public ::testing::Test, public UnwrappedLineMemoryHandler { public: TokenPartitionsLayoutOptimizerTest() - : sample_( + : sample_backing_( // : |10 : |20 : |30 : |40 "one two three four\n" "eleven twelve thirteen fourteen\n"), + sample_(sample_backing_), tokens_( absl::StrSplit(sample_, absl::ByAnyChar(" \n"), absl::SkipEmpty())), style_(CreateStyle()), @@ -2399,7 +2401,7 @@ class TokenPartitionsLayoutOptimizerTest : public ::testing::Test, ftokens_.emplace_back(1, token); } CreateTokenInfosExternalStringBuffer(ftokens_); - ConnectPreFormatTokensPreservedSpaceStarts(sample_.data(), + ConnectPreFormatTokensPreservedSpaceStarts(sample_.begin(), &pre_format_tokens_); // Set token properties @@ -2423,7 +2425,8 @@ class TokenPartitionsLayoutOptimizerTest : public ::testing::Test, } protected: - const std::string sample_; + const std::string sample_backing_; + const absl::string_view sample_; const std::vector tokens_; std::vector ftokens_; const BasicFormatStyle style_; diff --git a/verible/common/formatting/unwrapped-line-test-utils.h b/verible/common/formatting/unwrapped-line-test-utils.h index 84052a7fd..acbba3f2c 100644 --- a/verible/common/formatting/unwrapped-line-test-utils.h +++ b/verible/common/formatting/unwrapped-line-test-utils.h @@ -56,8 +56,10 @@ class UnwrappedLineMemoryHandler { // Points to the end of joined_token_text_ string buffer. // Same concept as TextStructureView::EOFToken(). TokenInfo EOFToken() const { - const absl::string_view s(joined_token_text_); - return TokenInfo(verible::TK_EOF, absl::string_view(s.end(), 0)); + return TokenInfo( + verible::TK_EOF, + absl::string_view( // NOLINT might be easier with c++20 + joined_token_text_.data() + joined_token_text_.length(), 0)); } protected: diff --git a/verible/common/strings/range.cc b/verible/common/strings/range.cc index 8a3723886..23322fe37 100644 --- a/verible/common/strings/range.cc +++ b/verible/common/strings/range.cc @@ -23,10 +23,12 @@ namespace verible { -absl::string_view make_string_view_range(const char *begin, const char *end) { +absl::string_view make_string_view_range( + absl::string_view::const_iterator begin, + absl::string_view::const_iterator end) { const int length = std::distance(begin, end); CHECK_GE(length, 0) << "Malformed string bounds."; - return absl::string_view(begin, length); + return absl::string_view(&*begin, length); } std::pair SubstringOffsets(absl::string_view substring, diff --git a/verible/common/strings/range.h b/verible/common/strings/range.h index ab608095d..b5dad6194 100644 --- a/verible/common/strings/range.h +++ b/verible/common/strings/range.h @@ -24,8 +24,10 @@ namespace verible { // Construct a string_view from two end-points. // string_view lacks the two-iterator constructor that (iterator) ranges and // containers do. -// This exploits the fact that string_view's iterators are just const char*. -absl::string_view make_string_view_range(const char *begin, const char *end); +// Note, this can go with c++20 built-in string_view constructor. +absl::string_view make_string_view_range( + absl::string_view::const_iterator begin, + absl::string_view::const_iterator end); // Returns [x,y] where superstring.substr(x, y-x) == substring. // Precondition: substring must be a sub-range of superstring. diff --git a/verible/common/strings/rebase_test.cc b/verible/common/strings/rebase_test.cc index dc46da410..c64dc1b4c 100644 --- a/verible/common/strings/rebase_test.cc +++ b/verible/common/strings/rebase_test.cc @@ -83,11 +83,12 @@ TEST(RebaseStringViewTest, NewSubstringNotAtFront) { // Test that substring in the middle of old string is rebased correctly. TEST(RebaseStringViewTest, UsingCharPointer) { const absl::string_view text = "hello"; - const absl::string_view new_base = "xxxhelloyyy"; - const char *new_view = new_base.begin() + 3; + const char *new_base = "xxxhelloyyy"; + const char *new_view_offset = new_base + 3; absl::string_view text_view(text); - RebaseStringView(&text_view, new_view); // assume original length - EXPECT_TRUE(BoundsEqual(text_view, new_base.substr(3, 5))); + RebaseStringView(&text_view, new_view_offset); // assume original length + const absl::string_view new_base_view(new_base); + EXPECT_TRUE(BoundsEqual(text_view, new_base_view.substr(3, 5))); } // Test integration with substr() function rebases correctly. diff --git a/verible/common/text/text-structure.cc b/verible/common/text/text-structure.cc index 367d5934a..a297d5929 100644 --- a/verible/common/text/text-structure.cc +++ b/verible/common/text/text-structure.cc @@ -71,7 +71,8 @@ void TextStructureView::Clear() { contents_ = contents_.substr(0, 0); // clear } -static bool TokenLocationLess(const TokenInfo &token, const char *offset) { +static bool TokenLocationLess(const TokenInfo &token, + const absl::string_view::const_iterator offset) { return token.text().begin() < offset; } @@ -408,8 +409,9 @@ absl::Status TextStructureView::SyntaxTreeConsistencyCheck() const { VLOG(2) << __FUNCTION__; // Check that first and last token in syntax_tree_ point to text // inside contents_. - const char *const lower_bound = contents_.data(); - const char *const upper_bound = lower_bound + contents_.length(); + const absl::string_view::const_iterator lower_bound = contents_.begin(); + const absl::string_view::const_iterator upper_bound = + lower_bound + contents_.length(); if (syntax_tree_ != nullptr) { const SyntaxTreeLeaf *left = GetLeftmostLeaf(*syntax_tree_); if (!left) return absl::OkStatus(); @@ -470,19 +472,21 @@ void TextStructureView::ConsumeDeferredExpansion( TokenSequence::const_iterator *next_token_iter, TokenStreamView::const_iterator *next_token_view_iter, DeferredExpansion *expansion, TokenSequence *combined_tokens, - std::vector *token_view_indices, const char *offset) { + std::vector *token_view_indices, + absl::string_view::const_iterator offset) { auto token_iter = *next_token_iter; auto token_view_iter = *next_token_view_iter; // Find the position up to each expansion point. - *next_token_iter = - std::lower_bound(token_iter, tokens_.cend(), offset, - [](const TokenInfo &token, const char *target) { - return std::distance(target, token.text().begin()) < 0; - }); + *next_token_iter = std::lower_bound( + token_iter, tokens_.cend(), offset, + [](const TokenInfo &token, absl::string_view::const_iterator target) { + return std::distance(target, token.text().begin()) < 0; + }); CHECK(*next_token_iter != tokens_.cend()); *next_token_view_iter = std::lower_bound( token_view_iter, tokens_view_.cend(), offset, - [](TokenStreamView::const_reference token_ref, const char *target) { + [](TokenStreamView::const_reference token_ref, + absl::string_view::const_iterator target) { return std::distance(target, token_ref->text().begin()) < 0; }); CHECK(*next_token_view_iter != tokens_view_.cend()); @@ -498,8 +502,8 @@ void TextStructureView::ConsumeDeferredExpansion( TextStructureView &sub_data = ABSL_DIE_IF_NULL(subanalysis)->MutableData(); const absl::string_view sub_data_text(sub_data.Contents()); CHECK(!IsSubRange(sub_data_text, contents_)); - CHECK_EQ(sub_data_text, absl::string_view(offset, sub_data_text.length())); - CHECK_GE(offset, contents_.begin()); + CHECK_EQ(sub_data_text, absl::string_view(&*offset, sub_data_text.length())); + CHECK(offset >= contents_.begin()); sub_data.RebaseTokensToSuperstring(contents_, sub_data_text, std::distance(contents_.begin(), offset)); diff --git a/verible/common/text/text-structure.h b/verible/common/text/text-structure.h index c901dee9d..1606bd9ec 100644 --- a/verible/common/text/text-structure.h +++ b/verible/common/text/text-structure.h @@ -222,7 +222,8 @@ class TextStructureView { TokenSequence::const_iterator *next_token_iter, TokenStreamView::const_iterator *next_token_view_iter, DeferredExpansion *expansion, TokenSequence *combined_tokens, - std::vector *token_view_indices, const char *offset); + std::vector *token_view_indices, + absl::string_view::const_iterator offset); // Resets all fields. Only needed in tests. void Clear(); diff --git a/verible/common/text/token-info.cc b/verible/common/text/token-info.cc index ca8109330..485fe1076 100644 --- a/verible/common/text/token-info.cc +++ b/verible/common/text/token-info.cc @@ -35,7 +35,7 @@ TokenInfo TokenInfo::EOFToken() { } TokenInfo TokenInfo::EOFToken(absl::string_view buffer) { - return {TK_EOF, absl::string_view(buffer.end(), 0)}; + return {TK_EOF, absl::string_view(buffer.data() + buffer.length(), 0)}; } bool TokenInfo::operator==(const TokenInfo &token) const { diff --git a/verible/common/text/token-info.h b/verible/common/text/token-info.h index 5c112add2..0f23ea531 100644 --- a/verible/common/text/token-info.h +++ b/verible/common/text/token-info.h @@ -95,7 +95,7 @@ class TokenInfo { // a series of abutting substring ranges. Useful for lexer operation. void AdvanceText(int token_length) { // The end of the previous token is the beginning of the next. - text_ = absl::string_view(text_.end(), token_length); + text_ = absl::string_view(text_.data() + text_.length(), token_length); } // Writes a human-readable string representation of the token. @@ -123,8 +123,8 @@ class TokenInfo { // same length as the current string_view. // string_view::iterator happens to be const char*, but don't rely on that // fact as it can be implementation-dependent. - void RebaseStringView(const char *new_text) { - RebaseStringView(absl::string_view(new_text, text_.length())); + void RebaseStringView(absl::string_view::const_iterator new_text) { + RebaseStringView(absl::string_view(&*new_text, text_.length())); } // Joins the text from a sequence of (text-disjoint) tokens, and also diff --git a/verible/common/text/token-stream-view.cc b/verible/common/text/token-stream-view.cc index 65f508492..419d02c38 100644 --- a/verible/common/text/token-stream-view.cc +++ b/verible/common/text/token-stream-view.cc @@ -52,7 +52,7 @@ void FilterTokenStreamViewInPlace(const TokenFilterPredicate &keep, } static bool TokenLocationLess(const TokenSequence::const_iterator &token_iter, - const char *offset) { + absl::string_view::const_iterator offset) { return token_iter->text().begin() < offset; } diff --git a/verible/common/text/tree-utils.cc b/verible/common/text/tree-utils.cc index 5dcdaaf89..b15e59d21 100644 --- a/verible/common/text/tree-utils.cc +++ b/verible/common/text/tree-utils.cc @@ -99,7 +99,7 @@ absl::string_view StringSpanOfSymbol(const Symbol &lsym, const Symbol &rsym) { if (left != nullptr && right != nullptr) { const auto range_begin = left->get().text().begin(); const auto range_end = right->get().text().end(); - return absl::string_view(range_begin, + return absl::string_view(&*range_begin, std::distance(range_begin, range_end)); } return ""; @@ -270,7 +270,8 @@ const Symbol *FindLastSubtree(const Symbol *tree, const TreePredicate &pred) { } ConcreteSyntaxTree *FindSubtreeStartingAtOffset( - ConcreteSyntaxTree *tree, const char *first_token_offset) { + ConcreteSyntaxTree *tree, + absl::string_view::const_iterator first_token_offset) { auto predicate = [=](const Symbol &s) { const SyntaxTreeLeaf *leftmost = GetLeftmostLeaf(s); if (leftmost != nullptr) { @@ -292,7 +293,8 @@ ConcreteSyntaxTree *FindSubtreeStartingAtOffset( // Helper function for PruneSyntaxTreeAfterOffset namespace { // Returns true if this node should be deleted by parent (pop_back). -bool PruneTreeFromRight(ConcreteSyntaxTree *tree, const char *offset) { +bool PruneTreeFromRight(ConcreteSyntaxTree *tree, + absl::string_view::const_iterator offset) { const auto kind = (*ABSL_DIE_IF_NULL(tree))->Kind(); switch (kind) { case SymbolKind::kLeaf: { @@ -329,14 +331,15 @@ bool PruneTreeFromRight(ConcreteSyntaxTree *tree, const char *offset) { } } // namespace -void PruneSyntaxTreeAfterOffset(ConcreteSyntaxTree *tree, const char *offset) { +void PruneSyntaxTreeAfterOffset(ConcreteSyntaxTree *tree, + absl::string_view::const_iterator offset) { PruneTreeFromRight(tree, offset); } // Helper functions for ZoomSyntaxTree namespace { // Return the upper bound offset of the rightmost token in the tree. -const char *RightmostOffset(const Symbol &symbol) { +absl::string_view::const_iterator RightmostOffset(const Symbol &symbol) { const SyntaxTreeLeaf *leaf_ptr = verible::GetRightmostLeaf(symbol); return ABSL_DIE_IF_NULL(leaf_ptr)->get().text().end(); } diff --git a/verible/common/text/tree-utils.h b/verible/common/text/tree-utils.h index 504c60b0a..3fb72812c 100644 --- a/verible/common/text/tree-utils.h +++ b/verible/common/text/tree-utils.h @@ -266,8 +266,9 @@ const Symbol *FindLastSubtree(const Symbol *, const TreePredicate &); // subtree that starts with the next whole token. // Nodes without leaves will never be considered because they have no location. // Both the tree and the returned tree are intended to be mutable. -ConcreteSyntaxTree *FindSubtreeStartingAtOffset(ConcreteSyntaxTree *tree, - const char *first_token_offset); +ConcreteSyntaxTree *FindSubtreeStartingAtOffset( + ConcreteSyntaxTree *tree, + absl::string_view::const_iterator first_token_offset); // Cuts out all nodes and leaves that start at or past the given offset. // This only looks at leaves' location offsets, and not actual text. @@ -275,7 +276,8 @@ ConcreteSyntaxTree *FindSubtreeStartingAtOffset(ConcreteSyntaxTree *tree, // of recursive pruning will also be pruned. // tree must not be null. // This will never prune away the root node. -void PruneSyntaxTreeAfterOffset(ConcreteSyntaxTree *tree, const char *offset); +void PruneSyntaxTreeAfterOffset(ConcreteSyntaxTree *tree, + absl::string_view::const_iterator offset); // Returns the pointer to the largest subtree wholly contained // inside the text range spanned by trim_range. diff --git a/verible/verilog/analysis/checkers/dff-name-style-rule.cc b/verible/verilog/analysis/checkers/dff-name-style-rule.cc index 1211f36c8..7c0401280 100644 --- a/verible/verilog/analysis/checkers/dff-name-style-rule.cc +++ b/verible/verilog/analysis/checkers/dff-name-style-rule.cc @@ -296,7 +296,7 @@ absl::string_view DffNameStyleRule::CheckSuffix( [&](const std::string &suffix) -> bool { if (absl::EndsWith(id, suffix)) { base = absl::string_view( - id.begin(), id.size() - suffix.size()); + id.data(), id.size() - suffix.size()); suffix_match = suffix; return true; } diff --git a/verible/verilog/analysis/symbol-table.h b/verible/verilog/analysis/symbol-table.h index 7173dc6f2..09862b8a0 100644 --- a/verible/verilog/analysis/symbol-table.h +++ b/verible/verilog/analysis/symbol-table.h @@ -394,7 +394,8 @@ struct SymbolInfo { template bool operator()(L l, R r) const { - static constexpr std::less compare_address; + static constexpr std::less + compare_address; return compare_address(ToString(l).begin(), ToString(r).begin()); } }; diff --git a/verible/verilog/formatting/formatter.cc b/verible/verilog/formatting/formatter.cc index ecbe98fa1..ccc1c7a2b 100644 --- a/verible/verilog/formatting/formatter.cc +++ b/verible/verilog/formatting/formatter.cc @@ -742,7 +742,8 @@ class ContinuationCommentAligner { const verible::FormattedToken &token, int *column) { switch (token.before.action) { case verible::SpacingDecision::kPreserve: { - if (token.before.preserved_space_start != nullptr) { + if (token.before.preserved_space_start != + verible::string_view_null_iterator()) { *column += token.OriginalLeadingSpaces().length(); } else { *column += token.before.spaces; diff --git a/verible/verilog/formatting/formatter_test.cc b/verible/verilog/formatting/formatter_test.cc index ffaaab7bc..df4b846ca 100644 --- a/verible/verilog/formatting/formatter_test.cc +++ b/verible/verilog/formatting/formatter_test.cc @@ -18623,7 +18623,7 @@ TEST(FormatterEndToEndTest, FunctionCallsWithComments) { std::string NLCountAndfirstWord(absl::string_view str) { std::string result; int newline_count = 0; - const char *begin = str.begin(); + absl::string_view::const_iterator begin = str.begin(); for (/**/; begin < str.end(); ++begin) { if (!isspace(*begin)) break; newline_count += (*begin == '\n'); @@ -18631,7 +18631,7 @@ std::string NLCountAndfirstWord(absl::string_view str) { // Emit number of newlines seen up to first token. result.append(1, static_cast(newline_count + '0')); // single digit itoa - const char *end_str = begin; + absl::string_view::const_iterator end_str = begin; for (/**/; end_str < str.end() && !isspace(*end_str); ++end_str) { } result.append(begin, end_str); @@ -18643,16 +18643,16 @@ std::string NLCountAndfirstWord(absl::string_view str) { std::string lastWordAndNLCount(absl::string_view str) { std::string result; int newline_count = 0; - const char *back = str.end() - 1; + absl::string_view::const_iterator back = str.end() - 1; for (/**/; back >= str.begin(); --back) { if (!isspace(*back)) break; newline_count += (*back == '\n'); } - const char *start_str = back; + absl::string_view::const_iterator start_str = back; for (/**/; start_str >= str.begin() && !isspace(*start_str); --start_str) { } - result.append(start_str + 1, back - start_str); + result.append(&*(start_str + 1), back - start_str); // Emit number of newlines seen following last token. result.append(1, static_cast(newline_count + '0')); // single digit itoa @@ -18722,7 +18722,7 @@ foobar, input bit [4] foobaz, // Area we cover in the input (include the final newline); const auto source_begin = lines[start_line].begin(); const auto source_end = lines[end_line - 1].end() + 1; // include \n - const absl::string_view range_unformatted(source_begin, + const absl::string_view range_unformatted(&*source_begin, source_end - source_begin); // To compare that we indeed formatted the requested reqgion, we make diff --git a/verible/verilog/formatting/token-annotator.cc b/verible/verilog/formatting/token-annotator.cc index f2a471110..5ad544a39 100644 --- a/verible/verilog/formatting/token-annotator.cc +++ b/verible/verilog/formatting/token-annotator.cc @@ -957,7 +957,7 @@ void AnnotateFormattingInformation( } void AnnotateFormattingInformation( - const FormatStyle &style, const char *buffer_start, + const FormatStyle &style, absl::string_view::const_iterator buffer_start, const verible::Symbol *syntax_tree_root, const verible::TokenInfo &eof_token, std::vector *format_tokens) { @@ -965,7 +965,7 @@ void AnnotateFormattingInformation( return; } - if (buffer_start != nullptr) { + if (buffer_start != verible::string_view_null_iterator()) { // For unit testing, tokens' text snippets don't necessarily originate // from the same contiguous string buffer, so skip this step. ConnectPreFormatTokensPreservedSpaceStarts(buffer_start, format_tokens); diff --git a/verible/verilog/formatting/token-annotator.h b/verible/verilog/formatting/token-annotator.h index 86f976826..abd60d10b 100644 --- a/verible/verilog/formatting/token-annotator.h +++ b/verible/verilog/formatting/token-annotator.h @@ -17,6 +17,7 @@ #include +#include "absl/strings/string_view.h" #include "verible/common/formatting/format-token.h" #include "verible/common/text/symbol.h" #include "verible/common/text/text-structure.h" @@ -42,7 +43,7 @@ void AnnotateFormattingInformation( // syntax_tree_root: syntax tree used for context-sensitive behavior. // eof_token: EOF token pointing to the end of the unformatted string. void AnnotateFormattingInformation( - const FormatStyle &style, const char *buffer_start, + const FormatStyle &style, absl::string_view::const_iterator buffer_start, const verible::Symbol *syntax_tree_root, const verible::TokenInfo &eof_token, std::vector *format_tokens); diff --git a/verible/verilog/formatting/token-annotator_test.cc b/verible/verilog/formatting/token-annotator_test.cc index 943d8e97e..e76da37cd 100644 --- a/verible/verilog/formatting/token-annotator_test.cc +++ b/verible/verilog/formatting/token-annotator_test.cc @@ -1828,7 +1828,8 @@ TEST(TokenAnnotatorTest, AnnotateFormattingInfoTest) { // context-insensitive annotation rules. // Since we're using the joined string buffer inside handler, // we need to pass an EOF token that points to the end of that buffer. - AnnotateFormattingInformation(test_case.style, nullptr, nullptr, + AnnotateFormattingInformation(test_case.style, + verible::string_view_null_iterator(), nullptr, handler.EOFToken(), &ftokens_range); EXPECT_TRUE(CorrectExpectedFormatTokens(test_case.expected_calculations, ftokens_range)) diff --git a/verible/verilog/formatting/tree-unwrapper.cc b/verible/verilog/formatting/tree-unwrapper.cc index 771909416..04e2728ed 100644 --- a/verible/verilog/formatting/tree-unwrapper.cc +++ b/verible/verilog/formatting/tree-unwrapper.cc @@ -3191,7 +3191,7 @@ void TreeUnwrapper::Visit(const verible::SyntaxTreeLeaf &leaf) { UpdateInterLeafScanner(tag); // Sanity check that NextUnfilteredToken() is aligned to the current leaf. - CHECK_EQ(NextUnfilteredToken()->text().begin(), leaf.get().text().begin()); + CHECK(NextUnfilteredToken()->text().begin() == leaf.get().text().begin()); // Start a new partition in the following cases. // In most other cases, do nothing. diff --git a/verible/verilog/tools/ls/autoexpand.cc b/verible/verilog/tools/ls/autoexpand.cc index ec0700621..992975df6 100644 --- a/verible/verilog/tools/ls/autoexpand.cc +++ b/verible/verilog/tools/ls/autoexpand.cc @@ -369,7 +369,7 @@ class AutoExpander { const auto begin = text_structure.Lines()[min].begin(); const auto end = text_structure.Lines()[max].end(); const size_t length = static_cast(std::distance(begin, end)); - expand_span_ = absl::string_view(begin, length); + expand_span_ = absl::string_view(&*begin, length); } AutoExpander(const TextStructureView &text_structure, @@ -797,7 +797,7 @@ void AutoExpander::Module::GenerateConnections( } size_t pos = connected.port_name.find('@'); while (pos != std::string::npos) { - connected.port_name.replace(pos, 1, instance_name.begin(), + connected.port_name.replace(pos, 1, &*instance_name.begin(), instance_name.length()); pos = connected.port_name.find('@', pos); } @@ -846,7 +846,7 @@ void AutoExpander::Module::AddGeneratedConnection( {port_name, {connected}, packed_dimensions, unpacked_dimensions}, direction, Port::Declaration::kUndeclared, - nullptr, + {}, }); } @@ -1000,8 +1000,8 @@ std::vector GetDimensionsFromNodes( const absl::string_view left_span = StringSpanOfSymbol(*left); const absl::string_view right_span = StringSpanOfSymbol(*right); dimensions.push_back(absl::string_view{ - left_span.begin(), static_cast(std::distance( - left_span.begin(), right_span.end()))}); + &*left_span.begin(), static_cast(std::distance( + left_span.begin(), right_span.end()))}); } } } @@ -1121,9 +1121,7 @@ std::optional FindMatchInSymbol(const Symbol &symbol, absl::string_view match; absl::string_view comment; if (RE2::PartialMatch(symbol_span, re, &match, &comment)) { - return AutoExpander::Match{ - .auto_span = {match.begin(), match.length()}, - .comment_span = {comment.begin(), comment.length()}}; + return AutoExpander::Match{.auto_span = match, .comment_span = comment}; } return std::nullopt; } @@ -1133,9 +1131,8 @@ std::optional FindSpanInSymbol(const Symbol &symbol, const RE2 &re) { const absl::string_view symbol_span = StringSpanOfSymbol(symbol); absl::string_view match; - if (RE2::PartialMatch({symbol_span.data(), symbol_span.length()}, re, - &match)) { - return absl::string_view{match.begin(), match.length()}; + if (RE2::PartialMatch(symbol_span, re, &match)) { + return match; } return std::nullopt; } @@ -1574,7 +1571,7 @@ std::optional AutoExpander::FindSpanToReplace( const absl::string_view symbol_span = StringSpanOfSymbol(symbol); const size_t replaced_length = static_cast( std::distance(auto_span.begin(), symbol_span.end() - 1)); - const absl::string_view replaced_span{auto_span.begin(), replaced_length}; + const absl::string_view replaced_span{&*auto_span.begin(), replaced_length}; if (!SpansOverlapping(replaced_span, expand_span_)) { return std::nullopt; }