RFC: replacing base::Contains()

[Victor Vianna]

Hi cxx,

C++23 brings std::ranges::contains() and std::basic_string::contains(). Together with previous C++20 additions like std::map::contains() and friends, that covers the vast majority of uses of base::Contains(). So we could now potentially remove the //base function. I wrote Gemini wrote a clang refactoring tool that can do the job and allowed me to gather some statistics here. I ran the tool on 90% of the base::Contains() usages and observed that:

1. 66% can be replaced by a contains() method call
2. 32% can be replaced by std::ranges::contains()
3. 2% (122 uses) can be replaced by find() calls. These could also be contains() if we introduced that method in the corresponding containers.

So I'd like to propose 2 things:

a) That we allow use of std::ranges::contains() when C++23 is allowed (which is pending a styleguide CL and an email at this point)

b) That we migrate base::Contains() using the tool above.

-- Victor

[Adam Rice]

I'm in favour of changing to the standard methods and functions, but 122 uses are enough to justify the existence of base::Contains() by themselves. I wouldn't want to change them to use find() as it is a loss in readability*. Ideally we would add contains() methods to all the containers involved, but I don't know how realistic that is.

Footnote:

There are some cases where find() is better. I've seen code like

if (base::Contains(map, key)) {

  auto it = map.find(key);

  ...

}

which is better written as

if (auto it = map.find(key); it != map.end()) {

  ...

}

I don't know if it's possible for a clang refactoring tool to fix things like that.

--
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 visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CANFyMwyFRjZ9_qzh1%2BUs2ZCYDqfGzuT2%3DXPKJ%3DW55HWQH3bs6g%40mail.gmail.com.

[Chris Fredrickson]

+1, for containers that don't have a `contains()` method, `base::Contains` is still better than `find()` IMO. (It's more readable and conveys more intent: "I don't need an iterator, don't give me one".)

One option could be to migrate the callsites that can use the standard `contains()` method or `std::ranges::contains()`, and then add static_asserts within `base::Contains` to prevent backsliding?

[Kyle Charbonneau]

a) That we allow use of std::ranges::contains() when C++23 is allowed (which is pending a styleguide CL and an email at this point)

That sgtm.

 

2% (122 uses) can be replaced by find() calls. These could also be contains() if we introduced that method in the corresponding containers.

Do you have a list of which container types are the problem here?

To view this discussion visit https://groups.google.com/a/chromium.org/d/msgid/cxx/1f55a5f7-d50a-4a04-af24-bb62e915ed93n%40chromium.org.

[Victor Vianna]

Thanks all. I'm hoping there aren't that many containers with find() but not contains() (I remember a handful in blink and one in extensions). So I'll just try to fix them all. Then we can get rid of base::Contains() without regressing readability.

[Joe Mason]

As an aside - can you share your process for getting Gemini to write that tool?

I think we could be making a lot better use of clang refactoring, if the barrier to get it set up were lower, so I'm interested to know how easy / repeatable it is to get Gemini to do it.

--

[Victor Vianna]

I forked an existing tool, wrote tests that failed initially [1][2], then asked Gemini to make the tool pass the tests. Later, I ran the tool, found cases I'd missed, wrote new tests and iterated with gemini.

IIRC, at some point I told gemini to just read everything in tools/clang/ to get some inspiration.

[1] https://chromium.googlesource.com/chromium/src/+/main/docs/clang_tool_refactoring.md#testing

[2] https://crrev.com/c/7280266

The main obstacle I ran into is that a lot of the existing tools didn't build successfully or didn't pass their tests. I have fixes for most of them (such as crrev.com/c/7268813), but didn't send all of them for review yet.

> I think we could be making a lot better use of clang refactoring, if the barrier to get it set up were lower, so I'm interested to know how easy / repeatable it is to get Gemini to do it.

Big +1. I had tried to write refactoring tools in the past and failed, so this is definitely an AI success story for me

[Daniel Cheng]

On Mon, 5 Jan 2026 at 14:02, 'Victor Vianna' via cxx <c...@chromium.org> wrote:

I forked an existing tool, wrote tests that failed initially [1][2], then asked Gemini to make the tool pass the tests. Later, I ran the tool, found cases I'd missed, wrote new tests and iterated with gemini.

IIRC, at some point I told gemini to just read everything in tools/clang/ to get some inspiration.

[1] https://chromium.googlesource.com/chromium/src/+/main/docs/clang_tool_refactoring.md#testing

[2] https://crrev.com/c/7280266

The main obstacle I ran into is that a lot of the existing tools didn't build successfully or didn't pass their tests. I have fixes for most of them (such as crrev.com/c/7268813), but didn't send all of them for review yet.

> I think we could be making a lot better use of clang refactoring, if the barrier to get it set up were lower, so I'm interested to know how easy / repeatable it is to get Gemini to do it.

Big +1. I had tried to write refactoring tools in the past and failed, so this is definitely an AI success story for me

One significant limitation of clang tooling is that it won't process code that's not built on the platform you're targeting, e.g. if you have a DCHECK-enabled Linux build, a clang tool cannot rewrite Mac-specific instances or non-DCHECK-only code.

You can address this by running the tool multiple times, but this runs into another issue: clang tooling does not (currently) benefit from remoteexec, so running clang tooling is significantly slower than building Chrome. This was a big issue years ago when we did the "rename all the Blink identifiers to match Google style" rewrite.

These days, my workaround is to build my tooling directly into Chrome's clang plugin, e.g. something like https://chromium-review.googlesource.com/c/chromium/src/+/5784484

Daniel

On Monday, January 5, 2026 at 6:50:41 PM UTC-3 Joe Mason wrote:

As an aside - can you share your process for getting Gemini to write that tool?

I think we could be making a lot better use of clang refactoring, if the barrier to get it set up were lower, so I'm interested to know how easy / repeatable it is to get Gemini to do it.

On Tue, Dec 23, 2025 at 2:41 PM 'Victor Vianna' via cxx <c...@chromium.org> wrote:

Hi cxx,

C++23 brings std::ranges::contains() and std::basic_string::contains(). Together with previous C++20 additions like std::map::contains() and friends, that covers the vast majority of uses of base::Contains(). So we could now potentially remove the //base function. I wrote Gemini wrote a clang refactoring tool that can do the job and allowed me to gather some statistics here. I ran the tool on 90% of the base::Contains() usages and observed that:

1. 66% can be replaced by a contains() method call
2. 32% can be replaced by std::ranges::contains()
3. 2% (122 uses) can be replaced by find() calls. These could also be contains() if we introduced that method in the corresponding containers.

So I'd like to propose 2 things:

a) That we allow use of std::ranges::contains() when C++23 is allowed (which is pending a styleguide CL and an email at this point)

b) That we migrate base::Contains() using the tool above.

-- Victor

--

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 visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CANFyMwyFRjZ9_qzh1%2BUs2ZCYDqfGzuT2%3DXPKJ%3DW55HWQH3bs6g%40mail.gmail.com.

--
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 visit https://groups.google.com/a/chromium.org/d/msgid/cxx/5f907f46-b6da-4b3f-b398-c8955e9ee7bcn%40chromium.org.

[Jan Wilken Dörrie]

On Saturday, December 27, 2025 at 3:26:20 AM UTC+1 Adam Rice wrote:

I'm in favour of changing to the standard methods and functions, but 122 uses are enough to justify the existence of base::Contains() by themselves. I wouldn't want to change them to use find() as it is a loss in readability*. Ideally we would add contains() methods to all the containers involved, but I don't know how realistic that is.

Footnote:

There are some cases where find() is better. I've seen code like

if (base::Contains(map, key)) {

  auto it = map.find(key);

  ...

}

which is better written as

if (auto it = map.find(key); it != map.end()) {

  ...

}

I don't know if it's possible for a clang refactoring tool to fix things like that.

Note that there is also `base::FindOrNull`, providing the best of both worlds; boolean testability and a direct handle to the mapped typed with a single look-up:

if (auto* val = base::FindOrNull(map, key)) {

  // Do something with *val, rather than it->second.
  ...

}

[Marshall Greenblatt]

On Mon, Jan 5, 2026 at 5:02 PM 'Victor Vianna' via cxx <c...@chromium.org> wrote:

I forked an existing tool, wrote tests that failed initially [1][2], then asked Gemini to make the tool pass the tests. Later, I ran the tool, found cases I'd missed, wrote new tests and iterated with gemini.

IIRC, at some point I told gemini to just read everything in tools/clang/ to get some inspiration.

[1] https://chromium.googlesource.com/chromium/src/+/main/docs/clang_tool_refactoring.md#testing

[2] https://crrev.com/c/7280266

The main obstacle I ran into is that a lot of the existing tools didn't build successfully or didn't pass their tests. I have fixes for most of them (such as crrev.com/c/7268813), but didn't send all of them for review yet.

Thanks for your work on this! We were able to successfully build a clang tool (at M145) for updating our Chromium-based project (slightly different target C++ version/compiler requirements). This was also built using AI and may provide useful structure for anyone wishing to format code that isn't directly committed to chromium/src (our code lives in a separate repo placed at chromium/src/cef).

https://github.com/chromiumembedded/cef/tree/master/tools/clang

 

> I think we could be making a lot better use of clang refactoring, if the barrier to get it set up were lower, so I'm interested to know how easy / repeatable it is to get Gemini to do it.

Big +1. I had tried to write refactoring tools in the past and failed, so this is definitely an AI success story for me

On Monday, January 5, 2026 at 6:50:41 PM UTC-3 Joe Mason wrote:

As an aside - can you share your process for getting Gemini to write that tool?

I think we could be making a lot better use of clang refactoring, if the barrier to get it set up were lower, so I'm interested to know how easy / repeatable it is to get Gemini to do it.

On Tue, Dec 23, 2025 at 2:41 PM 'Victor Vianna' via cxx <c...@chromium.org> wrote:

Hi cxx,

C++23 brings std::ranges::contains() and std::basic_string::contains(). Together with previous C++20 additions like std::map::contains() and friends, that covers the vast majority of uses of base::Contains(). So we could now potentially remove the //base function. I wrote Gemini wrote a clang refactoring tool that can do the job and allowed me to gather some statistics here. I ran the tool on 90% of the base::Contains() usages and observed that:

1. 66% can be replaced by a contains() method call
2. 32% can be replaced by std::ranges::contains()
3. 2% (122 uses) can be replaced by find() calls. These could also be contains() if we introduced that method in the corresponding containers.

So I'd like to propose 2 things:

a) That we allow use of std::ranges::contains() when C++23 is allowed (which is pending a styleguide CL and an email at this point)

b) That we migrate base::Contains() using the tool above.

-- Victor

--

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 visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CANFyMwyFRjZ9_qzh1%2BUs2ZCYDqfGzuT2%3DXPKJ%3DW55HWQH3bs6g%40mail.gmail.com.

--
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 visit https://groups.google.com/a/chromium.org/d/msgid/cxx/5f907f46-b6da-4b3f-b398-c8955e9ee7bcn%40chromium.org.