Abseil now allowed in Chromium code

271 views
Skip to first unread message

Peter Kasting

unread,
Jul 31, 2020, 6:10:47 PM7/31/20
to Chromium-dev, blink-dev, cxx
Chromium contributors (and bcc abseil-io as FYI),

TLDR: It's now possible to use Abseil in first-party Chromium code.
  • Only absl::variant is currently allowed.
  • See http://chromium-cpp.appspot.com/ for the current status of features. To request more features, email c...@chromium.org.
  • To use an allowed feature, have your target depend appropriately on "//third_party/abseil-cpp:absl", and #include the relevant header from third_party/abseil-cpp/absl/.  
Longer version follows

As of today, first-party code in Chromium is allowed to use Abseil. As with new versions of the C++ standard, support is initially restricted to features which are explicitly allowlisted; consideration of a feature is by request. To begin with, absl::variant is the only allowed feature.

To use an Abseil feature in your code:
  • #include the relevant header(s) from third_party/abseil-cpp/absl/.  For example, #include "third_party/abseil-cpp/absl/types/variant.h".
  • In the relevant BUILD.gn, add "//third_party/abseil-cpp:absl" to your target's deps or "public_deps" as appropriate. Because of visibility restrictions designed to hinder inappropriate use, you will need to use public_deps if your #include is in a .h that another target #includes.
To see or request allowed features:
  • To see the current status of features, visit http://chromium-cpp.appspot.com/. Some features are explicitly banned for various reasons; most remain banned-by-default as "TBD".
  • To request consideration of a TBD or banned feature, mail c...@chromium.org, ideally with a use case and relevant context.
  • After two years, style arbiters will explicitly allow or ban all remaining TBD features.
FAQ:
  • What about third-party code?
    • Code in third_party can use Abseil freely, except for features explicitly banned due to incompatibility; code that needs those features is allowed if it won't actually cause problems. Contact cxx@ for more information.
  • What do you mean, incompatibility?  And why are there weird visibility restrictions?
    • Certain Abseil features (e.g. absl::any, ABSL_FLAG) rely on either RTTI (which Chromium disables) or a workaround called FastTypeId that isn't compatible with the component build. Using such features risks subtle bugs when code compiles and links but doesn't behave as expected, so these are banned from being linked into Chromium.
    • Partly to enforce these bans, both the .gn and DEPS files have various restrictions (such as include_rules and visibility adjustments) designed to hinder accidental misuse. It's not impossible, but hopefully we made it hard to accidentally blow your leg off.
  • What about first-party code that needs to interact with third-party code that uses types Chromium doesn't currently allow?
    • Assuming the third-party use is otherwise allowed (see above), you are allowed to use banned features as minimally necessary at the third-party code boundary to interface with it. Convert to Chromium types/features/etc. as early as possible. Contact cxx@ for more information.
  • Will we be replacing base:: types like Span and Optional with absl:: ones?
    • All other things being equal, we would slightly prefer to use an absl:: type to an identical base:: one.  That's one less thing we have to maintain, and we trust the Abseil team.
    • For cases like Span, the base:: type is more std::-compliant than the absl:: one. Since in such cases our ultimate goal would be to move to the std:: type, we won't migrate to absl:: unless compliance improves to at least match base::.
    • For cases like time handling where there are significant design/API differences, we'd need a compelling reason to migrate existing code, since a migration plan would be significantly more complicated. 
    • In the other cases, we won't migrate unless there's a migration plan (and responsible owner(s)). The default plan is "make base:: API match absl::; change base:: implementation to be an alias; replace usage of base:: with absl:: everywhere; remove base:: implementation". Those steps aren't free, and someone needs to be willing to pay the cost.
  • When Chromium supports C++17, will we replace absl:: types with std:: ones?
    • Yes, assuming we can do so without regressing security guarantees. We have various hardening tests that ensure certain misuses cause checkfailures. We won't migrate code to STL implementations that don't provide at least equivalent checks we can enable. The most likely path in many cases is that libc++ adds such checks.  I'm sure pal...@chromium.org would love to talk to you about this. :D
  • Something something Chromium C++14 Abseil support guarantees NaCl toolchain compatibility something uh oh?
    • If you have no idea what this question could be about, you don't need to worry about it.
    • Leadership on various sides have made commitments that should ensure it's safe to start relying on Abseil, and we won't get stuck in a year or two with libraries that stop working, unsupported code, etc.  I don't know what all is public so that's all I'll say.
  • Why can't I use ___________ from Abseil yet?
    • Likely because we haven't formally considered it.  Propose it per instructions above.
    • To make sure we don't unintentionally cause catastrophe, we reserve the right to slow the adoption rate of even clearly-OK features.  But in the limit that should be weeks, not years.
  • Abseil is cool, now can I get support for {boost, EASTL, $your_favorite_here}?
    • We'll consider anything reasonable, but no guarantees.  (Author's note: my recent proposition to add a couple Boost items already on Google's officially approved list was not met with great enthusiasm, if this tells you anything.)
  • pkasting, you're so amazing, I just want to hear more from you. Is there anything else you haven't mentioned?
    • Why yes! Gtest support for Abseil has also been enabled, which provides pretty-printers for various types.  For example, here's the output of an EXPECT_EQ() on two absl::variants, one containing <bool>(false), and one containing <gfx::Rect>({1, 2, 3, 4}):

      Before:
      error: Expected equality of these values:
        V(false)
          Which is: 24-byte object <00-F9 63-74 F7-7F 00-00 00-00 00-00 FA-00 00-00 01-00 00-00 00-00 00-00>
        c
          Which is: 24-byte object <01-00 00-00 02-00 00-00 03-00 00-00 04-00 00-00 02-00 00-00 00-00 00-00>

      After:
      error: Expected equality of these values:
        V(false)
          Which is: ('<type>(index = 1)' with value false)
        c
          Which is: ('<type>(index = 2)' with value 1,2 3x4)

