Skip to content

Remove an unused variable - #4100

Open
nico wants to merge 1 commit into
google:masterfrom
nico:unused
Open

nico wants to merge 1 commit into
google:masterfrom
nico:unused

Conversation

@nico

@nico nico commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

New libc++ has a mode that warns on this, and this is part of enabling that mode in Chromium builds.

No behavior change.

Part of https://crbug.com/581374525

@nico
nico requested a review from a team as a code owner November 5, 2026 16:20
@nico

nico commented Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

@battre @mandlil

New libc++ has a mode that warns on this, and this is part of enabling
that mode in Chromium builds.

No behavior change.

Part of https://crbug.com/581374525
@nico nico changed the title Remove an unused variables Remove an unused variable Nov 5, 2026
void EnsureTermination(const string& text, const string& default_country,
PhoneNumberMatcher::Leniency leniency) {
for (size_t index = 0; index <= text.length(); ++index) {
string sub = text.substr(index);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, not quite sure what this test tries to achieve. This line is the only use of index, so not sure what the point is of running this test text.length() times. Maybe the code below wanted to use sub somewhere? Do you know what this attempts to achieve?

As-is, this PR fixes the warning and keeps current behavior.

@battre battre left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

I am not sure how the google-internal library and this are kept in sync. The internal library does not have this line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants