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: chipsalliance#2323

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

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);
 }

verible/common/formatting/format-token.cc

@@ -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 {
@@ -228,11 +229,12 @@ std::ostream &operator<<(std::ostream &stream, const PreFormatToken &t) {
 }
 
 void ConnectPreFormatTokensPreservedSpaceStarts(
-    const char *buffer_start, std::vector<PreFormatToken> *format_tokens) {
+    absl::string_view::const_iterator buffer_start,
+    std::vector<PreFormatToken> *format_tokens) {
   VLOG(4) << __FUNCTION__;
   CHECK(buffer_start != nullptr);
   for (auto &ftoken : *format_tokens) {
-    ftoken.before.preserved_space_start = buffer_start;
+    ftoken.before.preserved_space_start = &*buffer_start;
     VLOG(4) << "space: " << VisualizeWhitespace(ftoken.OriginalLeadingSpaces());
     buffer_start = ftoken.Text().end();
   }

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<verible::PreFormatToken> *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;
 

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;

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:

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<int, int> SubstringOffsets(absl::string_view substring,

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.

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.

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_.data();
+  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<int> *token_view_indices, const char *offset) {
+    std::vector<int> *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));

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<int> *token_view_indices, const char *offset);
+      std::vector<int> *token_view_indices,
+      absl::string_view::const_iterator offset);
 
   // Resets all fields. Only needed in tests.
   void Clear();

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 {

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

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;
 }
 

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();
 }

verible/common/text/tree-utils.h

@@ -266,16 +266,17 @@ 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.
 // Any subtree node (in a rightmost position) that becomes empty as the result
 // 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.

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;
                                    }

verible/verilog/analysis/symbol-table.h

@@ -394,7 +394,7 @@ struct SymbolInfo {
 
     template <typename L, typename R>
     bool operator()(L l, R r) const {
-      static constexpr std::less<const void *> compare_address;
+      static constexpr std::less<absl::string_view::iterator> compare_address;
       return compare_address(ToString(l).begin(), ToString(r).begin());
     }
   };