Thanks
  • The Abseil team, who have been very helpful in working with Chromium to support our needs.
  • mbonadei@ and danilchap@, the people mainly responsible for keeping Abseil rolled into Chromium's tree and functioning as desired, including testing and adding various things to resolve blockers.
  • cxx@ et al., especially those who sent explicit input on this, including (but not limited to) danakj@, jbroman@, jdoerrie@, gab@, jam@, dpranke@, dcheng@.
  • If you read this far, you!  Hope it was informative.
PK

P.S. OK, I admit, no one asked that last FAQ.

Paul Wankadia

unread,
Aug 2, 2020, 11:15:13 AM8/2/20
to Peter Kasting, c...@chromium.org
[-*, +cxx]

Yay!

Any feelpinions regarding adoption of RE2 avec Abseil? FWIW, https://github.com/google/re2/blob/abseil/BUILD#L88-L97 is what's currently used. Migrating callers from re2::StringPiece to absl::string_view would probably require the most effort, I think?

Avi Drissman

unread,
Aug 2, 2020, 11:31:45 AM8/2/20
to Paul Wankadia, Peter Kasting, cxx
absl::string_view is banned so that might be an issue.


--
You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CACSOxh4KVimnFfZHGJa-m8rBGR0nyyecOPK7iKQVjgZ%2BG%3DbKUg%40mail.gmail.com.

Peter Kasting

unread,
Aug 2, 2020, 1:57:10 PM8/2/20
to Paul Wankadia, cxx
On Sun, Aug 2, 2020 at 8:15 AM 'Paul Wankadia' via cxx <c...@chromium.org> wrote:
Any feelpinions regarding adoption of RE2 avec Abseil? FWIW, https://github.com/google/re2/blob/abseil/BUILD#L88-L97 is what's currently used. Migrating callers from re2::StringPiece to absl::string_view would probably require the most effort, I think?

Sorry, I'm not familiar enough with re2 to understand the question sufficiently.  Can you give more detail?  It looks like you're proposing changing Chromium to pull from upstream's "abseil" branch rather than "master", but I don't know what that really connotes in terms of features, API, and support/stability.

As Avi notes, string_view is banned, but there are exceptions for interfacing with third-party code; however, to know how best to solve any difficulties that poses, I'd want to know more what's being proposed.

PK

Paul Wankadia

