feat(prelude): implement String equality comparison operators by bimokh · Pull Request #7610 · carbon-language/carbon-lang

bimokh · 2026-08-05T00:36:14Z

Implement EqWith(String) for Core.String in core/prelude/types/string.carbon to support equality (==) and inequality (!=) comparison operators. The equality comparison short-circuits on size inequality and compares character elements sequentially in an index loop.

Add testing/core/string_eq_test.carbon to verify equality and inequality operators across identical, distinct, length-mismatched, and empty strings with structured test reporting.

Fixes #7610

To satisfy the reviewed for-loop form, prelude/iterate now imports the specific type libraries it uses (types/int, types/int_literal, types/optional) instead of the whole prelude/types umbrella. The umbrella re-exports prelude/types/string, so String's new imports of prelude/range and prelude/iterate would otherwise close a library import cycle (ImportCycleDetected). Consumer visibility is unchanged, and the golden files pinning iterate.carbon line numbers shift by +2. The equality loop is now for (i: i64 in IntRange(64).Make(0, self.Size())), using IntRange(64) because IntRange takes a bit-width IntLiteral parameter.

cjdb

Thanks for implementing this. I have a few comments, but I'm confident we'll get it through in the next few days.

Additionally, would you mind confirming whether this was hand-written, or if an agent was involved in the development process, please?

cjdb · 2026-08-06T16:31:10Z

+    if (self.Size() != other.Size()) {
+      return false;
+    }


We can also short-circuit on self.ptr == other.ptr.

I investigated adding if (self.ptr == other.ptr) for the fast-path, but in Carbon currently, pointer types (T* / Char*) do not implement EqWith, and there are no pointer comparison built-ins (pointer.eq / pointer.neq) in toolchain/sem_ir/builtin_function_kind.def. Attempting self.ptr == other.ptr results in:

error: cannot access member of interface `EqWith(char*)` in type `char*` that does not implement that interface [MissingImplInMemberAccess]

Would you prefer to keep this PR scoped strictly to the prelude String equality loop, or would you like a separate precursor PR adding pointer.eq built-ins and impl forall [T: type] T* as EqWith(Self) first?

That's fine, let's leave it as-is for now.

cjdb · 2026-08-06T17:26:30Z

+    var i: i64 = 0;
+    while (i as u64 < self.Size()) {


Suggested change

var i: i64 = 0;

while (i as u64 < self.Size()) {

let indices: IntRange(u64) = IntRange(i64).Make(0, self.Size());

for (i: i64 in indices) {

This depends on #7614, but I think this will be cleaner.

This should be doable now.

Still waiting on this change, I think?

cjdb · 2026-08-06T17:29:44Z

    srcs = ["hello_world.carbon"],
 )

+carbon_binary(


Can you move your test to testing/core instead, please? You'll need to create that directory, but we should have a dedicated place for testing the core library.

cjdb · 2026-08-06T17:48:26Z

+  // Test 1: Identical strings
+  var s1: Core.String = "hello";
+  var s2: Core.String = "hello";
+  if (not (s1 == s2)) { return 1; }


I'd prefer our tests print help messages, instead of returning an error code. You won't have robust infrastructure, but producing output that's similar to GoogleTest or Catch2 output would be great.

It will probably be worthwhile setting up a rudimentary test infrastructure in a separate PR. Happy to discuss ideas over in Discord.

cjdb · 2026-08-06T17:48:56Z


 namespace Carbon::Lex {

 class StringLiteral {


Why are the changes in this file relevant to this PR?

I made a bit of a mess here, my apologies. I intend to separate the logic. I allowed my agent to not only leak in regressions but I also mixed commits(my fault for multi-tasking). I will make relevant corrections and remove the irrelevant bits.

ozguronsoy · 2026-08-06T18:27:38Z

+impl String as EqWith(String) {
+  fn Equal(self: String, other: String) -> bool {


IMHO we should eventually make this a built-in function, just like we did with IndexWith in #6329. Can we add a TODO comment for that?

I think it would be best to open a question for the leads to address which Core types should have toolchain-implemented functionality before adding TODOs.

Implement EqWith(String) for Core.String in core/prelude/types/string.carbon. The equality comparison short-circuits on size inequality and compares character elements in an index loop. Add testing/core/string_eq_test.carbon to verify equality and inequality operators across identical, distinct, length-mismatched, and empty strings with structured test reporting. Fixes: carbon-language#7610

cjdb

Only the while loop to go, then should be LGTM!

cjdb · 2026-08-12T00:24:50Z

+    if (self.Size() != other.Size()) {
+      return false;
+    }


That's fine, let's leave it as-is for now.

cjdb · 2026-08-12T00:25:40Z

+    var i: i64 = 0;
+    while (i as u64 < self.Size()) {


This should be doable now.

cjdb · 2026-08-12T17:00:12Z

Swapping with @dwblaikie so you get an approval after your update lands (I won't be able to press the button for a couple of days.)

dwblaikie · 2026-08-27T19:02:49Z

+      return false;
+    }
+    var i: i64 = 0;
+    while ((i as u64) < self.Size()) {


Suggested change

while ((i as u64) < self.Size()) {

let indices: IntRange(u64) = IntRange(i64).Make(0, self.Size());

for (i: i64 in indices) {

Previously suggested/still waiting on this (the comment got detached from the code due to other changes)

Both changes are in, and the suggestion surfaced two adaptations worth explaining.

On the spelling, IntRange is parameterized by a bit-width literal (class IntRange(N: IntLiteral)), so IntRange(i64) doesn't type-check ("cannot implicitly convert expression of type type to Core.IntLiteral"), and Size() returns i64 since #7614. I used for (i: i64 in IntRange(64).Make(0, self.Size())).

On the imports, string.carbon needs both prelude/range (for IntRange) and prelude/iterate (for the for desugaring's Core.Iterate lookup, since prelude files get no implicit prelude import), and each closed a library cycle (types/string → range → iterate → types → types/string, rejected with ImportCycleDetected) because iterate export-imported the whole prelude/types umbrella. I narrowed that to the three libraries iterate actually uses (types/int, types/int_literal, types/optional), which changes nothing for consumers (prelude re-exports prelude/types itself and range already imports the trio directly) and unblocks iteration for any future prelude type library. The goldens pinning iterate.carbon line numbers move by +2.

I verified against the 2026.08.28 nightly toolchain with the modified prelude. The cycle reproduces without the iterate change, and with it the test binary builds and all checks pass. Happy to split the iterate change into a precursor PR if you'd rather review it separately.

`prelude/iterate` export-imported the whole `prelude/types` umbrella, which re-exports `prelude/types/string`. Any prelude type library wanting iteration (via `prelude/range` or `prelude/iterate`) therefore closed a library cycle onto itself, which the toolchain rejects with ImportCycleDetected across the entire prelude. Iterate uses only Int, IntLiteral, and Optional from the umbrella, so import exactly those three libraries. Consumer visibility is unchanged: `prelude` re-exports `prelude/types` itself, and `prelude/range` already imports the three directly. Golden files pinning iterate.carbon line numbers shift by +2 (range_for.carbon diagnostics, array/iterate.carbon debug metadata). Assisted-by: Claude (Fable 5)

Replace the manual index while-loop in String's EqWith impl with `for (i: i64 in IntRange(64).Make(0, self.Size()))`, as requested in review. IntRange(64) rather than the suggested IntRange(i64): IntRange is parameterized by a bit-width IntLiteral, not a type, and String.Size() returns i64 since carbon-language#7614. This also retires the stale `(i as u64)` cast from before carbon-language#7614, trims the test comment claiming a pointer-identity fast path that does not exist, and wraps one over-long test line. Assisted-by: Claude (Fable 5)

dwblaikie

Thanks! & thanks for splitting those review changes up - made it easier to review the import change separately from the IntRange change.

bimokh requested a review from a team as a code owner August 5, 2026 00:36

bimokh requested review from cjdb and removed request for a team August 5, 2026 00:36

github-actions Bot added the toolchain label Aug 5, 2026

bimokh force-pushed the stdlib-string-equality branch from 4bc8c18 to 6211dad Compare August 5, 2026 04:25

cjdb reviewed Aug 6, 2026

View reviewed changes

ozguronsoy reviewed Aug 6, 2026

View reviewed changes

bimokh force-pushed the stdlib-string-equality branch from 9cfb801 to 1f84164 Compare August 11, 2026 20:12

bimokh changed the title ~~Implement String equality operators in the prelude.~~ feat(prelude): implement String equality comparison operators Aug 11, 2026

Merge branch 'trunk' into stdlib-string-equality

32bbfa7

cjdb reviewed Aug 12, 2026

View reviewed changes

cjdb requested review from a team and dwblaikie and removed request for a team August 12, 2026 16:58

dwblaikie reviewed Aug 27, 2026

View reviewed changes

bimokh added 3 commits August 28, 2026 15:05

Merge branch 'trunk' into stdlib-string-equality

94eba52

bimokh force-pushed the stdlib-string-equality branch from e2660f1 to f722873 Compare August 28, 2026 19:43

dwblaikie approved these changes Aug 31, 2026

View reviewed changes

Merge branch 'trunk' into stdlib-string-equality

3020a80

dwblaikie enabled auto-merge August 31, 2026 21:28

		impl String as EqWith(String) {
		fn Equal(self: String, other: String) -> bool {

	while ((i as u64) < self.Size()) {
	let indices: IntRange(u64) = IntRange(i64).Make(0, self.Size());
	for (i: i64 in indices) {

Conversation

bimokh commented Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cjdb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cjdb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cjdb commented Aug 12, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bimokh Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dwblaikie left a comment

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bimokh commented Aug 5, 2026 •

edited

Loading

bimokh Aug 28, 2026 •

edited

Loading