From ea50d9d03f4f5601674906a34bd37c583aa7ea8e 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. Issues: #2323 --- .github/workflows/verible-ci.yml | 4 +-- MODULE.bazel | 8 +---- bazel/absl.patch | 18 ----------- verible/common/analysis/lint-rule-status.cc | 4 +-- verible/common/formatting/BUILD | 1 + verible/common/formatting/align_test.cc | 30 +++++++++++-------- verible/common/formatting/format-token.cc | 22 +++++++------- verible/common/formatting/format-token.h | 12 ++++++-- .../common/formatting/format-token_test.cc | 4 +-- verible/common/formatting/layout-optimizer.cc | 1 + .../formatting/layout-optimizer_test.cc | 11 ++++--- .../formatting/unwrapped-line-test-utils.h | 6 ++-- verible/common/strings/range.cc | 6 ++-- verible/common/strings/range.h | 7 +++-- verible/common/strings/rebase_test.cc | 9 +++--- verible/common/text/text-structure.cc | 26 +++++++++------- 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 | 12 ++++---- verible/common/text/tree-utils.h | 7 +++-- verible/verilog/analysis/BUILD | 2 ++ .../analysis/checkers/dff-name-style-rule.cc | 2 +- verible/verilog/analysis/symbol-table.h | 2 +- verible/verilog/analysis/verilog-analyzer.cc | 1 + verible/verilog/analysis/verilog-linter.cc | 1 + 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/tools/ls/BUILD | 1 + verible/verilog/tools/ls/autoexpand.cc | 22 +++++++------- .../verilog/tools/ls/symbol-table-handler.cc | 1 + 35 files changed, 137 insertions(+), 121 deletions(-) delete mode 100644 bazel/absl.patch diff --git a/.github/workflows/verible-ci.yml b/.github/workflows/verible-ci.yml index 3ef8d4aa7..852837bd5 100644 --- a/.github/workflows/verible-ci.yml +++ b/.github/workflows/verible-ci.yml @@ -313,7 +313,7 @@ jobs: - name: Install Dependencies run: | - echo "USE_BAZEL_VERSION=6.5.0" >> $GITHUB_ENV + echo "USE_BAZEL_VERSION=7.4.1" >> $GITHUB_ENV - name: Checkout code uses: actions/checkout@v3 @@ -390,7 +390,7 @@ jobs: - name: Install dependencies run: | - choco install bazel --force --version=6.5.0 + choco install bazel --force --version=7.4.1 choco install winflexbison3 choco install llvm --allow-downgrade --version=17.0.6 diff --git a/MODULE.bazel b/MODULE.bazel index 2bad50952..aa88f43d8 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -19,13 +19,7 @@ bazel_dep(name = "rules_bison", version = "0.3") # depends on it :( -- to support all active bazel's, we're stuck till EOL bazel6 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", -) +bazel_dep(name = "abseil-cpp", version = "20240722.0.bcr.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..c41f9bbc9 100644 --- a/verible/common/analysis/lint-rule-status.cc +++ b/verible/common/analysis/lint-rule-status.cc @@ -47,12 +47,12 @@ std::string AutoFix::Apply(absl::string_view base) const { CHECK_GE(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/BUILD b/verible/common/formatting/BUILD index 7ae0df4b0..424a591db 100644 --- a/verible/common/formatting/BUILD +++ b/verible/common/formatting/BUILD @@ -153,6 +153,7 @@ cc_library( "//verible/common/util:vector-tree", "@abseil-cpp//absl/container:fixed_array", "@abseil-cpp//absl/log", + "@abseil-cpp//absl/log:vlog_is_on", "@abseil-cpp//absl/strings", "@abseil-cpp//absl/strings:string_view", ], 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..945804a56 100644 --- a/verible/common/formatting/format-token.h +++ b/verible/common/formatting/format-token.h @@ -28,6 +28,10 @@ namespace verible { +// TODO: find something platform independent that is less ugly. +inline absl::string_view::iterator string_view_null_iterator() { + return absl::string_view().begin(); +} // Enumeration for options for formatting spaces between tokens. // This controls what to explore (if not pre-determined). // Related enum: SpacingDecision @@ -76,7 +80,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::iterator preserved_space_start = + string_view_null_iterator(); InterTokenInfo() = default; InterTokenInfo(const InterTokenInfo &) = default; @@ -158,7 +163,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 +204,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::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..1b6b1a099 100644 --- a/verible/common/formatting/format-token_test.cc +++ b/verible/common/formatting/format-token_test.cc @@ -218,9 +218,9 @@ class ConnectPreFormatTokensPreservedSpaceStartsTest public UnwrappedLineMemoryHandler {}; TEST_F(ConnectPreFormatTokensPreservedSpaceStartsTest, Empty) { - const char *text = ""; + 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..43408db71 100644 --- a/verible/common/formatting/layout-optimizer.cc +++ b/verible/common/formatting/layout-optimizer.cc @@ -30,6 +30,7 @@ #include "absl/container/fixed_array.h" #include "absl/log/log.h" +#include "absl/log/vlog_is_on.h" #include "verible/common/formatting/basic-format-style.h" #include "verible/common/formatting/layout-optimizer-internal.h" #include "verible/common/formatting/token-partition-tree.h" 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..6cfccb7c0 100644 --- a/verible/common/strings/range.h +++ b/verible/common/strings/range.h @@ -24,8 +24,11 @@ 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, as that provides a std::string_view +// constructor for this. +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..e9feb4d6f 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,7 +502,7 @@ 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_EQ(sub_data_text, absl::string_view(&*offset, sub_data_text.length())); CHECK_GE(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..9ed87bdf8 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,7 @@ const Symbol *FindLastSubtree(const Symbol *tree, const TreePredicate &pred) { } ConcreteSyntaxTree *FindSubtreeStartingAtOffset( - ConcreteSyntaxTree *tree, const char *first_token_offset) { + ConcreteSyntaxTree *tree, absl::string_view::iterator first_token_offset) { auto predicate = [=](const Symbol &s) { const SyntaxTreeLeaf *leftmost = GetLeftmostLeaf(s); if (leftmost != nullptr) { @@ -292,7 +292,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::iterator offset) { const auto kind = (*ABSL_DIE_IF_NULL(tree))->Kind(); switch (kind) { case SymbolKind::kLeaf: { @@ -329,14 +330,15 @@ bool PruneTreeFromRight(ConcreteSyntaxTree *tree, const char *offset) { } } // namespace -void PruneSyntaxTreeAfterOffset(ConcreteSyntaxTree *tree, const char *offset) { +void PruneSyntaxTreeAfterOffset(ConcreteSyntaxTree *tree, + absl::string_view::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::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..1bbcfcd7b 100644 --- a/verible/common/text/tree-utils.h +++ b/verible/common/text/tree-utils.h @@ -266,8 +266,8 @@ 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::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 +275,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::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/BUILD b/verible/verilog/analysis/BUILD index 4740fd3ec..9e3a11b4a 100644 --- a/verible/verilog/analysis/BUILD +++ b/verible/verilog/analysis/BUILD @@ -193,6 +193,7 @@ cc_library( "//verible/verilog/parser:verilog-token-enum", "//verible/verilog/preprocessor:verilog-preprocess", "@abseil-cpp//absl/log", + "@abseil-cpp//absl/log:vlog_is_on", "@abseil-cpp//absl/status", "@abseil-cpp//absl/strings", "@abseil-cpp//absl/strings:string_view", @@ -276,6 +277,7 @@ cc_library( "//verible/verilog/parser:verilog-token-enum", "@abseil-cpp//absl/flags:flag", "@abseil-cpp//absl/log", + "@abseil-cpp//absl/log:vlog_is_on", "@abseil-cpp//absl/status", "@abseil-cpp//absl/status:statusor", "@abseil-cpp//absl/strings", 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..372e58f38 100644 --- a/verible/verilog/analysis/symbol-table.h +++ b/verible/verilog/analysis/symbol-table.h @@ -394,7 +394,7 @@ 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/analysis/verilog-analyzer.cc b/verible/verilog/analysis/verilog-analyzer.cc index 95a638746..39b2d5ec1 100644 --- a/verible/verilog/analysis/verilog-analyzer.cc +++ b/verible/verilog/analysis/verilog-analyzer.cc @@ -22,6 +22,7 @@ #include #include "absl/log/log.h" +#include "absl/log/vlog_is_on.h" #include "absl/status/status.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_split.h" diff --git a/verible/verilog/analysis/verilog-linter.cc b/verible/verilog/analysis/verilog-linter.cc index 84d4431e1..7a8af78b1 100644 --- a/verible/verilog/analysis/verilog-linter.cc +++ b/verible/verilog/analysis/verilog-linter.cc @@ -27,6 +27,7 @@ #include "absl/flags/flag.h" #include "absl/log/log.h" +#include "absl/log/vlog_is_on.h" #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/str_cat.h" 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..1fdcc2be5 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::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::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::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::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..2d0c8dd12 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::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..152212408 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::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/tools/ls/BUILD b/verible/verilog/tools/ls/BUILD index e71a2f124..2cc1c1ae2 100644 --- a/verible/verilog/tools/ls/BUILD +++ b/verible/verilog/tools/ls/BUILD @@ -191,6 +191,7 @@ cc_library( "@abseil-cpp//absl/container:flat_hash_map", "@abseil-cpp//absl/flags:flag", "@abseil-cpp//absl/log", + "@abseil-cpp//absl/log:vlog_is_on", "@abseil-cpp//absl/status", "@abseil-cpp//absl/status:statusor", "@abseil-cpp//absl/strings:str_format", diff --git a/verible/verilog/tools/ls/autoexpand.cc b/verible/verilog/tools/ls/autoexpand.cc index ec0700621..3403e5998 100644 --- a/verible/verilog/tools/ls/autoexpand.cc +++ b/verible/verilog/tools/ls/autoexpand.cc @@ -29,6 +29,7 @@ #include #include +#include "verible/common/formatting/format-token.h" #include "absl/container/flat_hash_map.h" #include "absl/container/flat_hash_set.h" #include "absl/container/node_hash_map.h" @@ -369,7 +370,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 +798,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 +847,7 @@ void AutoExpander::Module::AddGeneratedConnection( {port_name, {connected}, packed_dimensions, unpacked_dimensions}, direction, Port::Declaration::kUndeclared, - nullptr, + verible::string_view_null_iterator(), }); } @@ -1000,8 +1001,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 +1122,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 +1132,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 +1572,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; } diff --git a/verible/verilog/tools/ls/symbol-table-handler.cc b/verible/verilog/tools/ls/symbol-table-handler.cc index ef8bee5c7..96e9fda03 100644 --- a/verible/verilog/tools/ls/symbol-table-handler.cc +++ b/verible/verilog/tools/ls/symbol-table-handler.cc @@ -26,6 +26,7 @@ #include "absl/container/flat_hash_map.h" #include "absl/flags/flag.h" #include "absl/log/log.h" +#include "absl/log/vlog_is_on.h" #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/str_format.h"