unread,
Aug 5, 2020, 1:42:20 AM8/5/20
to Peter Kasting, cxx
On Mon, Aug 3, 2020 at 3:57 AM Peter Kasting <pkas...@google.com> wrote:

Sorry, I'm not familiar enough with re2 to understand the question sufficiently.  Can you give more detail?  It looks like you're proposing changing Chromium to pull from upstream's "abseil" branch rather than "master", but I don't know what that really connotes in terms of features, API, and support/stability.

The two branches are maintained in lockstep, so the differences are really about the use of Abseil components: absl::string_view, absl::Mutex and SwissTable would be the top three, I think, but absl::call_once() is typically faster than std::call_once() and then absl::FixedArray<> and absl::InlinedVector<> help to avoid heap allocations in some cases. (absl::string_view is important mostly as a "vocabulary" type, but it seems unimportant to Chromium in that regard.)

FWIW, the Abseil branch has been used by Google (i.e. internally) for about two years and by TensorFlow for about one year.

Peter Kasting

unread,
Aug 5, 2020, 2:03:45 AM8/5/20
to Paul Wankadia, cxx
When you say "use", do you mean in implementation or APIs, or both? Implementation uses are no problem; API changes are probably more of one.

PK

Paul Wankadia

unread,
Aug 5, 2020, 2:20:20 AM8/5/20
to Peter Kasting, cxx
On Wed, Aug 5, 2020 at 4:03 PM Peter Kasting <pkas...@google.com> wrote:

When you say "use", do you mean in implementation or APIs, or both? Implementation uses are no problem; API changes are probably more of one.

Mea culpa, I should have been clearer: the only changes to the public interfaces involve absl::string_view; everything else is for implementation purposes only.

Peter Kasting

unread,
Aug 5, 2020, 2:51:55 AM8/5/20
to Paul Wankadia, cxx, Jan Wilken Dörrie
Hmm. It's too bad the implementation and API changes aren't controlled separately.

If string_view is only used in parameter types, perhaps we should add implicit conversion from base::StringPiece somehow.  That would enable callers to call these functions without needing to use string_view or lose functionality.  If it's used in return types, that's more challenging; we'd have to write wrapper APIs, probably, unless we also wanted to give StringPiece a string_view implicit constructor.

I'd like jdoerrie's thoughts on this one.

PK

Jan Wilken Dörrie

unread,
Aug 5, 2020, 4:03:12 AM8/5/20
to Peter Kasting, Paul Wankadia, cxx
Thanks for looping me in! My thoughts are as follows:

In general I am supportive of absl::string_view. It has the same API as std::string_view, and thus is closer to the STL than base::StringPiece where we still have some work to do.

However, the big drawback of Abseil is that there is no absl::basic_string_view template, and thus not suitable for multi-byte characters. Thus while in theory we could alias base::StringPiece to absl::string_view,  we couldn't do the equivalent for base::StringPiece16. Because of this we should not yet adopt absl::string_view in Chromium, since the small difference in APIs between absl::string_view and base::StringPiece16 would be very confusing otherwise. 

That said, I don't think the small differences between base::StringPiece and absl::string_view should block us from adopting 3rd party libraries making use of the latter. The two types are very similar, have the exact same memory layout and converting between them is cheap and easy. Thus I am in favor of (sufficiently constrained) implicit constructors and conversion operators, as this should unblock the adoption of otherwise very useful Abseil libraries. However, we should ensure that we limit their usage as much as possible, and ideally only use them on 3rd party API boundaries. I expressed a similar opinion in the initial allow Abseil thread.

Best,
Jan

Paul Wankadia

unread,
Aug 5, 2020, 6:57:02 AM8/5/20
to Jan Wilken Dörrie, Peter Kasting, cxx
On Wed, Aug 5, 2020 at 6:03 PM Jan Wilken Dörrie <jdoe...@google.com> wrote:

That said, I don't think the small differences between base::StringPiece and absl::string_view should block us from adopting 3rd party libraries making use of the latter. The two types are very similar, have the exact same memory layout and converting between them is cheap and easy. Thus I am in favor of (sufficiently constrained) implicit constructors and conversion operators, as this should unblock the adoption of otherwise very useful Abseil libraries. However, we should ensure that we limit their usage as much as possible, and ideally only use them on 3rd party API boundaries. I expressed a similar opinion in the initial allow Abseil thread.

Thanks, jdoerrie. :)

pkasting, I'm guessing that whatever the Chromium codebase is already doing for re2::StringPiece could also be done for absl::string_view? (It's worth noting that both of them alias std::string_view under C++17 or later, but https://chromium-cpp.appspot.com/ says that's at least a year away.) I'm happy to help with whatever changes might be needed.

Peter Kasting

unread,
Aug 6, 2020, 11:16:25 AM8/6/20
to Jan Wilken Dörrie, Paul Wankadia, cxx
On Wed, Aug 5, 2020 at 1:03 AM Jan Wilken Dörrie <jdoe...@google.com> wrote:
That said, I don't think the small differences between base::StringPiece and absl::string_view should block us from adopting 3rd party libraries making use of the latter. The two types are very similar, have the exact same memory layout and converting between them is cheap and easy. Thus I am in favor of (sufficiently constrained) implicit constructors and conversion operators, as this should unblock the adoption of otherwise very useful Abseil libraries. However, we should ensure that we limit their usage as much as possible, and ideally only use them on 3rd party API boundaries. I expressed a similar opinion in the initial allow Abseil thread.

I'm not sure how we would do the conversion implicitly, yet limit its use to the boundary of third-party code (although if only third-party code can use string_view, then we sort of get that anyway).

It looks like today re2::StringPiece <-> StringPiece happens manually if at all (see https://source.chromium.org/chromium/chromium/src/+/master:extensions/browser/api/declarative_net_request/regex_rules_matcher.cc;l=40;drc=bfc918abc39f0349f4993b2ae47429ce05e7619f for a sorta-similar case).  Perhaps we could start by:
  1. Adding some conversion routines between base::StringPiece and absl::string_view, e.g. as new //base/strings/ APIs (StringPieceToStringView() and StringViewToStringPiece()?).  This lets us allow that specific file to access string_view.
  2. Changing our re2 deps to pull from the abseil branch and fixing the callers that use re2::StringPiece directly to use base::StringPiece by means of the above wrapper APIs (there don't look to be very many)
If this sounds like a good plan I could file it.

I do want to circle back to the original proposal here.  It sounds like the primary goal is to make re2 perform better (faster/less memory).  Is that correct, or is there more to the story?

PK

Jan Wilken Dörrie

unread,
Aug 6, 2020, 12:09:14 PM8/6/20
to Peter Kasting, Paul Wankadia, cxx
On Thu, Aug 6, 2020 at 5:16 PM Peter Kasting <pkas...@google.com> wrote:
On Wed, Aug 5, 2020 at 1:03 AM Jan Wilken Dörrie <jdoe...@google.com> wrote:
That said, I don't think the small differences between base::StringPiece and absl::string_view should block us from adopting 3rd party libraries making use of the latter. The two types are very similar, have the exact same memory layout and converting between them is cheap and easy. Thus I am in favor of (sufficiently constrained) implicit constructors and conversion operators, as this should unblock the adoption of otherwise very useful Abseil libraries. However, we should ensure that we limit their usage as much as possible, and ideally only use them on 3rd party API boundaries. I expressed a similar opinion in the initial allow Abseil thread.

I'm not sure how we would do the conversion implicitly, yet limit its use to the boundary of third-party code (although if only third-party code can use string_view, then we sort of get that anyway).

It looks like today re2::StringPiece <-> StringPiece happens manually if at all (see https://source.chromium.org/chromium/chromium/src/+/master:extensions/browser/api/declarative_net_request/regex_rules_matcher.cc;l=40;drc=bfc918abc39f0349f4993b2ae47429ce05e7619f for a sorta-similar case).  Perhaps we could start by:
  1. Adding some conversion routines between base::StringPiece and absl::string_view, e.g. as new //base/strings/ APIs (StringPieceToStringView() and StringViewToStringPiece()?).  This lets us allow that specific file to access string_view.

I fail to see how this would be different than allowing implicit conversions between absl::string_view and base::StringPiece. Couldn't we only allow //base/strings/string_piece.h to #include "third_party/abseil-cpp/absl/strings/string_view.h", and forbid other references to absl::string_view via a PRESUBMIT check?
 
  1. Changing our re2 deps to pull from the abseil branch and fixing the callers that use re2::StringPiece directly to use base::StringPiece by means of the above wrapper APIs (there don't look to be very many)
Things will be a bit trickier for APIs that don't consume a single string_view, but rather an array of them, e.g. RE2::Match. Here you'd either have to create a temporary array and copy them over one-by-one, or resort to reinterpret_cast, which likely borders on UB. Implicit conversions won't solve this either (can't convert between Container<T> and Container<U>, even if T and U are convertible),  so we'd have to extend the range of utility functions here.

Peter Kasting

unread,
Aug 6, 2020, 2:48:14 PM8/6/20
to Jan Wilken Dörrie, Paul Wankadia, cxx
On Thu, Aug 6, 2020 at 9:09 AM 'Jan Wilken Dörrie' via cxx <c...@chromium.org> wrote:
On Thu, Aug 6, 2020 at 5:16 PM Peter Kasting <pkas...@google.com> wrote:
On Wed, Aug 5, 2020 at 1:03 AM Jan Wilken Dörrie <jdoe...@google.com> wrote:
That said, I don't think the small differences between base::StringPiece and absl::string_view should block us from adopting 3rd party libraries making use of the latter. The two types are very similar, have the exact same memory layout and converting between them is cheap and easy. Thus I am in favor of (sufficiently constrained) implicit constructors and conversion operators, as this should unblock the adoption of otherwise very useful Abseil libraries. However, we should ensure that we limit their usage as much as possible, and ideally only use them on 3rd party API boundaries. I expressed a similar opinion in the initial allow Abseil thread.

I'm not sure how we would do the conversion implicitly, yet limit its use to the boundary of third-party code (although if only third-party code can use string_view, then we sort of get that anyway).

It looks like today re2::StringPiece <-> StringPiece happens manually if at all (see https://source.chromium.org/chromium/chromium/src/+/master:extensions/browser/api/declarative_net_request/regex_rules_matcher.cc;l=40;drc=bfc918abc39f0349f4993b2ae47429ce05e7619f for a sorta-similar case).  Perhaps we could start by:
  1. Adding some conversion routines between base::StringPiece and absl::string_view, e.g. as new //base/strings/ APIs (StringPieceToStringView() and StringViewToStringPiece()?).  This lets us allow that specific file to access string_view.

I fail to see how this would be different than allowing implicit conversions between absl::string_view and base::StringPiece. Couldn't we only allow //base/strings/string_piece.h to #include "third_party/abseil-cpp/absl/strings/string_view.h", and forbid other references to absl::string_view via a PRESUBMIT check?

My concern is twofold:

(1) Since #includes are transitive, if string_piece.h #includes string_view.h, now everyone who #includes string_piece.h -- including indirectly -- has string_view in scope.  We can probably PRESUBMIT-check against these, but I'd rather the compile fail.  If instead some_string_piece_conversion_file.h #includes string_view.h, the only places bringing that in scope will be individual .cc files that want to call the conversions.  It's not viral.
(2) As an author or reviewer, an explicit type conversion is obvious; an implicit one is not.  It's much easier to accidentally write or commit code using a banned type with implicit conversions.

I don't think the difference between the proposals is enormous, but I think there's a small win here.
  1. Changing our re2 deps to pull from the abseil branch and fixing the callers that use re2::StringPiece directly to use base::StringPiece by means of the above wrapper APIs (there don't look to be very many)
Things will be a bit trickier for APIs that don't consume a single string_view, but rather an array of them, e.g. RE2::Match. Here you'd either have to create a temporary array and copy them over one-by-one, or resort to reinterpret_cast, which likely borders on UB. Implicit conversions won't solve this either (can't convert between Container<T> and Container<U>, even if T and U are convertible),  so we'd have to extend the range of utility functions here.

Correct, and this applies no matter what we decide on the previous question.  OTOH, this is already an issue today since we don't use re2::StringPiece except for boundary code here, so in my view "this is no worse".

PK 

Jan Wilken Dörrie

unread,
Aug 6, 2020, 4:08:24 PM8/6/20
to Peter Kasting, Paul Wankadia, cxx
On Thu, Aug 6, 2020 at 8:48 PM Peter Kasting <pkas...@google.com> wrote:
On Thu, Aug 6, 2020 at 9:09 AM 'Jan Wilken Dörrie' via cxx <c...@chromium.org> wrote:
On Thu, Aug 6, 2020 at 5:16 PM Peter Kasting <pkas...@google.com> wrote:
On Wed, Aug 5, 2020 at 1:03 AM Jan Wilken Dörrie <jdoe...@google.com> wrote:
That said, I don't think the small differences between base::StringPiece and absl::string_view should block us from adopting 3rd party libraries making use of the latter. The two types are very similar, have the exact same memory layout and converting between them is cheap and easy. Thus I am in favor of (sufficiently constrained) implicit constructors and conversion operators, as this should unblock the adoption of otherwise very useful Abseil libraries. However, we should ensure that we limit their usage as much as possible, and ideally only use them on 3rd party API boundaries. I expressed a similar opinion in the initial allow Abseil thread.

I'm not sure how we would do the conversion implicitly, yet limit its use to the boundary of third-party code (although if only third-party code can use string_view, then we sort of get that anyway).

It looks like today re2::StringPiece <-> StringPiece happens manually if at all (see https://source.chromium.org/chromium/chromium/src/+/master:extensions/browser/api/declarative_net_request/regex_rules_matcher.cc;l=40;drc=bfc918abc39f0349f4993b2ae47429ce05e7619f for a sorta-similar case).  Perhaps we could start by:
  1. Adding some conversion routines between base::StringPiece and absl::string_view, e.g. as new //base/strings/ APIs (StringPieceToStringView() and StringViewToStringPiece()?).  This lets us allow that specific file to access string_view.

I fail to see how this would be different than allowing implicit conversions between absl::string_view and base::StringPiece. Couldn't we only allow //base/strings/string_piece.h to #include "third_party/abseil-cpp/absl/strings/string_view.h", and forbid other references to absl::string_view via a PRESUBMIT check?

My concern is twofold:

(1) Since #includes are transitive, if string_piece.h #includes string_view.h, now everyone who #includes string_piece.h -- including indirectly -- has string_view in scope.  We can probably PRESUBMIT-check against these, but I'd rather the compile fail.  If instead some_string_piece_conversion_file.h #includes string_view.h, the only places bringing that in scope will be individual .cc files that want to call the conversions.  It's not viral.
(2) As an author or reviewer, an explicit type conversion is obvious; an implicit one is not.  It's much easier to accidentally write or commit code using a banned type with implicit conversions.

I don't think the difference between the proposals is enormous, but I think there's a small win here.

These are both good arguments, point taken.
 
  1. Changing our re2 deps to pull from the abseil branch and fixing the callers that use re2::StringPiece directly to use base::StringPiece by means of the above wrapper APIs (there don't look to be very many)
Things will be a bit trickier for APIs that don't consume a single string_view, but rather an array of them, e.g. RE2::Match. Here you'd either have to create a temporary array and copy them over one-by-one, or resort to reinterpret_cast, which likely borders on UB. Implicit conversions won't solve this either (can't convert between Container<T> and Container<U>, even if T and U are convertible),  so we'd have to extend the range of utility functions here.

Correct, and this applies no matter what we decide on the previous question.  OTOH, this is already an issue today since we don't use re2::StringPiece except for boundary code here, so in my view "this is no worse".

Agreed. I think we should follow your suggestion of adding a dedicated header to //base/strings (or potentially //base/util/... like previously mentioned), containing the following conversions:

* absl::string_view to base::StringPiece
* base::StringPiece to absl::string_view
* base::span<const absl::string_view> to std::vector<base::StringPiece>
* base::span<const base::StringPiece> to std::vector<absl::string_view>

The last two are intended to cover the array and vector cases, which should be the most popular outside of the singular conversions. This then should be sufficient to unblock the adoption of re2/abseil and other Abseil features that rely on absl::string_view in their public API.

PK 

Peter Kasting

unread,
Aug 6, 2020, 4:21:55 PM8/6/20
to Jan Wilken Dörrie, Paul Wankadia, cxx
On Thu, Aug 6, 2020 at 1:08 PM Jan Wilken Dörrie <jdoe...@google.com> wrote:
I think we should follow your suggestion of adding a dedicated header to //base/strings (or potentially //base/util/... like previously mentioned), containing the following conversions:

* absl::string_view to base::StringPiece
* base::StringPiece to absl::string_view
* base::span<const absl::string_view> to std::vector<base::StringPiece>
* base::span<const base::StringPiece> to std::vector<absl::string_view>

The last two are intended to cover the array and vector cases, which should be the most popular outside of the singular conversions. This then should be sufficient to unblock the adoption of re2/abseil and other Abseil features that rely on absl::string_view in their public API.

Filed https://bugs.chromium.org/p/chromium/issues/detail?id=1113880 , since I think we want this in general regardless of this specific library.

Let's assume we have that.  Then it comes back to the original question here.  Do we want to pull the abseil re2 instead of the main one?  Is this even a cxx@ question at this point, since it's not really about allowing something new?

PK

Jan Wilken Dörrie

unread,
Aug 7, 2020, 8:54:55 AM8/7/20
to Peter Kasting, Paul Wankadia, cxx
On Thu, Aug 6, 2020 at 10:21 PM Peter Kasting <pkas...@google.com> wrote:
On Thu, Aug 6, 2020 at 1:08 PM Jan Wilken Dörrie <jdoe...@google.com> wrote:
I think we should follow your suggestion of adding a dedicated header to //base/strings (or potentially //base/util/... like previously mentioned), containing the following conversions:

* absl::string_view to base::StringPiece
* base::StringPiece to absl::string_view
* base::span<const absl::string_view> to std::vector<base::StringPiece>
* base::span<const base::StringPiece> to std::vector<absl::string_view>

The last two are intended to cover the array and vector cases, which should be the most popular outside of the singular conversions. This then should be sufficient to unblock the adoption of re2/abseil and other Abseil features that rely on absl::string_view in their public API.

Filed https://bugs.chromium.org/p/chromium/issues/detail?id=1113880 , since I think we want this in general regardless of this specific library.

Agreed, thanks!
 

Let's assume we have that.  Then it comes back to the original question here.  Do we want to pull the abseil re2 instead of the main one?  Is this even a cxx@ question at this point, since it's not really about allowing something new?

IMO it isn't anymore. Assuming Abseil support as given, deciding whether we use https://github.com/google/re2/tree/abseil instead of https://github.com/google/re2 should probably be done by third_party/re2/OWNERS.
 

PK

Peter Kasting

unread,
Aug 7, 2020, 11:15:37 AM8/7/20
to Jan Wilken Dörrie, Paul Wankadia, cxx
OK.  FIled https://bugs.chromium.org/p/chromium/issues/detail?id=1114173 to track this question, then.  Interested parties can star/comment there.

Thanks for the suggestion, Paul!

PK 

Paul Wankadia

unread,
Aug 10, 2020, 6:36:13 AM8/10/20
to Peter Kasting, Jan Wilken Dörrie, cxx
On Fri, Aug 7, 2020 at 1:16 AM Peter Kasting <pkas...@google.com> wrote:

I do want to circle back to the original proposal here.  It sounds like the primary goal is to make re2 perform better (faster/less memory).  Is that correct, or is there more to the story?

Correct, the only goal in this case is to improve performance by using Abseil widgets. (In previous cases, the other goal was to eliminate the friction caused by converting to/from the RE2-specific StringPiece.)

Paul Wankadia

unread,
Aug 10, 2020, 6:38:55 AM8/10/20
to Peter Kasting, Jan Wilken Dörrie, cxx
On Sat, Aug 8, 2020 at 1:15 AM Peter Kasting <pkas...@google.com> wrote:

OK.  FIled https://bugs.chromium.org/p/chromium/issues/detail?id=1114173 to track this question, then.  Interested parties can star/comment there.

Thanks for the suggestion, Paul!

Thank you very much for your help in paving the way!

Reply all
Reply to author
Forward
0 new messages